PolymerElements / iron-selector

Manages a list of elements that can be selected
32 stars 55 forks source link

Disabled items should not be selectable #99

Open cdata opened 8 years ago

cdata commented 8 years ago

When calling methods on selectable elements, it should be possible to exclude disabled items from being selectable.

User story:

I'm looking to change the tabs within a paper-tabs element, so for example based on a property I only want to show a few of the tabs. I use element.selectNext() quite frequently and I noticed that hiding a tab is not enough for it not to be selected.

bicknellr commented 8 years ago

I think it's reasonable to check for an item's disabled property / attribute in _itemActivate and bail out if set, but I don't think we should prevent selection through the API.

Maybe we should expose the _xToY internal functions? (And the function that checks if an item is 'disabled', if added.) This might make it easier to map between items / values / indices and implement this logic externally. Particularly problematic: we expose .items (giving the array of selectable nodes) and .select(value) (which takes the value to select) but not _valueForItem. This means that you can't use the same logic IronSelectableBehavior does internally to determine the value to pass to .select(value). So, as a user of IronSelectableBehavior's API, you have to duplicate this logic yourself somewhere, but this logic can change without notice because it's 'protected'!

Zecat commented 8 years ago

+1 It's a common case to have disabled items in a selectable list e.g. IronMenuBehavior.

Sorry @bicknellr I don't really understand what you say in your 2nd paragraph so I'll talk about the first one. _itemActivate is only triggered when activateEvent is received. So if the selection happens because of a method or a change of selectedattribute, the disabled check will be ignored.

_updateSelected seems to me a better place to do the check.

Changing

observers: [
  …
  '_updateSelected(selected)',
  …
],

to

selected: {
  type: String,
  notify: true,
  observer: `_updateSelected`
},

That way, _updateSelected would know the previous value of selected and reset it if necessary according to the check.

Also, we would need two new methods, to select the previous and next non-disabled item.

MartonEstok commented 7 years ago

+1

ronnyroeller commented 7 years ago

+1

PPCM commented 6 years ago

+1

benesjan commented 6 years ago

+25.482631586