a11y: label highlight color picker buttons and rendered highlights#198
Merged
Conversation
Adds VoiceOver / UI-automation accessibility metadata to two spots in
the Bible reader:
1. Highlight color picker buttons — each add-state button gets
`.accessibilityLabel("<Color> highlight")` and each remove-state
(with checkmark overlay) button gets `.accessibilityLabel("Remove
<Color> highlight")`. Reads the color live from the existing
`highlightColors` array — no palette hardcoded on our side.
2. Rendered verse text — the per-block Text carries an
`.accessibilityValue("verse N highlighted <color>")` per
highlighted verse in the block, comma-joined. Reads the color live
from `highlightFor(reference:)` on the actual server-stored
highlight — no palette hardcoded.
Color names come from UIColor(color).accessibilityName, so:
* Any RGB value the user has (palette or custom) gets a real name.
* Names are localized by iOS to the user's device language
("orange" in en, "naranja" in es, etc.).
* Zero maintenance if the palette grows.
Additive metadata only — no behavior change beyond the (correct)
VoiceOver announcements. Nothing pre-existing was refactored.
Contributor
✅ Commit Lint: passedAll commit messages in this PR conform to Conventional Commits. 📦 Release preview:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
davidfedor
requested changes
Jul 15, 2026
davidfedor
left a comment
davidfedor
left a comment
Member
There was a problem hiding this comment.
There are a lot of improvements we'd need here, to make this a good user experience for non-developers using assistive tech.
Alternatively, you could consider only adding these labels on debug builds, if you're testing on debug builds. That'd dodge most of the complexity here.
…e + macOS build Addresses PR #198 review feedback from davidfedor (CHANGES_REQUESTED) and greptile (2 P1 findings). 1. VoiceOver UX (davidfedor): the previous shape put a concatenated summary of every highlight in the block onto `.accessibilityValue`, which VoiceOver reads. A user focusing on a paragraph would hear "verse 1 highlighted green, verse 2 highlighted light vibrant yellow, verse 3 highlighted light vibrant yellow, verse 4 highlighted cyan, verse 5 highlighted cyan" before the scripture itself. That's noise for real assistive-tech users. The Bible iOS app's model (NativeReaderHost.swift:1648-1652 in the bible-ios repo) is per-verse a11y elements whose label is just "<verseNumber> <verseBody>" — highlights are visual only, never narrated inline. Following that: route the color-name string into `.accessibilityIdentifier` instead of `.accessibilityValue`. `accessibilityIdentifier` is UI-automation-only — VoiceOver never reads it, so real users hear scripture only, while Appium can still query per-verse highlight state via `@name` fallback on XCUIElementTypeStaticText. Same change on the color picker buttons in BibleReaderDrawer (`.accessibilityLabel` → `.accessibilityIdentifier`). 2. macOS build (greptile P1): UIColor is UIKit-only, unguarded in a package that declares macOS as a supported platform. Wrapped both accessibilityColorName helpers in `#if canImport(UIKit) / #elseif canImport(AppKit)` per davidfedor's suggested fix, using NSColor on macOS. Added the matching conditional imports at the top of each file. Needs macOS smoke to confirm NSColor.accessibilityName returns comparable strings — no functional dependency in the SDK code itself since the value is now identifier-only. 3. Range highlights (greptile P1): highlightFor(reference:) matched only when the stored highlight's verseStart equaled the run's verse. So a stored highlight covering v1-v5 announced only v1 and dropped v2-v5 from the identifier. Fixed to match inclusion in [verseStart, verseEnd], with `verseEnd ?? verseStart` fallback for single-verse highlights. No hardcoded palette map — accessibilityName still handles every color the SDK ever ships (palette + custom), and since the string is no longer spoken, the localized name is acceptable data for the identifier. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
# Conflicts: # Sources/YouVersionPlatformUI/Views/BibleTextView+Rendering.swift
…_newline)
The `guard let refVerse = reference.verseStart else { return nil }`
one-liner added in the merge-resolution commit tripped SwiftLint's
conditional_returns_on_newline rule. Splitting the guard body onto
its own line.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Code Coverage ReportCoverage after merging feat/highlights-a11y into main will be
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
davidfedor
approved these changes
Jul 16, 2026
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
2 participants
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.
Summary
Adds UI-automation accessibility metadata to two spots in the Bible reader (
BibleReaderDrawer.swift,BibleTextView+Rendering.swift), plus a range-highlight lookup fix. Purely additive to the accessibility surface — nothing pre-existing was refactored.Highlight state ships in
.accessibilityIdentifier, not.accessibilityLabel/.accessibilityValue. Rationale: VoiceOver readslabel/value(spoken to real users) but never readsidentifier(silent, automation-only). Putting highlight-color announcements in the spoken attributes would make VoiceOver users hear a comma-joined summary of every highlight in a block before the scripture — bad UX for assistive-tech users. The Bible iOS app's own model (NativeReaderHost.swift) treats highlights as visual affordances only, never narrated. We match that: real users hear scripture; UI automation queries the identifier via Appium's@namefallback on iOS.Color picker buttons (
BibleReaderDrawer.swift).accessibilityIdentifier("<Color> highlight").accessibilityIdentifier("Remove <Color> highlight").accessibilityLabelset — VoiceOver falls back to defaults (a follow-up ticket, YPE-4021, tracks adding localized user-facing labels once davidfedor confirms the wording).Rendered verse text (
BibleTextView+Rendering.swift)Textcarries.accessibilityIdentifier(highlightSummary(for: string))— a comma-joined"verse N highlighted <color>"per highlighted verse in the block..accessibilityLabel(the scripture text from the SwiftUIText) is unchanged — that's what VoiceOver reads.Range highlight lookup fix (
highlightFor(reference:))highlight.verseStart == reference.verseStart— a stored highlight covering v1–v5 announced only v1.[verseStart, verseEnd]inclusive.Color-name source
UIColor(color).accessibilityName(macOS:NSColor(color).accessibilityName), guarded via#if canImport(UIKit) / #elseif canImport(AppKit). Fixes macOS build regression..accessibilityIdentifier(silent, automation-only); target automation devices are English-only today. Non-English automation would need an internal hex→English mapping (deferred).Automation contract change
The corpus (
platform_swift_sdk_automation) queries these identifiers via@name. Previous locators used@label/@value; the new locators use@namebecause iOS Appium returnsaccessibilityIdentifierunder@name(falling back toaccessibilityLabelif identifier is unset). Corpus PR #22 updates the affected PageStore entries. Live-proven against BrowserStack real device: 42/43 flows pass, all 12 highlights flows green.Test plan
UIColorreference outside the#if canImport(UIKit)guard).AM88PVBRSG) verified viaxcrun devicectl device install app."button"on add-color,"highlight_checkmark button"on remove-color from the image asset name). This is intentional for this PR; localized user-facing labels are tracked in YPE-4021 as a davidfedor follow-up.XCUIElementTypeStaticTextnow surfaces the highlight summary under@name(from.accessibilityIdentifier);@valueno longer carries the summary;@labelstill carries the scripture text.XCUIElementTypeButton @name="<Color> highlight"/"Remove <Color> highlight".bs://947d661e698ae9baefa844cd68d9a822a5adccab→ 42/43 flows passed, 113/117 TCs (only 4 pre-existinguntested_formatfrom unrelated PageStore gaps; 1 flakeversion_detail_sample= BS session-init, not corpus).Related follow-ups (tracked, not blocking this PR)
.accessibilityLabelon highlight color picker + remove-color buttons (addresses davidfedor's UX concern for real VoiceOver users; requires his input on wording + newString.localizedkeys).sign_inSharedStep with internal Path A/B branching (Codex design critique on corpus PR feat(reader): save and load remaining user settings e.g. fontFamily #22)..accessibilityLabelvs.accessibilityIdentifier, "passed=True ≠ intended effect").Greptile Summary
This PR adds accessibility metadata for Bible reader highlights. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "style: split return nil to new line (Swi..." | Re-trigger Greptile
Context used: