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

iron-selectable (via iron-pages) fails to apply selected class #16

Closed
hawkett opened this issue May 18, 2015 · 3 comments
Closed

iron-selectable (via iron-pages) fails to apply selected class #16

hawkett opened this issue May 18, 2015 · 3 comments

Comments

@hawkett
Copy link

hawkett commented May 18, 2015

I'm having an issue with iron-pages whereby iron-selectable can get out of sync with the DOM - i.e. the selectedItem is one thing, while the DOM element with the 'iron-selected' class is another.

Unfortunately I have not been able to replicate the issue with a trivial test - but I do see this repeatedly in a more complicated single-page app with a lot of iron-pages components. The problem is reliably repeatable in this context - following the same flow through the application will always deliver the invalid state. Basically it looks like iron-selectable doesn't reliably apply the selected class.

Here's a screen shot example (chrome), which is the result of:

this.$.completionSelector.select(1);
console.log("Selected", this.$.completionSelector.selected, this.$.completionSelector.selectedItem, this.$.completionSelector.items);
console.log("Selector", this.$.completionSelector);

screen shot 2015-05-18 at 11 46 04 am

Item 0 was correctly selected prior to the select(1) call. Note that the iron-pages component is itself an 'iron-page', but I see this problem occurring when they're not nested.

@hawkett hawkett changed the title iron-selectable (via iron-pages) can get into invalid state iron-selectable (via iron-pages) has invalid state May 18, 2015
@hawkett hawkett changed the title iron-selectable (via iron-pages) has invalid state iron-selectable (via iron-pages) fails to apply selected class May 18, 2015
@manifest
Copy link

Same issue. We should remove 'activateEvent' listener of the 'Polymer.IronSelectableBehavior' when we are using 'iron-pages'.

var p = this.$.pages;
p._removeListener(p.activateEvent);

I have created an issue for the 'iron-pages' element with the code example.

@hawkett
Copy link
Author

hawkett commented Jun 6, 2015

Further digging into this problem, @manifest is describing a different issue - just with very similar effects.

jsbin demonstrating this issue

There's a bunch of things going on here, but essentially there's a race condition that will prevent a call to select() from actually triggering the complex observer on the state change.

The problem occurs in two parts - highlighted in the jsbin comments.

1 - Trying to call select() before the iron-pages component is finished initialising results in an invalid state - properties have not yet been initialised, and the complex observer _updateSelected(attrForSelected, selected) in iron-selectable needs attrForSelected to have been set. The following debug view helps understand why - when we run through the args to the observer attrForSelected is undefined, which returns immediately when there's more than 1 argument. Not returning args means we never call _updateSelected(): code ref

screen shot 2015-06-05 at 5 38 42 pm

2 - Nothing will be actioned when we subsequently call select() if we don't actually change the value: code ref - therefore subsequent attempts to select the same page will also fail.

I can see a whole bunch of ways this might be resolved, but maybe a first step is to make sure the 'old' value is not set if the component is in an invalid state. At least subsequent calls to set the value when the component becomes initialised will succeed.

Perhaps the answer is for the developer to be responsible for avoiding these races, but I can see them being a nightmare to debug and weed out in complex applications.

Note: I've deleted my previous comment that contained an incorrect jsbin - it's still available in PolymerElements/iron-pages#3

@hawkett
Copy link
Author

hawkett commented Jun 6, 2015

Thinking on this some more, I can see that the approach used in the jsbin above is considered an anti-pattern, and that this jsbin is a far better way to approach this trivial case. That said, there's definitely been discussion polymer slack about difficulties reasoning about code with undefined properties, so there might still be something here.

Independent of that, I think there's still a problem lurking here - namely that polymer considers setting a property to be idempotent code ref, however the first jsbin shows that this is not the case. It's easy to see when undefined values break idempotence from the image in the previous post. Perhaps the observer should be called with the undefined value rather than not called at all.

Anyhow, this discussion is no longer about iron-selector - going to close this issue and link to it from something over at Polymer/polymer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants