PolymerElements / iron-menu-behavior

Accessible menu behavior
18 stars 28 forks source link

Property to opt out of focus() on _focusedItemChanged #97

Closed jpodwys closed 4 years ago

jpodwys commented 7 years ago

This PR goes with #96.

jpodwys commented 7 years ago

@bicknellr @notwaldorf @cdata @rictic

jpodwys commented 6 years ago

Boy, what a roller coaster of anticipation this past month since I opened this PR has been. Not sure my tender heart can take any more. What say you we merge this little guy?

@bicknellr @notwaldorf @cdata @rictic

bicknellr commented 6 years ago

I think this request might be better served by preventing this line from asking to focus the item. I don't think it really makes sense to disable the focusing behavior in the observer for focusedItem itself, since that's pretty much the only point of setting it. It seems reasonable not to want to focus when selecting an item though - maybe replacing that line with something like this._setFocusedItem(this.theNewPropertyName ? item : null) is closer to what you're looking for? Is calling select the only case that this is a problem?

bicknellr commented 6 years ago

I just noticed that IronMultiSelectableBehavior and IronSelectableBehavior are inconsistent in how they handle fallbackSelection: the first uses select and the second sets selected. Maybe we should update IronMultiSelectableBehavior to set selected instead?

jpodwys commented 6 years ago

I'm currently selecting the first item with .select(0). I tried switching to .selected = 0 and that seems to help a lot. I still see glitches sometimes, but I'm not certain where they're coming from. Regardless, it's objectively better than what I currently have.

As for your proposed change to iron-multi-selectable, I'm still pinned to 1.x of the component which is significantly different, so I wasn't able to check if that fix would work for me.

bicknellr commented 6 years ago

I still see glitches sometimes

Do you mean that it still occasionally focuses the items when you select them?

jpodwys commented 6 years ago

If by "when you select them," you mean when .selected = 0 runs, it appears that way, yes. But I haven't confirmed that's what's happening.