fix: give the dashboard card and tile controls a keyboard (#1140) - #1145
Open
MOHITKOURAV01 wants to merge 1 commit into
Open
fix: give the dashboard card and tile controls a keyboard (#1140)#1145MOHITKOURAV01 wants to merge 1 commit into
MOHITKOURAV01 wants to merge 1 commit into
Conversation
…#1140) Five dashboards implemented their primary controls as bare <div onClick>. A mouse could operate them; nothing else could. No tab stop, no Enter or Space, no role -- a WCAG 2.1.1 (Keyboard) failure and a 4.1.2 (Name, Role, Value) one. In each case the clickable div was the feature: selecting a weather condition, an ocean region or a CO2 scenario, or expanding a card to reveal the health guidance behind it. Three of them even draw a rotating chevron and then wire it to a pointer only. src/components/ui/PressableCard.jsx adds DisclosureButton and SelectionButton. Both render a real <button>, which is the point: tab order, Enter and Space activation, the button role, focus ring and disabled handling come from the element rather than from hand-rolled onKeyDown handlers that would have to be got right five separate times. DisclosureButton carries aria-expanded and aria-controls -- the part a div could never provide, since it is what tells a screen reader user there is detail here at all. SelectionButton carries aria-pressed rather than role="option": these are buttons that stay pressed, not a listbox, and claiming listbox semantics would oblige arrow-key navigation and aria-activedescendant that none of these panels implement. RESET_BUTTON_STYLE strips the user-agent button styling, and callers spread their own style after it so the existing look is unchanged. Applied at all six sites. In HealthImpactDashboard the disclosure panel moves out of the button and alongside it, so opening a card no longer folds its whole body into the button's accessible name. Also fixed, because it is in the way: HealthScoreRing computed `2 * Math.PI(40)`. Math.PI is a number, so calling it threw "TypeError: Math.PI is not a function" and took HealthImpactDashboard down on render -- the keyboard fix there could not be verified on a component that cannot mount. npm run lint drops from 22 errors to 10; the 12 removed are exactly these sites. The remaining 10 are Aditya8369#1129's three parse errors, RouteForm (Aditya8369#1135), VoiceAlertManager (Aditya8369#1136), Dashboard (Aditya8369#1133) and a jest reference in a Vitest file. Tests: 15 for the shared buttons, 8 driving the three dashboards that can be mounted. All 8 fail against the code before this change.
|
@MOHITKOURAV01 is attempting to deploy a commit to the Aditya Mahajan's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thank You for Your Contribution! 🎉Hi @MOHITKOURAV01, Thank you for opening this Pull Request and contributing to our project. We truly appreciate your efforts.
The maintainer @Aditya8369 will review your PR shortly! Happy Contributing! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Five dashboards implement their primary controls as bare
<div onClick={...}>. A mouse canoperate them. Nothing else can — they are not in the tab order, Enter and Space do nothing,
and a screen reader announces them as plain text with no hint that they do anything at all.
These are not decorative elements. In each case the clickable div is the feature:
AIPollutionForecast.jsxHealthImpactDashboard.jsxHealthRiskCards.jsxOceanAcidificationMonitor.jsxOceanAcidificationMonitor.jsxReportCards.jsxThree of them even render a
ChevronDownthat rotates on expand — the affordance is drawn,and then wired to a pointer only.
WCAG 2.1 2.1.1 Keyboard (A) and, with no role, 4.1.2 Name, Role, Value (A). For an
app whose purpose is public health information, the collapsed detail is the health
guidance.
Related Issue
Closes #1140
Type of Change
Changes Made
src/components/ui/PressableCard.jsx(new) —DisclosureButtonandSelectionButton.Both render a real
<button>, and that is the point: tab order, Enter and Spaceactivation, the
buttonrole, the focus ring and disabled handling all come from theelement, rather than from hand-rolled
onKeyDownhandlers that would have to be got rightfive separate times and would still miss Space's keyup-activation semantics.
DisclosureButtoncarriesaria-expandedandaria-controls— the part a<div>couldnever provide, since
aria-expandedis what tells a screen reader user there is detailhere at all and whether it is currently open.
SelectionButtoncarriesaria-pressedrather thanrole="option". These are buttonsthat stay pressed, not a
<select>drawn as cards; claiming listbox semantics wouldoblige the group to implement arrow-key navigation and
aria-activedescendant, which isa bigger promise than any of these panels keeps. There is a test asserting no
optionor
listboxrole is claimed, so this doesn't get "upgraded" later by accident.RESET_BUTTON_STYLEstrips the user-agent button styling (background, border, padding,font, centred text). Callers spread their own style after it, so the existing look is
unchanged at every site.
The six call sites now use those. Two details worth flagging in review:
HealthImpactDashboardthe disclosure panel moves out of the button and alongside it.Wrapping the expanded body in the button would work, but it would fold the entire card's
text into the button's accessible name the moment it opened. There's a test for that.
chevron) get
aria-hidden, so the accessible name is the pollutant or the region ratherthan the name plus a read-aloud emoji.
One thing outside the a11y scope, fixed because it was in the way:
Math.PIis a number, so calling it threwTypeError: Math.PI is not a functionand tookHealthImpactDashboarddown on render. The keyboard fix there can't be verified on acomponent that can't mount, so it's fixed here (
2 * Math.PI * 40, matching the<circle r="40">the dash array is drawn onto). Happy to split it out if you'd rather.Testing
keyboardCardControls.test.jsxdrives the real dashboards rather than the shared button, soa component that merely imports
PressableCardwithout using it would still fail. All 8of those fail against the code before this change.
Lint —
npm run lintgoes from 22 errors to 10, and the 12 removed are exactlythese sites:
The 10 that remain are #1129's three parse errors,
RouteForm(#1135),VoiceAlertManager(#1136),Dashboard(#1133), and ajestreference in a Vitest file —each already tracked elsewhere.
Two things I did not take on
RouteForm.jsxhas the same class of error, but that one is Commute route form's location autocomplete is unusable by keyboard: suggestions are click-only and the ARIA is invalid #1135 and wantslistbox /
aria-activedescendantsemantics rather than a button, so it needs its ownchange.
HealthRiskCardsandReportCardsare not rendered in the tests. Both importframer-motion, which is not inpackage.jsonand is not installed, so they cannot bemounted at all in this repo today (
Failed to resolve import "framer-motion"). Tencomponents are in that state. They still get the fix here — it's a source change that makes
the controls real buttons and clears their lint errors — but I can't honestly claim a
rendered test for them. That missing dependency looks worth its own issue; say the word
and I'll open one.
Note on CI
Lint, Build and Playwright are red on
mainand on every open PR (npm run buildfails on#1129's parse errors). This branch reduces the lint error count but cannot make that job
green on its own.