nuxt / ui

A UI Library for Modern Web Apps, powered by Vue & Tailwind CSS.
https://ui.nuxt.com
MIT License
3.94k stars 490 forks source link

[SelectMenu] Broken focus/tab index after selecting #2350

Open OrbisK opened 4 days ago

OrbisK commented 4 days ago

Environment

System: OS: Linux 6.5 Ubuntu 23.10 23.10 (Mantic Minotaur) CPU: (8) x64 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz Memory: 44.48 GB / 62.57 GB Container: Yes Shell: 5.9 - /usr/bin/zsh Browsers: Chrome: 126.0.6478.126

Version

v3.0.0-alpha.6

Reproduction

https://ui3.nuxt.dev/components/select-menu

Description

  1. open docs
  2. open any select menu
  3. select any option
  4. tab to next field
  5. tab index is reset

Additional context

No response

Logs

No response

benjamincanac commented 4 days ago

I'm sorry but I don't understand what the issue is. Would you mind sharing a video maybe?

OrbisK commented 3 days ago

https://github.com/user-attachments/assets/023e6a96-2e2a-4084-b5f4-46847ecc8671

at 0:04 i am opening the select menu at 0:05 i ve selected the option now i press tab to jump tho the next index. next index is not at type?: "label" | "separator" | "item" (it should be see pre 0:04)

benjamincanac commented 3 days ago

You're right, the focus should come back on the button after the menu closes. I'll investigate!

benjamincanac commented 3 days ago

Not sure I can fix this one, the Combobox is designed to focus the ComboboxInput when it closes. However, I've put the ComboboxInput inside the menu to make the SelectMenu.

https://github.com/unovue/radix-vue/blob/main/packages/radix-vue/src/Combobox/ComboboxRoot.vue#L143

OrbisK commented 3 days ago

Not sure I can fix this one, the Combobox is designed to focus the ComboboxInput when it closes. However, I've put the ComboboxInput inside the menu to make the SelectMenu.

https://github.com/unovue/radix-vue/blob/main/packages/radix-vue/src/Combobox/ComboboxRoot.vue#L143

Yes, because the search is in the content. Crap. It's probably also difficult to simply refocus the trigger when closing :/

Maybe it's a bit hacky but you could do something with the provides of the root. https://github.com/unovue/radix-vue/blob/main/packages/radix-vue/src/Combobox/ComboboxRoot.vue#L249

If you want I can test a bit and see how it would be possible. Is it already possible to develop nuxt/ui@v3 locally?

benjamincanac commented 3 days ago

Of course, you just have to checkout the v3 branch, run pnpm install and pnpm run dev:prepare. Then you can run the playground: pnpm run dev or the docs: pnpm run docs.

OrbisK commented 3 days ago

So. the ComboboxInput is very strongly linked to the focusmanagment and keyboards.

I think there are only 2-3 ways to proceed:

  1. get the search input out of the content (input has to be there, but input can be readonly)
  2. separate the input and the search more strongly in radix-vue

(3.) A: I can probably manage to implement it as planned with a few small hacks. I just don't know how strong these hacks are.

PoC of the hack

<ComboboxRoot v-model:open="open">
  <ComboboxAnchor>
          <ComboboxInput
            as="div"
            tabindex="0"
            @keydown="open=true"
          >My real value</ComboboxInput>
          <ComboboxTrigger>
<!-- ... -->
          </ComboboxTrigger>
        </ComboboxAnchor>
        <ComboboxContent>
            <WrapperComponent>
                <ComboboxInput></ComboboxInput>
            </WrapperComponent>
<!-- ... -->
        </ComboboxContent>
</ComboboxRoot>

WrapperComponent has to inject and reprovide the root context but rewrite the onInputElementChange and onInputNavigation to prevent overriding the main inputElement and handle navigation

B: Second hack option might be to try refocusing the trigger when an option is selected and fiddeling the rest (keyboard controls (if no search), multiple selection,...) around that.

In the long term, it would probably be better to create a feature request in radix-vue and until then probably proceed with option 1 or 3. I can also go into more detail for option 3 about how it could work.

I thought i'd just ask you for your opinion before i dive right in.

I hope my explanations make sense

benjamincanac commented 3 days ago

Thanks for the explanation. Not sure I want to start hacking this new version like we did with Headless UI back then. I'm gonna migrate to reka-ui (v2 of radix-vue) once their alpha-2 is released and there are breaking changes on the Combobox component so I would wait to try this.

Once it's done we can submit an issue on Radix Vue to make this work.

OrbisK commented 3 days ago

Thanks for the explanation. Not sure I want to start hacking this new version like we did with Headless UI back then. I'm gonna migrate to reka-ui (v2 of radix-vue) once their alpha-2 is released and there are breaking changes on the Combobox component so I would wait to try this.

Once it's done we can submit an issue on Radix Vue to make this work.

Ahh exciting. Yes, with v2 there's definitely a lot different in terms of keyboard navigation. Apparently, from v2 onwards, the focus for navigation will be directly on the items. As it looks so far, the input component will probably still be the one that gets the focus after the blur. Maybe you could simply ask here with a mini feature request whether you can simply deactivate this with a prop (e.g. passive) so that the component does not register as “main input”.