Conversation
There was a problem hiding this comment.
Pull request overview
Improves the Player’s sleep timer accessibility so VoiceOver users can hear whether the sleep timer is active and what it’s set to.
Changes:
- Plumbs a new
sleepAccessibilityLabelfromPlayerViewModelintoMediaActionRow. - Updates
MediaActionRow’s accessibility label logic to use the sleep timer’s state-aware text when applicable. - Extends
PlayerViewModel’sSleepTimerStatesubscription to publish both a visual timer label (sleepText) and a VO-specific label (sleepAccessibilityLabel).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| BookPlayer/Player/Views/PlayerView.swift | Passes the new sleep timer accessibility label into the action row. |
| BookPlayer/Player/Views/MediaActionRow.swift | Adjusts accessibility label selection for the timer action to reflect state. |
| BookPlayer/Player/ViewModels/PlayerViewModel.swift | Publishes a dedicated VO label for sleep timer state changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case .speed: | ||
| return "\(speedText) \("speed_title".localized)" | ||
| case .timer: | ||
| return sleepAccessibilityLabel ?? ma.accessibilityLabel |
There was a problem hiding this comment.
For .timer, accessibilityText returns only sleepAccessibilityLabel (e.g., “Sleeping in 5 minutes”). That can be ambiguous in a row of multiple actions because the control’s name (“Sleep Timer”) is no longer announced. Consider keeping the label as ma.accessibilityLabel and exposing the dynamic state via .accessibilityValue, or concatenating the base label + state so VoiceOver always identifies the control.
| return sleepAccessibilityLabel ?? ma.accessibilityLabel | |
| if let sleepAccessibilityLabel { | |
| return "\(ma.accessibilityLabel), \(sleepAccessibilityLabel)" | |
| } else { | |
| return ma.accessibilityLabel | |
| } |
Bugfix