Skip to content

Conversation

@charliepark
Copy link
Contributor

Closes #2417

This does two things. One, it gets rid of the strange empty id that had been in the Listbox elements. (We had already fixed Comboboxes in #2474.)

This also better links the Label and Listbox element, so when you click on the label, the focus goes to the Listbox.

@vercel
Copy link

vercel bot commented Oct 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Oct 9, 2024 1:21am

Copy link
Collaborator

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Do you know why this change fixes the click on the label? More specifically, any idea why it doesn't work on main? On main, the headless <Label> component is hooking up the IDs for us, resulting in the correct accessible name on the button, but I confirmed clicks on it don't focus the field. Weird.

image

Here's what it looks like on this branch. Maybe the fact that we have both the id + for thing and aria-labelledby is making it mad on main. Here we only use the former.

image

@charliepark
Copy link
Contributor Author

I'm trying a few different things in dev tools to get main to function properly, but it doesn't seem to cooperate. Removing aria-labelledby didn't really change it, from what I could tell. Will keep an eye on it on main, though, to see if there's (for some inexplicable reason) a regression from the current branch's working state. Not expecting that, but it's odd that the current version on main seems like it should be working.

@charliepark charliepark merged commit 0a47097 into main Oct 9, 2024
@charliepark charliepark deleted the listbox-id-refactor branch October 9, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix id props in Listbox and Combobox

3 participants