search

polymer-dart / polymer_autonotify

auto notifier support for polymer 1.0.
Other
6 stars 2 forks source link

update to not bash over Polymer.Dart.InteropBehavior #2

Closed jakemac53 closed 9 years ago

dam0vm3nt commented 9 years ago

ah ah ah : did an almost identical commit some seconds ago ... funny.

2015-09-30 22:31 GMT+02:00 Jacob MacDonald notifications@github.com:

You can view, comment on, or merge this pull request online at:

https://github.com/dam0vm3nt/polymer_autonotify/pull/2 Commit Summary

  • update to not bash over Polymer.Dart.InteropBehavior

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/dam0vm3nt/polymer_autonotify/pull/2.

dam0vm3nt commented 9 years ago

I'll buy the new behavior + levereging "_applied" stuff. I'm merging.

dam0vm3nt commented 9 years ago

@jakemac53 mmh. it is not working because InteropBehavior._propertyChanged is going to be executed before AutoNotifyBehavior causing the endless loop. Any idea to force it?

dam0vm3nt commented 9 years ago

sorry: had to still bash InteropBehavior a little ...

jakemac53 commented 9 years ago

What did you need to bash over for InteropBehavior? I did a really simple example app where I could add/remove items from a list and it seemed ok. The problem with bashing over InteropBehavior is that is going to affect ALL elements, not just the ones using this behavior.

jakemac53 commented 9 years ago

Is the issue if the list is bound to multiple elements? Maybe the non-auto-notify element is getting its _propertyChanged called first? I didn't check this case, but its worth verifying.

dam0vm3nt commented 9 years ago

exactly my test case is one element and a 2 child elements. the list is in main element and binded in the two children. Think I will add this example in the project someday.

2015-09-30 23:58 GMT+02:00 Jacob MacDonald notifications@github.com:

Is the issue if the list is bound to multiple elements? Maybe the non-auto-notify element is getting its _propertyChanged called first?

— Reply to this email directly or view it on GitHub https://github.com/dam0vm3nt/polymer_autonotify/pull/2#issuecomment-144553923 .

dam0vm3nt commented 9 years ago

actually seams that AutoNotify._propertyChanged is never called I'm investigating.

dam0vm3nt commented 9 years ago

So : now it is working again. For some reason AutoNotify._propertyChanged was never called ( @jakemac53 : can you give a hint? ). Until I understand better I've hooked original InteropBehavior._propertyChanged but only for element that mix PolymerAutonotifySupportBehavior.

dam0vm3nt commented 9 years ago

In the end I had to add this to make the mixin reflected. Is this the correct way (I didn't find this on the docs). Also in elements using the behavior I had to specify both in the right order (I thought it was implicit ).

jakemac53 commented 9 years ago

In the end I had to add this to make the mixin reflected. Is this the correct way (I didn't find this on the docs).

You should not have to add the @jsProxyReflectable annotation, there was a bug for a little bit where you did so I would just make sure you have the most recent version.

Also in elements using the behavior I had to specify both in the right order (I thought it was implicit ).

Polymer should throw an error if you get them in the wrong order, but it is up to the developer to physically do it since polymer can't change the semantics of how mixins work. The error messages here should be pretty good though (I think it actually gives you the correct ordering which you can copy and paste it).

dam0vm3nt commented 9 years ago

Polymer should throw an error if you get them in the wrong order,..

It will and it is also pretty clear. But before I believed that having a behavior implementing another was just enough and one was not obliged to specify it again to enforce them on polymer js side. After all polymer-dart has all the info that are needed to create the chain of behaviors and in the right order

jakemac53 commented 9 years ago

Behaviors can also define instance methods and properties, in order to be able to access those you have to physically mix in the class. This is a bit weird for some behaviors (specifically ones which don't actually add any instance methods or properties), but that should be a relatively uncommon case, and at least at this time its not worth optimizing for in my opinion.