unovue / radix-vue

Vue port of Radix UI Primitives. An open-source UI component library for building high-quality, accessible design systems and web apps.
https://radix-vue.com
MIT License
3.54k stars 217 forks source link

[Bug]: Web component radio group arrow key bug #1003

Closed cain closed 1 month ago

cain commented 4 months ago

Environment

Node version: v18.20.3
Radix Vue version: ^1.8.3
Vue version: "^3.4.27"

Client OS: Macos 14.4.1
Browser: Chromium Engine Version 126.0.6478.57

Link to minimal reproduction

https://stackblitz.com/edit/vitejs-vite-shxgju?file=src%2Fcomponents%2FRadioGroup.vue

Steps to reproduce

see reproduction link, and try the up and down arrow once selecting an item.

Describe the bug

The selection seems to occur multiple times based on the number of radio inputs. Inspecting the code it appears to be something todo with the ref https://github.com/radix-vue/radix-vue/blob/main/packages/radix-vue/src/RadioGroup/RadioGroupItem.vue#L32

Is working fine when not using web components

Expected behavior

Radio group should be selecing up and down based on the arrow keys

Context & Screenshots (if applicable)

https://github.com/radix-vue/radix-vue/assets/15246256/f4a8f402-9999-4982-bdea-9736eb5b9e0c

https://github.com/radix-vue/radix-vue/assets/15246256/c412390a-7cf2-4b2c-b52c-97494742f3fc

Im assuming its something to do with the context of being inside a web component? Just guessing here, Im not to familiar with this project, but love and appreciate all the work that has been done.

I've only tested radio group component, im assuming others are breaking as well

sunnylost commented 1 month ago

This happens because, when using a web component, the element resides in the shadow DOM. The focusFirst function in RovingFocus/utils.ts relies on document.activeElement, which always returns the RadioGroup element instead of the actual focused radio item. As a result, every time you use the arrow keys, all three items are iterated over, and eventually, the last item receives focus.

To resolve this bug, we need to always pass a rootNode to retrieve its activeElement:

// RovingFocusItem.vue
const elRef = ref()
const rootNode = computed(() => elRef.value?.$el.getRootNode())

// utils.ts
export function focusFirst(candidates: HTMLElement[], preventScroll = false, rootNode = null) {
  const PREVIOUSLY_FOCUSED_ELEMENT = (rootNode ?? document).activeElement
  // remains the same
}
cain commented 1 month ago

@sunnylost thanks for checking out my issue and giving such a detailed response. Appreciate it! I've submitted a PR