Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correctly concatenate aria-describedby in a slot #2600

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Correctly concatenate aria-describedby in a slot #2600

wants to merge 1 commit into from
+71 −4

Conversation

wbolduc
Copy link

@wbolduc wbolduc commented Dec 19, 2023

Description

aria-describedby is technically a list of ids and should be concatenated kind of like className.

This pr fixes my own issue where this is described

Tests are included

@wbolduc
Copy link
Author

wbolduc commented May 13, 2024

Is there something wrong with my pr?

@chaance
Copy link
Member

chaance commented Sep 27, 2024

I'm not entirely sure we want to do this. Yes, aria-describedby can be more than one string but I think the component should be explicit about which element(s) are used as descriptions. With this solution there's no way for a consumer to explicitly override what Radix does internally, which may be valid in some cases.

@chaance chaance added Priority: Medium Medium priority issue Resolution: Needs Investigation This issue needs more investigation labels Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefanofa stefanofa stefanofa approved these changes

@benoitgrelard benoitgrelard Awaiting requested review from benoitgrelard

@andy-hook andy-hook Awaiting requested review from andy-hook

Assignees
No one assigned
Labels
Priority: Medium Medium priority issue Resolution: Needs Investigation This issue needs more investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

aria-describedby is not assigned for tooltip if an aria-describedby is already present
3 participants
@wbolduc @chaance @stefanofa