-
Notifications
You must be signed in to change notification settings - Fork 84
Updated Sort Order #244
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
Updated Sort Order #244
Changes from all commits
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 |
|---|---|---|
|
|
@@ -79,9 +79,18 @@ public protocol UserInterfaceLayoutDirectionProviding { | |
| var userInterfaceLayoutDirection: UIUserInterfaceLayoutDirection { get } | ||
|
|
||
| } | ||
|
|
||
| extension UIApplication: UserInterfaceLayoutDirectionProviding {} | ||
|
|
||
|
|
||
| public protocol UserInterfaceIdiomProviding { | ||
|
|
||
| var userInterfaceIdiom: UIUserInterfaceIdiom { get } | ||
|
|
||
| } | ||
|
|
||
| extension UIDevice: UserInterfaceIdiomProviding {} | ||
|
|
||
|
|
||
| // MARK: - | ||
|
|
||
| public final class AccessibilityHierarchyParser { | ||
|
|
@@ -171,17 +180,20 @@ public final class AccessibilityHierarchyParser { | |
| /// In most cases, this should use the default value, `UIApplication.shared`. | ||
| public func parseAccessibilityElements( | ||
| in root: UIView, | ||
| userInterfaceLayoutDirectionProvider: UserInterfaceLayoutDirectionProviding = UIApplication.shared | ||
| userInterfaceLayoutDirectionProvider: UserInterfaceLayoutDirectionProviding = UIApplication.shared, | ||
| userInterfaceIdiomProvider: UserInterfaceIdiomProviding = UIDevice.current | ||
| ) -> [AccessibilityMarker] { | ||
| let userInterfaceLayoutDirection = userInterfaceLayoutDirectionProvider.userInterfaceLayoutDirection | ||
|
|
||
| let userInterfaceIdiom = userInterfaceIdiomProvider.userInterfaceIdiom | ||
|
|
||
| let accessibilityNodes = root.recursiveAccessibilityHierarchy() | ||
|
|
||
| let uncontextualizedElements = sortedElements( | ||
| for: accessibilityNodes, | ||
| explicitlyOrdered: false, | ||
| in: root, | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection, | ||
| userInterfaceIdiom: userInterfaceIdiom | ||
| ) | ||
|
|
||
| let accessibilityElements = uncontextualizedElements.map { element in | ||
|
|
@@ -190,7 +202,8 @@ public final class AccessibilityHierarchyParser { | |
| context: context( | ||
| for: element.object, | ||
| from: element.contextProvider, | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection, | ||
| userInterfaceIdiom: userInterfaceIdiom | ||
| ) | ||
| ) | ||
| } | ||
|
|
@@ -271,11 +284,13 @@ public final class AccessibilityHierarchyParser { | |
| /// this should typically be `false`. | ||
| /// - parameter root: The root view to which the nodes' shapes are relative. | ||
| /// - parameter userInterfaceLayoutDirection: The device's current user interface layout direction. | ||
| /// - parameter userInterfaceIdiom: the device's interface idiom, used to calculate the sort order | ||
| private func sortedElements( | ||
| for nodes: [AccessibilityNode], | ||
| explicitlyOrdered: Bool, | ||
| in root: UIView, | ||
| userInterfaceLayoutDirection: UIUserInterfaceLayoutDirection | ||
| userInterfaceLayoutDirection: UIUserInterfaceLayoutDirection, | ||
| userInterfaceIdiom: UIUserInterfaceIdiom = UIDevice.current.userInterfaceIdiom | ||
| ) -> [Element] { | ||
| // VoiceOver flick navigation iterates through elements in a horizontal, then vertical order. The horizontal | ||
| // ordering matches the application's user interface layout direction. The vertical ordering is always | ||
|
|
@@ -300,9 +315,9 @@ public final class AccessibilityHierarchyParser { | |
| fatalError("Unknown user interface layout direction: \(userInterfaceLayoutDirection)") | ||
| } | ||
|
|
||
| // 8 seems to be the magic number for VoiceOver to consider it | ||
| // to be vertically "above" other views. | ||
| let minimumVerticalSeparation = 8.0 | ||
| // Derived via experimentation, these magic numbers are the cutoff for VoiceOver to consider | ||
| // an element to be vertically "above" other views. | ||
| let minimumVerticalSeparation = userInterfaceIdiom == .phone ? 8.0 : 13.0 | ||
|
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. Not for this PR, but I am wondering if Voiceover is also a bit more clever when seeing a hierarchy like this: Eg does it try to also group content into "rows" (eg, 1, 2, 3 would be in one group, 4, 5, 6) in another, and then orders the items in the groups LTR/RTL based on layout direction, rather than purely by vertical separation? That would make a bit more sense to me.
Collaborator
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. I thought I responded to this comment, but maybe not? I set up a test project and tested out some scenarios similar to the one you described. In all cases, it seems like VoiceOver is simply doing it based on the leading-to-trailing, top to bottom (within a tolerance) regardless of the layout of the elements. |
||
|
|
||
| let sortedNodes = explicitlyOrdered ? nodes : nodes | ||
| .map { ($0, accessibilitySortFrame(for: $0, in: root)) } | ||
|
|
@@ -331,7 +346,8 @@ public final class AccessibilityHierarchyParser { | |
| for: elements, | ||
| explicitlyOrdered: explicitlyOrdered, | ||
| in: root, | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection, | ||
| userInterfaceIdiom: userInterfaceIdiom | ||
| ) | ||
| ) | ||
| } | ||
|
|
@@ -391,7 +407,8 @@ public final class AccessibilityHierarchyParser { | |
| private func context( | ||
| for element: NSObject, | ||
| from contextProvider: ContextProvider?, | ||
| userInterfaceLayoutDirection: UIUserInterfaceLayoutDirection | ||
| userInterfaceLayoutDirection: UIUserInterfaceLayoutDirection, | ||
| userInterfaceIdiom: UIUserInterfaceIdiom | ||
| ) -> Context? { | ||
| guard let contextProvider = contextProvider else { | ||
| return nil | ||
|
|
@@ -436,7 +453,8 @@ public final class AccessibilityHierarchyParser { | |
| for: hierarchy, | ||
| explicitlyOrdered: false, | ||
| in: view, | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection | ||
| userInterfaceLayoutDirection: userInterfaceLayoutDirection, | ||
| userInterfaceIdiom: userInterfaceIdiom | ||
| ).map { $0.object } | ||
| viewToElementsMap[view] = accessibleElements | ||
| } | ||
|
|
||
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.
Noting that in the Market snapshot tests we will want to inject a different
UserInterfaceIdiomProviding, since they're kinda magic: They can run on any 3x iOS device and do swizzling and such to simulate all different device sizes, safe areas, etc, that we need.