wet-boew / GCWeb

Canada.ca theme - A reference implementation of the Canada.ca Content and Information Architecture Specification, the Canada.ca Content Style Guide and the Canada.ca Design System
https://wet-boew.github.io/GCWeb/
Other
91 stars 130 forks source link

Page feedback tool: Polish AJAX fragments (FR pageData, role="status") #2401

Open EricDunsworth opened 2 months ago

EricDunsworth commented 2 months ago

The AJAX fragments had two minor flaws:

This resolves the flaws by:

Garneauma commented 2 months ago

Pre-approved upon successful code review and accessibility testing. This will need to be coordinated with an AEM release.

EricDunsworth commented 2 months ago

@Garneauma Here's my in-depth rationale for the aria-live="polite" to role="status" change:

EricDunsworth commented 2 months ago

Two more points:

duboisp commented 1 month ago

To remove the confusion, we would need to do and document actual test with AT that illustrate the issue and that are illustrating the solution are fixing the issue. Those test can be documented into a partial accessibility assessment.

EricDunsworth commented 1 month ago

To remove the confusion, we would need to do and document actual test with AT that illustrate the issue and that are illustrating the solution are fixing the issue. Those test can be documented into a partial accessibility assessment.

@duboisp Just to clarify, my goal with adding role="status" wasn't to fix a specific screen reader issue.

My initial intent with this PR was to fix the French pageData references, but while prepping it I randomly noticed that ARIA semantics were being used inconsistently on what I'd consider to be two status messages. So I decided to make them consistent by using role="status" on both (which appears to be the most semantically-correct way of denoting status messages).

I only have NVDA at my disposal, but from what I tested while prepping the PR, both aria-live="polite" and role="status" functioned identically in practice (which makes sense given that role="status" implicitly sets aria-live="polite" and aria-atomic="true").

I don't think there'd be much value in attempting to document this change's impact on screen readers. Both the "before" and "after" behaviour is identical. Not to mention that role="status" is already being used in one of the status messages without any issues. This situation doesn't involve quirky AT behaviour, so there isn't really anything worth documenting.

In any case, if you feel strongly about this and would prefer to keep the status quo, I could revert the role="status" change and focus solely on the French pageData fixes.

Thoughts?