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

TCCP: Tooltip accessibility improvements #8506

Merged
merged 6 commits into from
Jul 15, 2024
Merged

TCCP: Tooltip accessibility improvements #8506

merged 6 commits into from
Jul 15, 2024

Conversation

contolini
Copy link
Member

@contolini contolini commented Jul 10, 2024

Adds some code to close tooltips when esc key is pressed per digital.gov guidance. Also adds an aria-label to card results that summarizes all the card details to assistive technologies.

How to test this PR

  1. Everything should look the same.
  2. Hover over a tooltip and click esc to close it.
  3. yarn cypress run --spec test/cypress/integration/consumer-tools/credit-cards/explore-cards.cy.js

Checklist

  • PR has an informative and human-readable title
    • PR titles are used to generate the change log in releases; good ones make that easier to scan.
    • Consider prefixing, e.g., "Mega Menu: fix layout bug", or "Docs: Update Docker installation instructions".
  • Changes are limited to a single goal (no scope creep)
  • Code follows the standards laid out in the CFPB development guidelines
  • Future todos are captured in comments and/or tickets
  • Project documentation has been updated, potentially one or more of:
    • This repo’s docs (edit the files in the /docs folder) – for basic, close-to-the-code docs on working with this repo
    • CFGOV/platform wiki on GHE – for internal CFPB developer guidance
    • CFPB/hubcap wiki on GHE – for internal CFPB design and content guidance

Browser testing

Check the current browser support cutoff list](https://cfpb.github.io/consumerfinance.gov/browser-support#current-browser-support-metrics) for browsers that are advisable
to prioritize for testing.

Accessibility

  • Keyboard friendly (navigable with tab, space, enter, arrow keys, etc.)
  • Screen reader friendly
  • Does not introduce new errors or warnings in WAVE

Other

  • Is useable without CSS
  • Is useable without JS
  • Does not introduce new lint warnings
  • Flexible from small to large screens

Using Axe inside our Cypress tests lets us check a11y compliance at any
desired state of the app.

It's installed globally so that other Bureau products can use it.

https://www.npmjs.com/package/cypress-axe
The contrast fails WCAG 2.0 AA at normal-sized text. The text size in wells
is debatably "large" text but because the color change is visually
imperceivable, IMHO it's worth changing.

Pacific: https://webaim.org/resources/contrastchecker/?fcolor=0072CE&bcolor=F0F7F6
Pacific mid-dark: https://webaim.org/resources/contrastchecker/?fcolor=0061C1&bcolor=F0F7F6
Predictably and concisely list all the card details to screen readers as they move between
cards on the results page.
Copy link
Member

@niqjohnson niqjohnson left a comment

Choose a reason for hiding this comment

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

Working great for me locally. Just had a few minor questions and possible suggestions, but nothing to hold up merging.

{%- set card_purchase_apr = apr_range(
card.purchase_apr_for_tier_min,
card.purchase_apr_for_tier_max,
asterisk=true
Copy link
Member

Choose a reason for hiding this comment

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

How is the asterisk read by a screen reader when this is used in the aria-label? If it sounds weird, I was going to suggest using an asterisk=false version in the aria-label. But if it's also read awkwardly in the card content, maybe we can update the apr_range macro to make the asterisk aria-hidden everywhere so it's never read.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great point, the asterisk does not read well either on the results page or the card detail pages. I'm thinking we should either migrate to a footnote format (e.g. [1]) that links to the explanatory text or we can try replacing the asterisks with the full explanatory text for screenreaders only. I'll experiment and open another PR.

href="{{ card.url }}"
data-js-hook="behavior_ignore-link-targets"
data-ignore-link-targets="[data-tooltip]"
aria-label="{{ card.product_name }} from {{ card.institution_name }} with {{ card_purchase_apr }} purchase APR, {{ card_account_fee }} account fee, {{ card_rewards(card) }} card rewards{%- if card.requirements_for_opening or card.issued_by_credit_union -%}, eligibility requirements{%- endif -%}"
Copy link
Member

Choose a reason for hiding this comment

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

Should we add the data published date in the description so it reads something like "9.99% purchase APR as of December 31, 2023"? That might be overkill and make for a too long summary, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was originally including the date in the aria label but it felt very repetitive and obnoxious to repeat "as of December 31, 2023" over and over again as the user tabs through the cards. I figure the card detail page has all the juicy details so it's okay to keep the card summaries concise.

@contolini contolini added this pull request to the merge queue Jul 15, 2024
Merged via the queue into main with commit c386919 Jul 15, 2024
17 checks passed
@contolini contolini deleted the tccp/a11y branch July 15, 2024 20:29
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.

2 participants