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

fix: allow pass-through of accessibilityRole on Chip #4327

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

meatnordrink
Copy link
Contributor

Motivation

The Chip component is frequently used for display purposes, and in these cases is often never clickable. However, the Chip element still announces as a button because it has accessibilityRole="button" hardcoded.

When the Chip element is not being used as a button, a more sensible accessibilityRole should be able to be passed through. This PR keeps the default "button" accessibilityRole, but adds the ability to pass in a different accessibilityRole if "button" doesn't make sense.

Related issue

n/a

Test plan

  • Pass accessibilityRole='text' as a prop to a Chip, and navigate to the Chip via screen reader. It should not announce as a button.
  • Do not pass any accessibilityRole prop, and navigate to the chip via screen reader; it should announce as a button.

I have tested this locally, and it works as expected.

@callstack-bot
Copy link

Hey @meatnordrink, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@billomore
Copy link

billomore commented Apr 25, 2024

very simple, straightforward and important PR. As one who uses chips for display purposes often, it will help a lot

@gedu gedu added this to the 5.12.4 milestone Apr 26, 2024
@gedu
Copy link
Contributor

gedu commented Apr 27, 2024

LGTM, this PR will be on version 5.12.4

@jcubic
Copy link

jcubic commented May 19, 2024

@gedu can you explain why the PR was not merged, only is waiting for release? How you can release something if the PR was not merged?

I'm asking because I want to contribute, and no one even bother to merge a valid PR that was accepted. A lot of unanswered issues. Doesn't look like a well maintained project.

@gedu
Copy link
Contributor

gedu commented May 20, 2024

@gedu can you explain why the PR was not merged, only is waiting for release? How you can release something if the PR was not merged?

I'm asking because I want to contribute, and no one even bother to merge a valid PR that was accepted. A lot of unanswered issues. Doesn't look like a well maintained project.

Hey, the main maintainer is on vacation, and I was hopping to chat with him to know more about the process, that's why I didn't merge it. On the other hand, the last weeks were really complicated on my side. I tried to replay Issues and on Discord
We really appreciate your contribution, hope you can keep doing it.

@gedu gedu self-requested a review June 6, 2024 06:42
@lukewalczak lukewalczak merged commit b1fd739 into callstack:main Jul 27, 2024
3 checks passed
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.

6 participants