cashapp / AccessibilitySnapshot

Easy regression testing for iOS accessibility
Apache License 2.0
554 stars 69 forks source link

Incorrect sort order when using NavigationStack #168

Open DavidBrunow opened 1 year ago

DavidBrunow commented 1 year ago

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 commented 1 year ago

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.

DavidBrunow commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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