theappbusiness / TABTestKit

Library designed to make writing and maintaining automated tests for iOS applications. This includes automation of bio-metrics and controlling of mock servers
MIT License
59 stars 13 forks source link

Added back button for nav bar #141

Closed stefanrenne closed 1 year ago

stefanrenne commented 3 years ago

What's in this PR?

Replace this text with information about what's in your PR. What's changed? Why has it changed? Reference any GitHub issue this PR relates to, etc.


Pre-merge checklist

Before merging any PR, please check the following common things that should be done beforehand. These aren't all always required, so just check the box if it doesn't apply.

stefanrenne commented 3 years ago

A different solution would be to add a custom Element, don't know what would be preferred?

Example:

struct BackBarButton: Element, Tappable {
    let id: String? = nil
    let index: Int = 0
    let parent: Element = NavBar()
    let type: XCUIElement.ElementType = .button
}
KaneCheshire commented 3 years ago

A different solution would be to add a custom Element, don't know what would be preferred?

Example:

struct BackBarButton: Element, Tappable {
    let id: String? = nil
    let index: Int = 0
    let parent: Element = NavBar()
    let type: XCUIElement.ElementType = .button
}

I personally prefer this because it means you're still enforcing that all buttons should have some sort of ID, unlike if you allow nil as the id (which I hope helps with accessibility in the app because people will have to assign an accessibility label or identifier)

KaneCheshire commented 3 years ago

Don't most back buttons have a specific title or label? Either "Back" or the title of the previous screen? This could maybe be solved with an extension or update on NavBar:

extension NavBar {

  func backButton(titled title: String = "Back") -> Button {
    return Button(id: title, parent: self)
  }
}
stefanrenne commented 3 years ago

Don't most back buttons have a specific title or label? Either "Back" or the title of the previous screen? This could maybe be solved with an extension or update on NavBar:

extension NavBar {

  func backButton(titled title: String = "Back") -> Button {
    return Button(id: title, parent: self)
  }
}

By default the back item has the title of the previous screen, would be nice to make it more foolproof

print(navigationController!.navigationBar.backItem!)
// output: <UINavigationItem: 0x7ff8fb006180> title='sdgsdfgdf'
KaneCheshire commented 3 years ago

Don't most back buttons have a specific title or label? Either "Back" or the title of the previous screen? This could maybe be solved with an extension or update on NavBar:

extension NavBar {

  func backButton(titled title: String = "Back") -> Button {
    return Button(id: title, parent: self)
  }
}

By default the back item has the title of the previous screen, so I guess that won't work

print(navigationController!.navigationBar.backItem!)
// output: <UINavigationItem: 0x7ff8fb006180> title='sdgsdfgdf'

Yeah that's what I said 😬 in your tests you'd have to know what the previous screen was so you can tap the back button, but since you're in control of your tests that shouldn't be a problem?

Given(I: tap(navBar.backButton(titled: "sdgsdfgdf")))
stefanrenne commented 3 years ago

I have to check in our real project, because we are doing some magic to hide the title in the navigationbar. Also the default "Back" could be language specific. In our project the title seems to be empty

po navigationController?.navigationBar.backItem?.backBarButtonItem
▿ Optional<UIBarButtonItem>
  - some : <UIBarButtonItem: 0x7fa3ffc5d8c0> menuOnTouchDown menu=0x6000013bad50 title=''
KaneCheshire commented 3 years ago

I have to check in our real project, because we are doing some magic to hide the title in the navigationbar. Also the default "Back" could be language specific. In our project the title seems to be empty

po navigationController?.navigationBar.backItem?.backBarButtonItem
▿ Optional<UIBarButtonItem>
  - some : <UIBarButtonItem: 0x7fa3ffc5d8c0> menuOnTouchDown menu=0x6000013bad50 title=''

You could just remove the default, it was there as a suggestion :)

I'm guessing the title is "" because you've removed the title and just have an arrow? What happens for VoiceOver when it navigates over that, I assume it just reads out "Back"? It might be worth putting a breakpoint in your tests when the button is visible and then printing out the hierarchy so you can see what the tests can see:

po App.shared or po NavBar()

Alternatively you might be able to just use the Accessibility Inspector. As a last resort you could add a custom accessibilityIdentifier to the back button wherever you're removing the title?

stefanrenne commented 3 years ago

Yeah there you go, "Vorige" is Dutch for "Previous"

NavigationBar, 0x60000309a4c0, {{0.0, 58.0}, {414.0, 56.0}}, identifier: 'LargeTitleView'
   Button, 0x60000309a5a0, {{0.0, 64.0}, {44.0, 44.0}}, label: 'Vorige'
KaneCheshire commented 3 years ago

Yeah there you go, "Vorige" is Dutch for "Previous"

NavigationBar, 0x60000309a4c0, {{0.0, 58.0}, {414.0, 56.0}}, identifier: 'LargeTitleView'
   Button, 0x60000309a5a0, {{0.0, 64.0}, {44.0, 44.0}}, label: 'Vorige'

Cool so in your tests you could just use navBar.backButton(titled: "Vorige"), or you might be able to add an accessibilityIdentifier which would remain consistent between languages

stefanrenne commented 3 years ago

Yeah there you go, "Vorige" is Dutch for "Previous"

NavigationBar, 0x60000309a4c0, {{0.0, 58.0}, {414.0, 56.0}}, identifier: 'LargeTitleView'
   Button, 0x60000309a5a0, {{0.0, 64.0}, {44.0, 44.0}}, label: 'Vorige'

Cool so in your tests you could just use navBar.backButton(titled: "Vorige"), or you might be able to add an accessibilityIdentifier which would remain consistent between languages

Sounds like a plan! I just reverted my previous commit & processed your feedback :)