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.

Sign up for GitHub

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

Jump to bottom

Incorrect sort order when using NavigationStack #168

Open
DavidBrunow opened this issue Oct 18, 2023 · 4 comments · May be fixed by #169
Open

Incorrect sort order when using NavigationStack #168

DavidBrunow opened this issue Oct 18, 2023 · 4 comments · May be fixed by #169
Labels
bug Something isn't working as expected P1: High Priority Issues that would result in incorrectly accessing whether or not a view is accessible. Switch Control parity Mismatch between output of framework and actual Switch Control behavior VoiceOver parity Mismatch between output of framework and actual VoiceOver behavior

Comments

@DavidBrunow
Copy link
Contributor

DavidBrunow commented Oct 18, 2023 •
edited
Loading

I've found that when using a NavigationStack in a SwiftUI view accessibility snapshot puts the contents of the navigation bar below the contents of the view that is contained in the NavigationStack. That is a lot of words that may or may not be helpful so I'll share some snapshots:

NavigationView NavigationStack
testNavigationView 1 testNavigationStack 1

Here is the code I used to generate those snapshots:

import AccessibilitySnapshot
import SnapshotTesting
import SwiftUI
import XCTest

class NavigationExampleSnapshotTests: XCTestCase {
  func testNavigationView() {
    let view = NavigationView {
      Text("Text inside a NavigationView")
        .navigationTitle("Navigation View")
        .toolbar {
          ToolbarItem {
            Button {
              // no-op
            } label: {
              Text("Add")
            }
          }
        }
    }

    let hostingController = UIHostingController(rootView: view)
    hostingController.view.frame = UIScreen.main.bounds

    assertSnapshot(
      of: hostingController,
      as: .accessibilityImage(drawHierarchyInKeyWindow: true)
    )
  }

  func testNavigationStack() {
    let view = NavigationStack {
      Text("Text inside a NavigationStack")
        .navigationTitle("Navigation Stack")
        .toolbar {
          ToolbarItem {
            Button {
              // no-op
            } label: {
              Text("Add")
            }
          }
        }
    }

    let hostingController = UIHostingController(rootView: view)
    hostingController.view.frame = UIScreen.main.bounds

    assertSnapshot(
      of: hostingController,
      as: .accessibilityImage(drawHierarchyInKeyWindow: true)
    )
  }
}

I've noticed this issue when running Xcode 15 in an iOS 17 simulator. I have not tested previous simulators but I think the issue is related to NavigationStack and not to the simulator.
@NickEntin
Copy link
Collaborator

Hey @DavidBrunow, thanks for the bug report! Have you confirmed on an iOS 17 device whether this matches VoiceOver behavior for these view setups, or whether VO reads them the same as expected? Curious whether this is a bug in how AccessibilitySnapshot reads the accessibility hierarchy or how NavigationStack constructs the hierarchy.

Also noticed the highlighting of the elements looks off. That definitely seems like an issue, though separate from the ordering.

@NickEntin NickEntin added bug Something isn't working as expected VoiceOver parity Mismatch between output of framework and actual VoiceOver behavior P1: High Priority Issues that would result in incorrectly accessing whether or not a view is accessible. Switch Control parity Mismatch between output of framework and actual Switch Control behavior labels Oct 24, 2023
@DavidBrunow
Copy link
Contributor Author

Testing in my app where I first saw this issue which also uses a NavigationStack, VoiceOver reads the order as expected (Trailing Navigation Bar Button -> Title -> First Heading in List). I'll look through the code and see if I can submit a PR to fix.

And yes, the highlighting of the elements in the navigation is off as well. I've been able to fix this in my project by using the key window instead of creating a separate UIWindow here: https://github.com/cashapp/AccessibilitySnapshot/blob/master/Sources/AccessibilitySnapshot/SnapshotTesting/SnapshotTesting%2BAccessibility.swift#L68

I do not have enough experience with the change yet to know whether it has other side effects. I can create a PR for that as well to at least spur discussion.

@DavidBrunow
Copy link
Contributor Author

Small and not very useful update:

Changing this explicitlyOrdered value from true to false resolves the issue: https://github.com/cashapp/AccessibilitySnapshot/blob/master/Sources/AccessibilitySnapshot/Core/Swift/Classes/AccessibilityHierarchyParser.swift#L606C32-L606C32

Of course that value is set to true for a reason and other things break so I'm still digging in.

@DavidBrunow
Copy link
Contributor Author

Also noticed the highlighting of the elements looks off. That definitely seems like an issue, though separate from the ordering.

Looking at this more, the highlighting of the elements is an artifact of my working environment. Here are screenshots from working inside the AccessibilitySnapshot module:

NavigationStack NavigationView
testNavigationStack 393x852-16-4-3x testNavigationView 393x852-16-4-3x

@DavidBrunow DavidBrunow linked a pull request Nov 5, 2023 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working as expected P1: High Priority Issues that would result in incorrectly accessing whether or not a view is accessible. Switch Control parity Mismatch between output of framework and actual Switch Control behavior VoiceOver parity Mismatch between output of framework and actual VoiceOver behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix incorrect sort order of NavigationStack DavidBrunow/AccessibilitySnapshot
2 participants
@DavidBrunow @NickEntin