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

[Badge] Add customAccessibilityLabel prop #4028

Merged
merged 4 commits into from Mar 4, 2021

Conversation

emma-boardman
Copy link
Contributor

WHY are these changes introduced?

Experimenting with a proposed feature request #4027

WHAT is this pull request doing?

  • Adds a customAccessibilityLabel for consumer's who need to pass more specific labels.

How to 🎩

  • Access Badge with customAccessibilityLabel story
  • Enable Voiceover

🎩 checklist

@emma-boardman emma-boardman added Accessibility Needs design, development, or content work relating to accessibility. Feature request labels Mar 2, 2021
@emma-boardman emma-boardman self-assigned this Mar 2, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2021

🟡 This pull request modifies 4 files and might impact 63 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 63)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Badge/Badge.tsx (total: 63)

Files potentially affected (total: 63)

📄 src/components/Badge/README.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Badge/tests/Badge.test.tsx (total: 0)

Files potentially affected (total: 0)

@@ -23,6 +23,8 @@ export interface BadgeProps {
* @default 'medium'
*/
size?: Size;
/** Pass a custom accessibilityLabel (overrides status and progress labels) */
customAccessibilityLabel?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR looks good to me but I worry that this prop might be misunderstood and get misused. I like that it's different from the default accessibilityLabel prop but I'm not sure the name tells you enough about what it does.

What about something terribly verbose?
statusAndProgressLabelOverride

<Badge
status="success"
progress="complete"
customAccessibilityLabel="Status: Published. Your online store is visible."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
customAccessibilityLabel="Status: Published. Your online store is visible."
statusAndProgressLabelOverride="Status: Published. Your online store is visible."

@emma-boardman
Copy link
Contributor Author

Hello again 👋 😄

First time merging since Percy got added - do I need to be authorised in Percy to approve? I can access the UI, but my Approve build button is disabled. Or is that because I'm the PR author?

image

@kyledurand
Copy link
Contributor

✅ Percy requires a polaris admin to approve

Good to 🚢 now! Thanks Emma!

@emma-boardman emma-boardman merged commit 96cd8e2 into main Mar 4, 2021
@emma-boardman emma-boardman deleted the badge-add-custom-a11y-label branch March 4, 2021 16:17
@emma-boardman emma-boardman restored the badge-add-custom-a11y-label branch March 4, 2021 16:18
@emma-boardman emma-boardman deleted the badge-add-custom-a11y-label branch March 4, 2021 17:34
sylvhama pushed a commit that referenced this pull request Mar 26, 2021
* add customA11yLabel prop, update docs and tests

* update unreleased

* update prop name and description

* update readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Needs design, development, or content work relating to accessibility. Feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants