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.

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

feat: add "Accessible Label" setting #5063

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

Conversation

aleclarson
Copy link
Contributor

@aleclarson aleclarson commented Mar 27, 2025

Description

Allow the user to set the aria-label attribute.

This is maybe a controversial feature, but it would be useful for post-processing that I'm doing on the generated output. 😅

Screenshot 2025-03-27 at 14 54 23 Screenshot 2025-03-27 at 14 56 51

Before merging

  • tested locally and on preview environment (preview dev login: 0000)
  • updated test cases document
  • added tests
  • if any new env variables are added, added them to .env file

@kof
Copy link
Member

kof commented Mar 27, 2025

User can add aria-label to any instance, not sure why aria-label needs to become a system property.

image

@kof
Copy link
Member

kof commented Mar 27, 2025

It would be nice to expose aria-label on all components where likelyhood of using it is high as initialProperties, so that user doesn't need to add it, its already visible

@TrySound
Copy link
Member

Is it necessary for all components or only buttons and links which often have icons as content?

@TrySound
Copy link
Member

So yes the best option is specifying aria-label where needed
https://github.com/webstudio-is/webstudio/blob/main/packages/sdk-components-react/src/button.ws.ts#L33

System props and core components are used to manage their behaviour with builder and cli instead of generic code generation.

@kof
Copy link
Member

kof commented Mar 28, 2025

I meant adding it to initial only on components where users would be using aria label potentially, buttons, links and similar, I don't remember if we have it in initialProps in all of them, maybe its a non-issue.

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.

3 participants