AttributedLabel Accessibility Links #537
Conversation
RoyalPineapple
changed the title
| @@ -389,7 +364,7 @@ extension AttributedLabel { | |||
| } | |||
|
|
|||
| assert( | |||
| alignments.count == 1, | |||
There was a problem hiding this comment.
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.
RoyalPineapple
changed the title
RoyalPineapple
force-pushed
the
alex/RotorAdditions
branch
2 times, most recently
from
e5ccc90 to
91e208b
Compare
RoyalPineapple
force-pushed
the
alex/RotorAdditions
branch
from
91e208b to
173630b
Compare
| 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.
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.
RoyalPineapple
force-pushed
the
alex/RotorAdditions
branch
from
ece74e1 to
8ff1979
Compare
RoyalPineapple
force-pushed
the
alex/RotorAdditions
branch
from
8ff1979 to
881411f
Compare
| extension BidirectionalCollection where Element: Equatable, Element: NSObjectProtocol { | ||
| private func itemSearch(_ predicate: UIAccessibilityCustomRotorSearchPredicate) -> UIAccessibilityCustomRotorItemResult? { | ||
| guard let first else { return nil } | ||
| guard let currentItem = predicate.currentItem.targetElement as? Element, |
There was a problem hiding this comment.
Thinking outloud, im surprised to see both the generic constraint and this dynamic check; are both required?
There was a problem hiding this comment.
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.
| @@ -0,0 +1,21 @@ | |||
| import UIKit | |||
|
|
|||
| extension BidirectionalCollection where Element: Equatable, Element: NSObjectProtocol { | |||
There was a problem hiding this comment.
Nit: I think you can also combine the constraints
| extension BidirectionalCollection where Element: Equatable, Element: NSObjectProtocol { | |
| extension BidirectionalCollection where Element: Equatable & NSObjectProtocol { |
There was a problem hiding this comment.
Ah, of course!
I initially wrote Element: Equatable, NSObjectProtocol and this was Xcode's suggested fix.
| } | ||
|
|
||
| /// Returns a UIAccessibilityCustomRotor with the provided system type which cycles through the contained elements. | ||
| public func accessibilityRotor(systemType type: UIAccessibilityCustomRotor.SystemRotorType) -> UIAccessibilityCustomRotor { |
There was a problem hiding this comment.
Nit: Put the public func at the top of the file, and the private extension after?
This PR migrates
AttributedLabel's link rotor from a stateful to functional pattern, eliminating the need for two variables and preventing voiceover from getting out of sync.I also fixed a bug in
links(at location: CGPoint)that could cause a crash if theAttributedStringlacked a specifiedNSTextAlignment.