-
Notifications
You must be signed in to change notification settings - Fork 114
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
Changes from all commits
a546f39
d2ff3b5
3be7f44
4fda157
d930463
359fdca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,9 +51,23 @@ | |
<div class="o-card-group"> | ||
<div class="o-card-group__cards"> | ||
{%- for card in cards %} | ||
{%- set card_purchase_apr = apr_range( | ||
card.purchase_apr_for_tier_min, | ||
card.purchase_apr_for_tier_max, | ||
asterisk=true | ||
) -%} | ||
{%- set card_account_fee = currency( | ||
card.annual_fee_estimated, | ||
default=('See details' if card.annual_fee_estimated is none and card.periodic_fee_type else '$0') | ||
) -%} | ||
{%- set show_more = loop.index > 11 and ordering_by != 'product_name' -%} | ||
<article class="m-card m-card--tabular{% if show_more %} u-show-more{% endif %}"> | ||
<a href="{{ card.url }}" data-js-hook="behavior_ignore-link-targets" data-ignore-link-targets="[data-tooltip]"> | ||
<a | ||
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 -%}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
> | ||
<div class="m-card__heading-group"> | ||
<h2 class="h3 m-card__heading">{{ card.institution_name }}</h2> | ||
<p class="m-card__subtitle">{{ card.product_name }}</p> | ||
|
@@ -95,17 +109,13 @@ <h2 class="h3 m-card__heading">{{ card.institution_name }}</h2> | |
</div> | ||
</dt> | ||
<dd> | ||
<strong>{{ apr_range( | ||
card.purchase_apr_for_tier_min, | ||
card.purchase_apr_for_tier_max, | ||
asterisk=true | ||
) }}</strong> | ||
<strong>{{ card_purchase_apr }}</strong> | ||
</dd> | ||
</div> | ||
<div class="m-data-spec m-data-spec--fee"> | ||
<dt><strong>Account fee</strong></dt> | ||
<dd> | ||
<strong>{{ currency(card.annual_fee_estimated, default=('See details' if card.annual_fee_estimated is none and card.periodic_fee_type else '$0')) }}</strong> | ||
<strong>{{ card_account_fee }}</strong> | ||
</dd> | ||
</div> | ||
<div class="m-data-spec m-data-spec--transfer"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1478,6 +1478,11 @@ aws4@^1.8.0: | |
resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.12.0.tgz#ce1c9d143389679e253b314241ea9aa5cec980d3" | ||
integrity sha512-NmWvPnx0F1SfrQbYwOi7OeaNGokp9XhzNioJ/CSBs8Qa4vxug81mhJEAVZwxXuBmYB5KDRfMq/F3RR0BIU7sWg== | ||
|
||
[email protected]: | ||
version "4.9.1" | ||
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-4.9.1.tgz#fcd0f4496dad09e0c899b44f6c4bb7848da912ae" | ||
integrity sha512-QbUdXJVTpvUTHU7871ppZkdOLBeGUKBQWHkHrvN2V9IQWGMt61zf3B45BtzjxEJzYuj0JBjBZP/hmYS/R9pmAw== | ||
|
||
axe-core@=4.7.0: | ||
version "4.7.0" | ||
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-4.7.0.tgz#34ba5a48a8b564f67e103f0aa5768d76e15bbbbf" | ||
|
@@ -1952,6 +1957,11 @@ cssstyle@^2.3.0: | |
dependencies: | ||
cssom "~0.3.6" | ||
|
||
[email protected]: | ||
version "1.5.0" | ||
resolved "https://registry.yarnpkg.com/cypress-axe/-/cypress-axe-1.5.0.tgz#95082734583da77b51ce9b7784e14a442016c7a1" | ||
integrity sha512-Hy/owCjfj+25KMsecvDgo4fC/781ccL+e8p+UUYoadGVM2ogZF9XIKbiM6KI8Y3cEaSreymdD6ZzccbI2bY0lQ== | ||
|
||
[email protected]: | ||
version "7.1.0" | ||
resolved "https://registry.yarnpkg.com/cypress-fail-fast/-/cypress-fail-fast-7.1.0.tgz#36e28e39fffaacf852b4866c8459e5274eb8326d" | ||
|
There was a problem hiding this comment.
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 anasterisk=false
version in thearia-label
. But if it's also read awkwardly in the card content, maybe we can update theapr_range
macro to make the asteriskaria-hidden
everywhere so it's never read.There was a problem hiding this comment.
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.