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

add style.label to aria-label #2169

Merged
merged 13 commits into from Jun 7, 2023
Merged

add style.label to aria-label #2169

merged 13 commits into from Jun 7, 2023

Conversation

jshawl
Copy link
Member

@jshawl jshawl commented May 15, 2023

Description

This PR adds localized text to the aria-label for the PayPal button when style.label is set.

Why are we making these changes? Include references to any related Jira tasks or GitHub Issues

Improve accessibility with more accurate aria-label values. Previously, the name of the funding source was used, which ended up being "PayPal" even in cases where a label was set.

Screenshots (if applicable)

Screen Shot 2023-06-06 at 16 20 14

❤️ Thank you!

@jshawl jshawl changed the title read aria-label from xo-content add style.label to aria-label May 15, 2023
@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Patch coverage: 54.54% and project coverage change: +0.08 🎉

Comparison is base (6209c4f) 51.49% compared to head (a798134) 51.58%.

❗ Current head a798134 differs from pull request most recent head 44c86aa. Consider uploading reports for the commit 44c86aa to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2169      +/-   ##
==========================================
+ Coverage   51.49%   51.58%   +0.08%     
==========================================
  Files         101      101              
  Lines        1967     1983      +16     
  Branches      572      582      +10     
==========================================
+ Hits         1013     1023      +10     
- Misses        856      862       +6     
  Partials       98       98              
Flag Coverage Δ
jest 31.81% <54.54%> (-0.04%) ⬇️
karma 50.33% <25.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/funding/common.jsx 60.86% <ø> (ø)
src/funding/paypal/config.jsx 58.33% <50.00%> (-41.67%) ⬇️
src/ui/buttons/button.jsx 82.27% <100.00%> (ø)

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jshawl jshawl marked this pull request as ready for review June 6, 2023 21:39
@jshawl jshawl requested a review from a team as a code owner June 6, 2023 21:39
@jshawl jshawl requested a review from wsbrunson June 6, 2023 21:44
Copy link
Contributor

@nbierdeman nbierdeman left a comment

Choose a reason for hiding this comment

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

Nice work! 🔥 🥇

@jshawl jshawl requested review from nbierdeman and a team June 7, 2023 00:16
Comment on lines +46 to +61
labelText: ({ content, label, period }) => {
let text = `${FUNDING_BRAND_LABEL.PAYPAL}`;
if (content && label === BUTTON_LABEL.INSTALLMENT) {
if (period) {
text = content["label.installment.withPeriod"].replace(
"{period}",
String(period)
);
} else {
text = content["label.installment.withoutPeriod"];
}
} else if (content && label) {
text = content[`label.${label}`];
}
return text;
},
Copy link
Member

Choose a reason for hiding this comment

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

we should consider unit testing this

Copy link
Member Author

Choose a reason for hiding this comment

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

good call - let me see if that could be a quick win!

@jshawl jshawl merged commit 29a18ec into paypal:main Jun 7, 2023
2 checks passed
@jshawl jshawl deleted the a11y-label-text branch June 7, 2023 16:02
jshawl added a commit to jshawl/paypal-checkout-components that referenced this pull request Jun 15, 2023
This PR is a follow up to paypal#2169 (29a18ec) that introduced
an issue related to the correct content not being available.
@jshawl jshawl mentioned this pull request Jun 15, 2023
jshawl added a commit that referenced this pull request Jun 16, 2023
This PR is a follow up to #2169 (29a18ec) that introduced
an issue related to the correct content not being available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants