Skip to content

2176 fixes merged into main_accessibility - #2180

Open
smutniak wants to merge 2 commits into
main_accessibilityfrom
main_2176
Open

smutniak wants to merge 2 commits into
main_accessibilityfrom
main_2176

Conversation

@smutniak

Copy link
Copy Markdown
Contributor

Description

add aria-label for spans using glyphicon-pencil
add tab-index="0" and role="button" when needed
decorative icons get aria-hidden="true"

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • [ x] Accessibility

AI Usage Disclosure

AI Tool(s) Used Including Version:

claude-sonnet-5-5
Nature of Use:

  • [ x] Code generation
  • Code suggestions/refactoring
  • Documentation
  • Tests
  • Other (please describe)

Additional Description:

"read the ticket #2176 and help me identify all pecild edit icons that require alt text"

manually used git diff to study all the changes before commit

still learning my way through using claude and testing tools to verify the results so this is going to be merged with my branch 'main_accessibility' for now

  • No AI Used

@smutniak

Copy link
Copy Markdown
Contributor Author

Following the above, I did /clear and then changed /model from Sonnet 5 to Opus 5 and then asked:

starting with https://github.com/TexasDigitalLibrary/Vireo/tree/main_accessibility the branch https://github.com/TexasDigitalLibrary/Vireo/tree/main_2176 was created using a different model. Confirm that the changes in main_2176 fulfil the ticket #2176

the Opus 5 response:
Verdict: partially fulfills the ticket

Confirmed the branch is a single commit (03aaf6d "2176") on top of main_accessibility, touching 20 HTML templates. main_accessibility is currently identical to main, so the diff is the whole change.

What it gets right

Coverage is complete for the literal ask (WCAG 2.2 SC 1.1.1, non-text content). I grepped the branch for glyphicon-pencil: all 24 occurrences in src/main/webapp are now handled, with no stragglers. Two correct patterns are used:

  • Decorative icons → aria-hidden="true" where adjacent text already carries the meaning: lockingTextArea.html:10, submissionStatus.html:6,23, and controlledVocabularyManagement.html:127 (which also correctly moves the name onto the via aria-label). This is the right call — a pencil next to "Assignee: Jane Doe" shouldn't be announced twice.
  • Labelled interactive icons elsewhere, with contextual names ("Edit reviewer notes", "Edit file {{…}}").

I verified the interpolations actually resolve: properties and item are in scope in the drag-and-drop item templates (dragAndDropListDirective.js:43 sets $scope.properties), properties[0] is the display name in every caller I checked (["name"], ["label", …], ["gloss"], ["value"], ["month"]), to-string='toMonthString(month)' is genuinely passed in graduationMonths.html:20, and fieldProfile is an isolate-scope binding in submissionInfoDirective.js:10. No broken labels.

What's wrong

  1. role="button" tabindex="0" with no keyboard activation (13 spans). Every span that got role="button" is now focusable but cannot be activated — no ng-keydown/ng-keypress was added, and there is no global delegate in the app that binds Enter/Space to role=button (I grepped the directives and views). ARIA requires a role=button to respond to Enter and Space; as written, a keyboard user tabs onto something announced as a button and nothing happens. That's a new SC 2.1.1 failure introduced while fixing 1.1.1.

  2. Those same spans are ng-show="isMouseHover" (13 of the 13). They render display:none until the mouse enters the row, so tabindex="0" is inert and the new aria-label is never exposed to AT at all. No :focus-within rule or ng-focus was added (.edit-setting-icon in app.scss:671 only sets cursor). The drag-and-drop edit pencils remain mouse-only, so the labels only help a sighted mouse user with a screen reader running.

  3. emailWorkflowRule.html:24 — the span is marked role="button" but the ng-click is on the parent . Mouse clicks work by bubbling; there is no handler on the span to attach a key handler to, so fixing Test Issue #1 here requires moving or duplicating the action.

  4. dragAndDropCustomAction.html:11 — this span has no ng-click at all, unlike every sibling template (which call edit({index:…})). The pencil in Custom Actions is already non-functional for mouse users too (edit='editCustomAction(index)' is passed in at customActionsChecklist.html:22 but never invoked). Giving it role="button" advertises a control that does nothing. It should be aria-hidden="true" — or the underlying missing ng-click should be filed separately.

  5. Inconsistency. controlledVocabularyManagement.html uses the correct pattern (real , aria-label on it, icon hidden); everywhere else keeps the bare span. Where a real button is feasible, that pattern is strictly better and removes problems Test Issue #1 and Investigate Library/Method Tighter Validation for Phone/Address/Name (Proquest) #2 for free.

Recommendation

The change is safe to merge as a strict improvement over the current state — nothing it does is worse than an unlabelled icon, and the 1.1.1 gap the ticket names is closed. But I would not call #2176 done without:

  • adding keyboard activation to the 13 role="button" spans (ng-keydown="($event.which===13||$event.which===32) && edit({index:item.position})" plus $event.preventDefault() for Space), or converting them to ;
  • making the hover-gated pencils reachable (ng-show="isMouseHover || hasFocus" with ng-focus/ng-blur, or a CSS :focus-within rule) — otherwise the labels are unreachable for the exact users the ticket is about;
  • dropping role="button" from dragAndDropCustomAction.html:11;
  • resolving emailWorkflowRule.html:24's handler placement.

The alt text added for the pencil icons marked them as role="button" and
gave them a tab stop, but nothing activated them from the keyboard and
most were hidden in a way that kept them out of the tab order entirely.

Add a keyboardActivate directive that activates role="button" elements on
enter and space by dispatching a click, which also reaches the handler
when it sits on an ancestor, as it does for the email workflow rules.

Reveal the hover-only icons on focus as well as hover. They were hidden
with display: none and visibility: hidden, neither of which leaves an
element focusable, so hide them with opacity instead.

The pencil in the custom action list sits outside the element carrying
the click handler, so mark it decorative and label that element instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant