[Simple Analytics] Accessibility#214
Open
alaca wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
the
needs:feedback
|
@alaca thanks for the PR! Could you please fill out the PR template with description, changelog, and credits information so that we can properly review and merge this? |
There was a problem hiding this comment.
Pull request overview
Adds accessibility improvements to the Simple Analytics admin page, focusing on keyboard interaction, chart alternatives, reduced motion, and contrast.
Changes:
- Updates date range controls with ARIA labeling, dialog semantics, and Escape handling.
- Adds reduced-motion Chart.js configuration and screen-reader chart data tables.
- Adjusts focus indicators, chart colors, and muted-state contrast.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
includes/admin/templates/analytics.php |
Adds ARIA attributes and hidden data-table containers for analytics charts. |
assets/js/analytics.js |
Adds reduced-motion handling, Escape popover behavior, chart color updates, and dynamic data-table rendering. |
assets/css/analytics.scss |
Adds screen-reader utility styles, updated focus-visible rings, contrast tweaks, and reduced-motion CSS. |
Comments suppressed due to low confidence (4)
assets/js/analytics.js:1353
- The screen-reader table is only populated on the ready path and is never cleared by the loading, empty, or error paths. After a successful load, changing filters to an empty range or failed request leaves stale rows in the element referenced by aria-describedby, so assistive technology can still receive the previous chart data.
renderDataTable(rows, fromLabel, toLabel);
assets/js/analytics.js:531
- The new screen-reader table rendering is not covered by the existing analytics Cypress success-state tests. Those tests already stub chart payloads, so they should assert that the aria-describedby target contains a table with the expected row data after a successful response.
function renderDataTable(rows, fromLabel, toLabel) {
assets/js/analytics.js:1044
- The new screen-reader table rendering is not covered by the existing analytics Cypress success-state tests. Those tests already stub chart payloads, so they should assert that the aria-describedby target contains a table with the expected row data after a successful response.
function renderDataTable(rows, fromLabel, toLabel) {
includes/admin/templates/analytics.php:380
- The chart still exposes tooltip details only through Chart.js pointer interactions: the canvas is not focusable and there are no keyboard handlers for moving between data points. The hidden table helps screen-reader users, but sighted keyboard users still cannot access the per-bucket tooltip values required for keyboard-accessible chart tooltips.
class="mailchimp-sf-sa__canvas"
role="img"
aria-label="<?php esc_attr_e( 'Subscriber change bar chart', 'mailchimp' ); ?>"
aria-describedby="mailchimp-sf-sa-data-table"
></canvas>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setOverlay(''); | ||
| setState('ready'); | ||
| renderChart(rows); | ||
| renderDataTable(rows, fromLabel, toLabel); |
Comment on lines
+388
to
+390
| <div | ||
| id="mailchimp-sf-sa-data-table" | ||
| class="mailchimp-sf-sa__data-table screen-reader-text" |
Comment on lines
+45
to
+48
| .mailchimp_page_mailchimp_sf_analytics .mailchimp-sf-button:focus-visible, | ||
| .mailchimp_page_mailchimp_sf_analytics button:focus-visible { | ||
| box-shadow: 0 0 0 2px var(--mailchimp-color-link, #017e89); | ||
| outline: none; |
Comment on lines
279
to
283
| class="mailchimp-sf-fp__canvas" | ||
| role="img" | ||
| aria-label="<?php esc_attr_e( 'Form views, submissions, and conversion rate chart', 'mailchimp' ); ?>" | ||
| aria-describedby="mailchimp-sf-fp-data-table" | ||
| ></canvas> |
Collaborator
There was a problem hiding this comment.
@alaca Could you please check if this is feasible?
Comment on lines
+445
to
+448
| closePopover(); | ||
| if (trigger) { | ||
| trigger.focus(); | ||
| } |
iamdharmesh
reviewed
May 14, 2026
Collaborator
iamdharmesh
left a comment
iamdharmesh
left a comment
There was a problem hiding this comment.
Thanks @alaca. Over all this looks good. Could you please check on the feedback from copilot, specifically for keyboard-navigable tooltips? Thank you.
| // while keeping it readable by assistive tech. Defined locally so the | ||
| // styles still apply if WP core's stylesheet load order changes. | ||
| // ----------------------------------------------------------------------------- | ||
| .screen-reader-text { |
Collaborator
There was a problem hiding this comment.
Can we scope this to make sure it only apply to screen reader text added by us?
Something like .mailchimp_page_mailchimp_sf_analytics .screen-reader-text
Comment on lines
279
to
283
| class="mailchimp-sf-fp__canvas" | ||
| role="img" | ||
| aria-label="<?php esc_attr_e( 'Form views, submissions, and conversion rate chart', 'mailchimp' ); ?>" | ||
| aria-describedby="mailchimp-sf-fp-data-table" | ||
| ></canvas> |
Collaborator
There was a problem hiding this comment.
@alaca Could you please check if this is feasible?
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 of the Change
Closes #208
How to test the Change
Run Lighthouse Accessibility audit in DevTools. Expect score 100.
Tab through the page. Every control shows a 2px teal ring.
Open date popover. Press Esc. Popover closes, focus returns to trigger.
DevTools Rendering panel, toggle prefers-reduced-motion: reduce. Charts snap, no animation, skeleton pulse frozen.
DevTools Elements, find #mailchimp-sf-fp-data-table and #mailchimp-sf-sa-data-table. Each contains a real table with chart data.
Changelog Entry
Credits
Props @alaca
Checklist: