-
Notifications
You must be signed in to change notification settings - Fork 50
Add accessibilityLabel override to AttributedLabel #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6bc470d
b3d8677
4ccde7c
d0a313d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import BlueprintUI | ||
| import BlueprintUIAccessibilityCore | ||
| import XCTest | ||
| @testable import BlueprintUICommonControls | ||
|
|
||
|
|
@@ -90,6 +91,91 @@ class AttributedLabelTests: XCTestCase { | |
| } | ||
| } | ||
|
|
||
| func test_accessibilityLabel_derivedFromTextByDefault() { | ||
| // `nil` (the default) preserves the existing behavior: the backing view's accessibility | ||
| // label is derived from the displayed text. | ||
| let label = AttributedLabel(attributedText: NSAttributedString(string: "Hello, World!")) | ||
|
|
||
| let labelView = AttributedLabel.LabelView() | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
|
|
||
| XCTAssertNil(label.accessibilityLabel) | ||
| XCTAssertEqual(labelView.accessibilityLabel, "Hello, World!") | ||
| } | ||
|
|
||
| func test_accessibilityLabel_override() { | ||
| // A non-nil value overrides the text-derived label. | ||
| let label = AttributedLabel(attributedText: NSAttributedString(string: "Hello, World!")) { | ||
| $0.accessibilityLabel = "Custom" | ||
| } | ||
|
|
||
| let labelView = AttributedLabel.LabelView() | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
|
|
||
| XCTAssertEqual(labelView.accessibilityLabel, "Custom") | ||
| } | ||
|
|
||
| // Load-bearing: guards the undocumented UIKit behavior that `""`-suppression relies on. | ||
| func test_accessibilityLabel_suppressed_guardsUIKitEmptyStringBehavior() { | ||
| // An empty string suppresses the spoken label entirely: the backing view reports "", | ||
| // *not* the displayed text. (UILabel returns an explicitly-set empty string verbatim, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 (on Rob's behalf) Good that this is called out inline — the whole suppression story rests on this undocumented UIKit behavior (an explicitly-set |
||
| // rather than falling back to its text the way it does for `nil`.) If a future OS changed | ||
| // this fallback, `""`-suppression would silently break — hence this test. | ||
| let label = AttributedLabel(attributedText: NSAttributedString(string: "Hello, World!")) { | ||
| $0.accessibilityLabel = AttributedLabel.suppressedAccessibilityLabel | ||
| } | ||
|
|
||
| let labelView = AttributedLabel.LabelView() | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
|
|
||
| XCTAssertEqual(labelView.accessibilityLabel, "") | ||
| } | ||
|
|
||
| func test_accessibilityLabel_appliesWithoutTextChange() { | ||
| // Changing only the override (with the text unchanged) must still update the backing view's | ||
| // label. This proves the override is applied outside the text-change guard. | ||
| let string = NSAttributedString(string: "Hello, World!") | ||
| let labelView = AttributedLabel.LabelView() | ||
|
|
||
| var label = AttributedLabel(attributedText: string) | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
| XCTAssertEqual(labelView.accessibilityLabel, "Hello, World!") | ||
|
|
||
| label.accessibilityLabel = "Custom" | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
| XCTAssertEqual(labelView.accessibilityLabel, "Custom") | ||
|
|
||
| label.accessibilityLabel = AttributedLabel.suppressedAccessibilityLabel | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
| XCTAssertEqual(labelView.accessibilityLabel, "") | ||
|
|
||
| // Back to `nil` falls through to the cached derived label, not UILabel's text fallback. | ||
| label.accessibilityLabel = nil | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
| XCTAssertEqual(labelView.accessibilityLabel, "Hello, World!") | ||
| } | ||
|
|
||
| func test_accessibilityLabel_suppressionInComposition() { | ||
| // A suppressed ("") label paired with a value contributes only to the combined *value* when | ||
| // merged into a composite accessibility element — the empty label is filtered out, so the | ||
| // content isn't announced twice. | ||
| let label = AttributedLabel(attributedText: NSAttributedString(string: "Displayed text")) { | ||
| $0.accessibilityLabel = AttributedLabel.suppressedAccessibilityLabel | ||
| $0.accessibilityValue = "X" | ||
| } | ||
|
|
||
| let labelView = AttributedLabel.LabelView() | ||
| labelView.update(model: label, text: label.displayableAttributedText, environment: .empty, isMeasuring: false) | ||
|
|
||
| XCTAssertEqual(labelView.accessibilityLabel, "") | ||
| XCTAssertEqual(labelView.accessibilityValue, "X") | ||
|
|
||
| let representation = AccessibilityComposition.CompositeRepresentation([labelView]) {} | ||
|
|
||
| XCTAssertNil(representation.label) | ||
| XCTAssertEqual(representation.value, "X") | ||
| } | ||
|
|
||
| func test_displaysText() { | ||
| let string = NSAttributedString() | ||
| .appending(string: "H", font: .boldSystemFont(ofSize: 24.0), color: .red) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 (on Rob's behalf) The
String?shape is the right call here — it's consistent with the siblingaccessibilityValue/accessibilityHinton this model, withAccessibilityElement.label, and with UIKit's ownaccessibilityLabel(nil= derive). And empty-string-means-absent is already the house convention:AccessibilityCombinedocuments that an element isn't accessible "unless it has a non-nil and non-empty label/value/hint," which is exactly why the composition test works. So the""-suppresses behavior isn't a novel magic value — it fits existing precedent. I'd keep it over an enum, which would make this the only accessibility prop that isn't a plain optional string and would lose thex.accessibilityLabel = someOptionalStringdrop-in ergonomics.One small discoverability nit:
""-as-suppression isn't legible at the call site, and a future reader could "fix" a bare""thinking it's a mistake. Consider a named constant so intent is greppable:Non-blocking — the doc comment already covers it well.