This repository has been archived by the owner on Mar 13, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 20
In single selection, polymer-select event should fire once per selection change #15
Comments
Yes, I see how this is potentially confusing. Fwiw, there are more uses cases than the one you describe. E.g., the selectors support multiple selection, where elements can come and go from the selection list without being in matched pairs. [Edit: ok, I see you considered this and put for 'single selection' in the issue title.] Initially we had two separate events, but it became onerous to have to listen to two events, and it was simpler to have a single 'selection state changed for item' event. Maybe there is a way to make everybody happy, will have to cogitate on it. |
I agree there's a lot to consider here. As you're thinking about this, here's some more food for thought:
Anyway, I appreciate the complexity here. Just sharing some thoughts. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Currently, selecting an item on a
<polymer-selection>
triggers two discrete polymer-select events: one for deselecting the previously-selected item, and another for selecting the new item. To me, at least, this was counter-intuitive — I was expecting a single event.I'd had code that needed to update when the selection changed, and sinking polymer-select caused the code to be invoked twice. I eventually worked out that my handler needed to check event.detail.isSelected to determine whether it was handling the deselection (which I didn't care about) or the selection (which I did).
I'd hazard that most UIs that need to track the selection state (e.g., to show details for a selected item) would want to do the same thing. So I thought it'd be nicer if the polymer-selection were smart enough to just fire the event once, at least for the common single-selection (not multi) case.
There's an existing comment in setItemSelected about replacing the current event with summary notifications. Perhaps that would address this issue, but even if that isn't pursued, it'd be nice to just fire a single event in this common case.
The text was updated successfully, but these errors were encountered: