open-ui
open-ui copied to clipboard
openui
Remove eager selection of selectlist option with arrow keys
I agree that a change shouldn’t fire until the user has purposefully chosen an option, but with current select elements the displayed option in the triggering element does change. And that can be useful to keep.
I agree that a change shouldn’t fire until the user has purposefully chosen an option, but with current select elements the displayed option in the triggering element does change. And that can be useful to keep.
Does that typically happen for userland select menus? I feel (nothing more than a feeling) like I haven't seen that behavior anywhere but the builtin <select>. If so, perhaps we should think about what's right?
I agree that a change shouldn’t fire until the user has purposefully chosen an option, but with current select elements the displayed option in the triggering element does change. And that can be useful to keep.
We resolved in the issue in the commit message to not do this, right?
I feel like the preview should show the actually selected option. If it was to change with navigation but then disappear with escape (or clickoutside) I would find that confusing personally. But like mason that's not based on anything more than intuition and not having seen it in many custom implementations.
I agree that a change shouldn’t fire until the user has purposefully chosen an option, but with current select elements the displayed option in the triggering element does change. And that can be useful to keep.
I think it's also important to point out that the above statement - that the displayed option in the triggering element does change - is a Windows-specific observation. It is not true, for example, on a Mac. For one, the popover covers the in-page element. But if you're careful, you can observe what's underneath it (e.g. via sharing your screen in a Meet, because that does not share the popover) and you can see that it does not change as you use arrow keys to change options in the popover.
Certainly a change event should not be fired until a new option is selected, however that's done.
Yeah I thought we basically agreed to do the mac behavior. This is what I implemented in the chromium prototype if you look in canary: https://jsfiddle.net/xhsofq2z/4/
I agree that a change shouldn’t fire until the user has purposefully chosen an option, but with current select elements the displayed option in the triggering element does change. And that can be useful to keep.
Does that typically happen for userland select menus? I feel (nothing more than a feeling) like I haven't seen that behavior anywhere but the builtin
<select>. If so, perhaps we should think about what's right?
i'd submit you don't typically see this in most custom implementations similarly to why you don't see the ability to start typing chracters and have selection/focus move to the option that matches the typed characters - i'd submit it's an overlooked detail (which can be useful if the selected vs focused option has poor visual indicators) rather than a purposeful omission.
Regardless, I'm not going to strongly advocate that we do this. I just didn't think that the decision to not have selection follow focus had to mean this behavior was off the table too. But if people don't see the value in it, then we can move on.
Separately though, @josepharhar, i noticed in the demo you posted some oddities. rather than get into those here, i'll post a new issue
So what is the next step to get this PR unblocked? @scottaohara said he won't push back here, the change aligns with the resolution in the issue. I'm going to approve and the issue can be re-opened if someone wants to change it.
I was concerned about https://github.com/openui/open-ui/issues/1087 and this contradicting each other but it seems that the desire is still to not do selection follow focus so this change is correct.
Correct me if I'm wrong @josepharhar ?
Given the issue I just mentioned though I think this probably does need some wider discussions. Specifically where do we draw the line between "platform convention" and actually having a good behaviour 😅
Either way I think we should remove this text as it's not currently the plan and won't always be correct regardless.
I was concerned about #1087 and this contradicting each other but it seems that the desire is still to not do selection follow focus so this change is correct.
Correct me if I'm wrong @josepharhar ?
This is correct, I approve this PR