chore: add accessibility guidelines to AGENTS.md - WPB-26264#4830
chore: add accessibility guidelines to AGENTS.md - WPB-26264#4830David-Henner wants to merge 2 commits into
Conversation
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit b4c1d43. ♻️ This comment has been updated with latest results. Summary: workflow run #27215479855 |
| - New UI elements require `accessibilityLabel` for VoiceOver / `accessibilityIdentifier` for UI tests | ||
| - Push notifications only work with the App Store-signed build (Apple security restriction) | ||
|
|
||
| ## Accessibility |
There was a problem hiding this comment.
thought: I wonder if this section can be pulled into context on demand so we don't stuff too much info in the context. Not sure how big should be this file
There was a problem hiding this comment.
pasting Claude's opinion on this
With a 1M context window, AGENTS.md is essentially free — even at 100× its current size (600 KB) it would only consume ~150K tokens. The context window is not the binding constraint here.
The practical limits are different:
Human readability — AGENTS.md is read by engineers, not just the AI. Once it exceeds ~300–400 lines it becomes hard to scan and maintain. That's probably the real ceiling (~15–20 KB).
Signal dilution — the longer it gets, the more the AI has to weight competing instructions. Dense, specific rules work better than long explanations. The accessibility section we just added is already on the edge of this — it's detailed because the topic is complex, but adding many more sections like it would start to dilute the more critical conventions at the top.
What to watch for — if you find yourself adding large "how-to" or tutorial content to AGENTS.md, that's a sign it belongs in a separate skill or referenced doc instead. AGENTS.md should stay as a checklist/ruleset, not a guide.
At the current growth rate, you have plenty of room — but keep sections tight.
| - Back buttons must describe the destination (e.g. "Go back to Settings", not just "Back") | ||
|
|
||
| ### Dynamic content | ||
| - When content changes dynamically (list updates, search results, state transitions), post a UIAccessibility notification: |
There was a problem hiding this comment.
suggestion: I would advise caution with these examples. I understand that these were generated by AI using our code, but UIAccessibility notifications are very legacy code, and we've never properly tested them. We should also pay attention to AI-generated suggestions.
We don't have a process for testing accessibility features, and placing UIAccessibility notifications everywhere could reduce the usability of the app with VoiceOver. We currently have a working scenario that announces a new message if the user is in conversation view.
There was a problem hiding this comment.
I did use AI to generate this but I've tried to be as close to our guidelines as possible by leaning heavily on this document https://wearezeta.atlassian.net/wiki/spaces/IOS/pages/654475313/Development+guide.
If you're saying UIAccessibility notifications aren't reliable, I'll happily remove this section
| - When a container element expresses the full meaning, hide its children and set the label/trait/hint on the container (use `.accessibilityElement(children: .ignore)` in SwiftUI) | ||
|
|
||
| ### Navigation bar | ||
| - All navigation bar buttons must have alternative text |
There was a problem hiding this comment.
suggestion: I wouldn't use "All navigation bar buttons", because some of them as "Next", "Start", etc. maybe don't need it. When defining them, we should always collaborate with the product team.
| - Combine traits when needed (e.g. `.button` + `.selected` for a selected toggleable button) | ||
|
|
||
| ### Hiding elements | ||
| - Hide purely decorative elements with `accessibilityHidden(true)` / `isAccessibilityElement = false` |
There was a problem hiding this comment.
suggestion: "purely decorative elements" is a very subjective definition. Perhaps examples or an explanation of how this phrase might be used in a real-life situation are needed.
| ### Dynamic type | ||
| - UI elements that display text must use the API that supports dynamic type / large fonts | ||
| - Push notifications only work with the App Store-signed build (Apple security restriction) No newline at end of file | ||
| - Test with the largest accessibility font size (Settings → Accessibility → Larger Text) No newline at end of file |
There was a problem hiding this comment.
suggestion: We also have snapshot tests for dynamic text. You can mention them here.
| - Set `accessibilityHint` to describe the result of an action (e.g. "Double tap to open profile"); omit it when the label already makes the outcome obvious | ||
| - Set `accessibilityValue` for elements with dynamic state (e.g. a text field's current content) | ||
| - VoiceOver reads in order: **label → value → trait → hint** | ||
| - Put all VoiceOver strings in `Accessibility.strings`, not `Localizable.strings` |
There was a problem hiding this comment.
suggestion: The Accessibility.strings files also have a naming structure, you can specify it here.
| Every new screen or UI element must pass the following checklist before merging: | ||
|
|
||
| ### Identifiers and labels | ||
| - Set `accessibilityIdentifier` on every new element — **never remove existing ones** (QA automation depends on them) |
There was a problem hiding this comment.
question: Do we have a separate file for accessibilityIdentifier? If so, we can add it here.
There was a problem hiding this comment.
not sure what you mean
There was a problem hiding this comment.
WireUI -> ... -> Locators
I think @findms created this file for accessibilityIdentifier which are used for automation tests.
Co-authored-by: François Benaiteau <netbe@users.noreply.github.com> Co-authored-by: David-Henner <david.henner@wire.com>
|
3 participants
This PR expands
AGENTS.mdwith a dedicated Accessibility section, consolidating the checklist from the internal development guide so it is discoverable alongside the rest of the project conventions.Testing
Review the
AGENTS.mddiff — no functional code changes.Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: