Declare tappable views as accessibility buttons#614
Open
RoyalPineapple wants to merge 6 commits intomainfrom
Open
Declare tappable views as accessibility buttons#614RoyalPineapple wants to merge 6 commits intomainfrom
RoyalPineapple wants to merge 6 commits intomainfrom
Conversation
`accessibilityTraits(add:)` takes `Set<Accessibility.Trait>`, so a single trait must be passed as a set literal (`[.button]`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The wrapped element now carries the .button trait via `accessibilityTraits(add: [.button])`, which produces a CombinableView that combines the inner accessibility label. Setting `isAccessibilityElement` and `accessibilityTraits` directly on TappableView is redundant and would actually shadow the wrapped element's accessibility label. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tions These tests asserted that TappableView itself was the accessibility element with the .button trait. That behavior was removed along with the redundant view-level mutations, so the tests no longer reflect how Tappable exposes accessibility. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Verifies that .tappable produces an accessibility element exposing the .button trait and the wrapped element's label, both directly and when further wrapped in .accessibilityCombine(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The single remaining test already verifies that the wrapped Label's accessibilityLabel is picked up alongside the .button trait, which is the property accessibilityCombine would have asserted as well. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
||
| public init(onTap: @escaping () -> Void, wrapping element: Element) { | ||
| wrappedElement = element | ||
| wrappedElement = element.accessibilityTraits(add: [.button]) |
Member
There was a problem hiding this comment.
Should do be doing this conditionally? For example if the element already has a .link trait does it make sense to also apply .button?
|
|
||
| public init(onTap: @escaping () -> Void, wrapping element: Element) { | ||
| wrappedElement = element | ||
| wrappedElement = element.accessibilityTraits(add: [.button]) |
Member
There was a problem hiding this comment.
Does the onTap need to be wired up to actually be activated by voiceover?
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:
TappableViewas an accessibility element with the.buttontrait.accessibilityCombine.Verification:
xcodebuild build -scheme BlueprintUICommonControls -destination 'platform=iOS Simulator,id=EF731F66-6F9B-40A7-9F42-0C6CF3CBEFAB'