Skip to content
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

AttributedLabel Accessibility Links #537

Merged
merged 8 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import UIKit

extension BidirectionalCollection where Element: Equatable & NSObjectProtocol {

/// Returns a UIAccessibilityCustomRotor with the provided system type which cycles through the contained elements.
public func accessibilityRotor(systemType type: UIAccessibilityCustomRotor.SystemRotorType) -> UIAccessibilityCustomRotor {
UIAccessibilityCustomRotor(systemType: type) { itemSearch($0) }
}

private func itemSearch(_ predicate: UIAccessibilityCustomRotorSearchPredicate) -> UIAccessibilityCustomRotorItemResult? {
guard let first else { return nil }
guard let currentItem = predicate.currentItem.targetElement as? Element,
Copy link
Collaborator

@kyleve kyleve Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking outloud, im surprised to see both the generic constraint and this dynamic check; are both required?

Copy link
Collaborator Author

@RoyalPineapple RoyalPineapple Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic constraint is on the array contents, but the dynamic check is being run on the item that we've been passed in via the predicate, which is only guaranteed to be an NSObjectProtocol not necessarily Equatable.

We could get away with just checking for Equatable, but I thought it would be nice to ensure that all the types match by using Element instead.

let currentIndex = firstIndex(of: currentItem),
predicate.searchDirection == .previous || predicate.searchDirection == .next
else {
return UIAccessibilityCustomRotorItemResult(targetElement: first, targetRange: nil)
}
let newIndex = (predicate.searchDirection == .next ? index(after: currentIndex) : index(before: currentIndex))
guard newIndex >= startIndex, newIndex < endIndex else { return nil }
return .init(targetElement: self[newIndex], targetRange: nil)
}
}
49 changes: 11 additions & 38 deletions BlueprintUICommonControls/Sources/AttributedLabel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -210,35 +210,11 @@ extension AttributedLabel {
}
}

/// The view needs to keep track of the current index for the accessibility rotor.
private var accessibilityLinkIndex = -1

/// These elements need to be retained by the view, and cannot be created inside the
/// `accessibilityCustomRotors` getter.
Copy link
Collaborator Author

@RoyalPineapple RoyalPineapple Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This concern about the elements being retained is still valid. We cannot pass back unretained elements created within the itemSearchBlock.

The trick here is the elements are now retained in an array which is itself captured by the itemSearchBlock.

Once VoiceOver releases the rotor these elements all get deallocated and then will be recreated the next time VoiceOver requests the rotor.

private var accessibilityLinks: [LinkAccessibilityElement] = []

override var accessibilityCustomRotors: [UIAccessibilityCustomRotor]? {
set { fatalError() }
set { fatalError("accessibilityCustomRotors is not settable.") }
get {
accessibilityLinks.isEmpty
? []
: [
UIAccessibilityCustomRotor(systemType: .link) { [weak self] predicate in
guard let self = self, !self.accessibilityLinks.isEmpty else {
return nil
}

self.accessibilityLinkIndex += predicate.searchDirection == .next ? 1 : -1
self.accessibilityLinkIndex = min(
self.accessibilityLinks.count - 1,
self.accessibilityLinkIndex
)
self.accessibilityLinkIndex = max(0, self.accessibilityLinkIndex)

let link = self.accessibilityLinks[self.accessibilityLinkIndex]
return UIAccessibilityCustomRotorItemResult(targetElement: link, targetRange: nil)
},
]
guard let attributedText, !links.isEmpty else { return [] }
return [accessibilityRotor(for: links, in: attributedText)]
}
}

Expand Down Expand Up @@ -275,7 +251,6 @@ extension AttributedLabel {
if !isMeasuring {
if previousAttributedText != attributedText {
links = attributedLinks(in: model.attributedText) + detectedDataLinks(in: model.attributedText)
accessibilityLinks = accessibilityLinks(for: links, in: model.attributedText)
accessibilityLabel = accessibilityLabel(
with: links,
in: model.attributedText.string,
Expand Down Expand Up @@ -389,7 +364,7 @@ extension AttributedLabel {
}

assert(
alignments.count == 1,
Copy link
Collaborator Author

@RoyalPineapple RoyalPineapple Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing a crash in the demo app.
Someone went through the trouble of writing the nil case below, it'd be a shame never to use it.

alignments.count <= 1,
"""
AttributedLabel links only support a single NSTextAlignment. \
Instead, found: \(alignments).
Expand Down Expand Up @@ -539,30 +514,28 @@ extension AttributedLabel {
return links
}

private func accessibilityLinks(for links: [Link], in string: NSAttributedString) -> [LinkAccessibilityElement] {
links
internal func accessibilityRotor(for links: [Link], in string: NSAttributedString) -> UIAccessibilityCustomRotor {
let elements: [LinkAccessibilityElement] = links
.sorted(by: { $0.range.location < $1.range.location })
.compactMap { link in
guard NSIntersectionRange(string.entireRange, link.range).length > 0 else {
return nil
}

return LinkAccessibilityElement(
container: self,
label: string.attributedSubstring(from: link.range).string,
link: link
)
}


return elements.accessibilityRotor(systemType: .link)
}

internal func accessibilityLabel(with links: [Link], in string: String, linkAccessibilityLabel: String?) -> String {
// When reading an attributed string that contains the `.link` attribute VoiceOver will announce "link" when it encounters the applied range. This is important because it informs the user about the context and position of the linked text within the greater string. This can be partocularly important when a string contains multiple links with the same linked text but different link destinations.
// When reading an attributed string that contains the `.link` attribute VoiceOver will announce "link" when it encounters the applied range. This is important because it informs the user about the context and position of the linked text within the greater string. This can be particularly important when a string contains multiple links with the same linked text but different link destinations.

// UILabel is extremely insistant about how the `.link` attribute should be styled going so far as to apply its own preferences above any other provided attributes. In order to allow custom link styling we replace any instances of the `.link` attribute with a `labelLink.` attribute (see `NSAttributedString.normalizingForView(with:)`. This allows us to track the location of links while still providing our own custom styling. Unfortunately this means that voiceover doesnt recognize our links as links and consequently they are not announced to the user.
// UILabel is extremely insistent about how the `.link` attribute should be styled going so far as to apply its own preferences above any other provided attributes. In order to allow custom link styling we replace any instances of the `.link` attribute with a `labelLink.` attribute (see `NSAttributedString.normalizingForView(with:)`. This allows us to track the location of links while still providing our own custom styling. Unfortunately this means that voiceover doesn't recognize our links as links and consequently they are not announced to the user.

// Ideally we'd be able to enumerate our links, insert the `.link` attribute back and then set the resulting string as the `accessibilityAttributedString` but unfortunately that doesnt seem to work. Apple's [docs](https://developer.apple.com/documentation/objectivec/nsobject/2865944-accessibilityattributedlabel) indicate that this property is intended "for the inclusion of language attributes in the string to control pronunciation or accents" and doesnt seem to notice any included `.link` attributes.
// Ideally we'd be able to enumerate our links, insert the `.link` attribute back and then set the resulting string as the `accessibilityAttributedString` but unfortunately that doesn't seem to work. Apple's [docs](https://developer.apple.com/documentation/objectivec/nsobject/2865944-accessibilityattributedlabel) indicate that this property is intended "for the inclusion of language attributes in the string to control pronunciation or accents" and doesnt seem to notice any included `.link` attributes.

// Insert the word "link" after each link in the label. This mirrors the VoiceOver behavior when encountering a `.link` attribute.

Expand Down Expand Up @@ -708,7 +681,7 @@ extension AttributedLabel {
}

override var accessibilityFrameInContainerSpace: CGRect {
set { fatalError() }
set { fatalError("accessibilityFrameInContainerSpace") }
get {
guard let container = container,
let textStorage = container.makeTextStorage(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,66 @@ class AttributedLabelTests: XCTestCase {
}


func test_linkAccessibility_Rotors() {
let labelView = AttributedLabel.LabelView()
let text: NSString = "The Fellowship of the ring was established at the Council of Elrond and consisted of Gandalf, Sam, Frodo, Aragorn, Gimli, Pippin, Boromir, Legolas, and Merry."

let url = URL(string: "https://one.ring")!

let links = ["Frodo", "Merry", "Sam", "Pippin"].map {
AttributedLabel.Link(url: url, range: text.range(of: $0))
}

let rotor = labelView.accessibilityRotor(for: links, in: NSAttributedString(string: text as String))
XCTAssertNotNil(rotor)

// links should be sorted by their position in the main string.
let sortedHobbits = rotor.dumpItems().map { $0.accessibilityLabel }
XCTAssertEqual(sortedHobbits, ["Sam", "Frodo", "Pippin", "Merry"])
}



func test_linkAccessibility_Rotors_update() {
let string = "The Fellowship of the ring was established at the Council of Elrond and consisted of Gandalf, Sam, Frodo, Aragorn, Gimli, Pippin, Boromir, Legolas, and Merry."
var attributedText = AttributedText(string)

for hobbit in ["Frodo", "Merry", "Sam", "Pippin"] {
let range = attributedText.range(of: hobbit)!
attributedText[range].link = URL(string: "https://one.ring")!
}

var label = AttributedLabel(attributedText: attributedText.attributedString)
let labelView = AttributedLabel.LabelView()
labelView.update(model: label, text: label.attributedText, environment: .empty, isMeasuring: false)

let rotor = labelView.accessibilityCustomRotors!.first!
XCTAssertNotNil(rotor)

// links should be sorted by their position in the main string.
let sortedHobbits = rotor.dumpItems().map { $0.accessibilityLabel }
XCTAssertEqual(sortedHobbits, ["Sam", "Frodo", "Pippin", "Merry"])

let removedLinks = AttributedText(string)
label.attributedText = removedLinks.attributedString
labelView.update(model: label, text: label.attributedText, environment: .empty, isMeasuring: false)
XCTAssertTrue(labelView.accessibilityCustomRotors!.isEmpty)

var updatedText = AttributedText(string)
for name in ["Aragorn", "Gandalf", "Gimli", "Legolas", "Boromir"] {
let range = updatedText.range(of: name)!
updatedText[range].link = URL(string: "https://one.ring")!
}
label.attributedText = updatedText.attributedString
labelView.update(model: label, text: label.attributedText, environment: .empty, isMeasuring: false)

let updatedRotor = labelView.accessibilityCustomRotors!.first!
XCTAssertNotNil(updatedRotor)

let notHobbits = updatedRotor.dumpItems().map { $0.accessibilityLabel }
XCTAssertEqual(notHobbits, ["Gandalf", "Aragorn", "Gimli", "Boromir", "Legolas"])
}


func test_textContainerRects() {
let lineBreakModes: [NSLineBreakMode?] = [
Expand Down Expand Up @@ -654,6 +714,44 @@ class AttributedLabelTests: XCTestCase {

}

extension UIAccessibilityCustomRotor {
fileprivate func dumpItems() -> [NSObject] {
var results = [UIAccessibilityCustomRotorItemResult]()

let predicate = UIAccessibilityCustomRotorSearchPredicate()
predicate.searchDirection = .next

let first = itemSearchBlock(predicate)
XCTAssertNotNil(first)
results = [first!]


predicate.currentItem = first!

while let last = results.last,
let next = itemSearchBlock(predicate),
last.targetElement as? NSObject != next.targetElement as? NSObject
{
results = results + [next]
predicate.currentItem = next
}

predicate.searchDirection = .previous
predicate.currentItem = first!

while let last = results.first,
let next = itemSearchBlock(predicate),
last.targetElement as? NSObject != next.targetElement as? NSObject
{
results = [next] + results
predicate.currentItem = next
}


return results.compactMap { $0.targetElement as? NSObject }
}
}

fileprivate struct LabelTapDetectionGrid: Element {

var wrapping: Element
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fixed a bug in `AttributedLabel` which could cause a crash if the attributed string lacked a specified `NSTextAlignment`.

### Added

- `AccessibilityContainer` now supports configuration of `UIAccessibilityContainerType`, `AccessibilityLabel` and `AccessibilityValue`.

### Removed
Expand All @@ -18,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

`AttributedLabel` accessibility links are now stateless.

### Deprecated

- `LayoutMode.legacy` is deprecated and will be removed in a future release.
Expand Down
Loading