Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polymer.dart attributes are missing features: literals, attributeChanged, case insensitivity #12262

Closed
sethladd opened this issue Aug 6, 2013 · 19 comments
Assignees
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Milestone

Comments

@sethladd
Copy link
Contributor

sethladd commented Aug 6, 2013

This works in polymer.js, but not polymer.dart.

<!DOCTYPE html>

<html>
  <head>
    <title>index</title>
    <script src="packages/polymer/boot.js"></script>
  </head>
 
  <body>
    <polymer-element name="my-element" attributes="volume">
      <template>
        <h1>Hello from the custom element. The volume is {{volume}}</h1>
      </template>
      <script type="application/dart">
        import 'package:polymer/polymer.dart';
        
        @­CustomTag('my-element')
        class MyElement extends PolymerElement with ObservableMixin {
          @­observable String volume;
        }
      </script>
    </polymer-element>
    
    <!-- I don't see "The volume is 11", I just see "The volume is" -->
    
    <my-element volume="11"></my-element>
      
    <script type="application/dart">main() {}</script>
  </body>
</html>

Here's the JS version that works:

<html>
<head>
    <title>Test</title>
  <script src="polymer.min.js"></script>
</head>
<body>

<polymer-element name="my-element" attributes="volume">
  <template>
    <h1>Hello from the custom element. The volume is {{volume}}</h1>
  </template>
  <script>
    Polymer('my-element', {
      volume: 1
    });
  </script>
</polymer-element>

<my-element volume="11"></my-element> <!-- I correctly see "The volume is 11" -->

</body>
</html>

@jmesserly
Copy link

fyi, fix is here:
https://chromiumcodereview.appspot.com/22417004/

we were tracking this in https://github.com/dart-lang/web-ui/issues/508 though :)

edit: fix link

@sethladd
Copy link
Contributor Author

sethladd commented Aug 6, 2013

@sethladd
Copy link
Contributor Author

sethladd commented Aug 6, 2013

Wonderful, I grabbed the diff from the review site and applied it locally. It works! (fwiw, the patch isn't yet in github's polymer branch)

@jmesserly
Copy link

yeah, we are debating in the code review what the right behavior is if the number fails to parse. Unfortunately polymer.js isn't useful for guidance as they preserve the string (which in Dart wouldn't work, would just cause a checked mode failure)

@sethladd
Copy link
Contributor Author

sethladd commented Aug 6, 2013

FWIW this worked:

    <polymer-element name="my-element" attributes="volume,intThing">
      <template>
        <h1>Hello from the custom element. The volume is {{volume}}
        and intThing is {{intThing}}</h1>
      </template>
      <script type="application/dart">
        import 'package:polymer/polymer.dart';
        
        @­CustomTag('my-element')
        class MyElement extends PolymerElement with ObservableMixin {
          @­observable String volume;
          @­observable String intThing;
        }
      </script>
    </polymer-element>
    
    <!-- I don't see "The volume is 11", I just see "The volume is" -->
    
    <my-element volume="11" intThing="42"></my-element>

but this did not:

    <polymer-element name="my-element" attributes="volume,intThing">
      <template>
        <h1>Hello from the custom element. The volume is {{volume}}
        and intThing is {{intThing}}</h1>
      </template>
      <script type="application/dart">
        import 'package:polymer/polymer.dart';
        
        @­CustomTag('my-element')
        class MyElement extends PolymerElement with ObservableMixin {
          @­observable String volume;
          @­observable int intThing;
        }
      </script>
    </polymer-element>
    
    <!-- I don't see "The volume is 11", I just see "The volume is" -->
    
    <my-element volume="11" intThing="42"></my-element>

When I use '@observable int intThing' I get "and intThing is"

However, if I initialize intThing to an integer, like this:

@observable int intThing = 0;

then I get "and intThing is 42"

Sounds like the rule is: if you want to use a type annotation other than String for your field, be sure to initialize it with a default value. I think this matches how polymer.js works.

@DartBot
Copy link

DartBot commented Aug 9, 2013

This comment was originally written by @sethladd


Hi guys,

Is it too late to get this in in time for the next integration build? This is a great feature.

@DartBot
Copy link

DartBot commented Aug 9, 2013

This comment was originally written by joh...@johnmccutchan.com


Is there a way to plugin my own attribute string parse routine for custom types like Color, Vector3, Matrix4, etc?

@jmesserly
Copy link

@john -- yes there most definitely should be!
Polymer has several features that help here:

deserializeValue -- you can override this to provide your own serialization

deserializing JSON -- it attempts to deserialize into JSON by default.

I'm pretty confused about what to do with respect to JSON. We could use json.parse in Dart, and give you maps and lists and primitives and stuff. Or we could try to deserialize into a Dart type. At that point, we'd have to look at type annotations... which suggests maybe we should just take advantage of our type annotations here.

Signed, - A Confused Polymer Porter.

@jmesserly
Copy link

update: leaning towards using type annotations if available, and fall back to the default value's type if no other information is available. This is one place we can lean on the type annotations in Dart to provide a better experience. (and this is how real DOM HTMLElements work, after all -- the property type is specified in the IDL).

@jmesserly
Copy link

Set owner to @jmesserly.
Removed Type-Defect label.
Added Type-Enhancement, Started labels.

@jmesserly
Copy link

Removed Priority-Unassigned label.
Added Priority-High label.

@sethladd
Copy link
Contributor Author

sethladd commented Aug 9, 2013

Woot!

@jmesserly
Copy link

Issue #12715 has been merged into this issue.

1 similar comment
@sethladd
Copy link
Contributor Author

Issue #12715 has been merged into this issue.

@larsbak
Copy link

larsbak commented Aug 27, 2013

Added this to the M7 milestone.

@sethladd
Copy link
Contributor Author

sethladd commented Sep 4, 2013

Changed the title to: "Custom element, custom attribute, setting to a literal value doesn't work".

@sethladd
Copy link
Contributor Author

sethladd commented Sep 9, 2013

Added Polymer-WebUICompat label.

@jmesserly
Copy link

edit subject -- this is now tracking all attribute incompatibilities vs Polymer:

* Setting to a literal value doesn't work

  • attributeChanged event does not work
  • there is no way to publish from code ("publish:" in JavaScript, probably @­publish in Dart), only "attributes=" on polymer-element is supported
  • there may be issues with property binding as well (https://code.google.com/p/dart/issues/detail?id=13152)

Changed the title to: "Polymer.dart attributes are missing features: literals, attributeChanged, case insensitivity".

@jmesserly
Copy link

* also, the dash-separated vs camelCase issue

@sethladd sethladd added Type-Enhancement P1 A high priority bug; for example, a single project is unusable or has many test failures labels Sep 13, 2013
@sethladd sethladd added this to the M8 milestone Sep 13, 2013
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants