-
Notifications
You must be signed in to change notification settings - Fork 45
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
Changes from all commits
85a5c59
adf2942
f50a50a
173630b
8c0ccaf
881411f
a9d98b1
ecea0b2
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 |
---|---|---|
@@ -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, | ||
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) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
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. This concern about the elements being retained is still valid. We cannot pass back unretained elements created within the The trick here is the elements are now retained in an array which is itself captured by the 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)] | ||
} | ||
} | ||
|
||
|
@@ -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, | ||
|
@@ -389,7 +364,7 @@ extension AttributedLabel { | |
} | ||
|
||
assert( | ||
alignments.count == 1, | ||
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. This was causing a crash in the demo app. |
||
alignments.count <= 1, | ||
""" | ||
AttributedLabel links only support a single NSTextAlignment. \ | ||
Instead, found: \(alignments). | ||
|
@@ -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. | ||
|
||
|
@@ -708,7 +681,7 @@ extension AttributedLabel { | |
} | ||
|
||
override var accessibilityFrameInContainerSpace: CGRect { | ||
set { fatalError() } | ||
set { fatalError("accessibilityFrameInContainerSpace") } | ||
get { | ||
guard let container = container, | ||
let textStorage = container.makeTextStorage(), | ||
|
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.
Thinking outloud, im surprised to see both the generic constraint and this dynamic check; are both required?
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.
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.