Only check for views visible to the user

[immenor]

The idea is to make it so that when searching for any type of UIKit component, it only can find ones that are visible to the user. If we were to go by apple's definition of if a view can't be touched, that would be isHidden == true or alpha < 0.01.

For example, if I have a label that is currently hidden, findLabel(withExactText:) should no longer be able to find that label.

[rgravina]

Just seconding this!

Sometimes I want to check if a label is hidden. hasLabel matches if hidden, and normally you can workaround this by using findLabel then checking isHidden. But if it's parent (or grandparent etc.) is hidden, the only option is to do something like this:

it("does not display guidance") {
    let label = vc.findLabel(withExactText: "Add Words from Sentences")!
    expect(label.superview!.superview!.isHidden).to(beTrue())
}

I think either hasLabels semantics could be changed to be false if it or a parent is hidden (preferred), or have a default second parameter like 'includeHidden: Bool = true' (if you want to keep current semantics but allow for adding this feature in).

[theextremeprogrammer]

@rgravina I completely agree that this should be added! Do you want to give a shot and make a PR? Or pair on it together? Or pair asynchronously? Happy to work together with you or @immenor to build this in. 👍🏻

[theextremeprogrammer]

I think either hasLabels semantics could be changed to be false if it or a parent is hidden (preferred)

This sounds like two different scenarios to me. Naturally the first is that hasLabel returns false if the UILabel itself is hidden. If the parent of a UILabel is hidden and hasLabel or findLabel should return false, then I would think that this has a much wider impact as all methods (hasX or findX) should all honor this. Plus, it's a much more complicated change as it would need information about the parent view. While this issue itself seems like it's covering all scenarios, maybe it's good to start with the UILabel scenario to start to build out this functionality. Then, we could add support for parent views, etc. What do you think?

, or have a default second parameter like 'includeHidden: Bool = true' (if you want to keep current semantics but allow for adding this feature in).

This is definitely possible but I can't think of a situation where the developer would want to turn this off. Besides, as you know, I'm not a huge fan of passing boolean flags into methods! 😂

[theextremeprogrammer]

How about this as a first failing test?

UIViewController+UILabelSpec.swift

final class UIViewController_UILabelSpec: QuickSpec {
    override func spec() {
        describe("finding labels with exact text") {
            ...

+           fcontext("when a UILabel is not displayed to the user") {
+               var viewController: UIViewController!
+           
+               beforeEach {
+                   viewController = UIViewControllerBuilder()
+                       .withSubview(
+                           UILabelBuilder().withTitleText("Username").isHidden().build()
+                       )
+                       .build()
+               }
+           
+               it("cannot find the label because it is hidden") {
+                   expect(viewController.findLabel(withExactText: "Username")).to(beNil())
+               }
+           }

            ...
}

Getting the test to compile requires adding this to the UILabelBuilder:

import UIKit

struct UILabelBuilder {
    private var label: UILabel

    init() {
        label = UILabel()
    }

    func withTitleText(_ titleText: String) -> UILabelBuilder {
        label.text = titleText
        return self
    }

+   func isHidden() -> UILabelBuilder {
+       label.isHidden = true
+       return self
+   }

    func build() -> UILabel {
        return label
    }
}

And I believe this generates the expected test result as the UILabel is found, but shouldn't have been:

Test Case '-[SuccinctTests.UIViewController_UILabelSpec finding_labels_with_exact_text__when_a_UILabel_is_not_displayed_to_the_user__cannot_find_the_label_because_it_is_hidden]' started.
/Users/dereklee/Dropbox/ios-dev/GitProjects/Succinct/SuccinctTests/UIViewController/UIViewController+UILabelSpec.swift:68: error: -[SuccinctTests.UIViewController_UILabelSpec finding_labels_with_exact_text__when_a_UILabel_is_not_displayed_to_the_user__cannot_find_the_label_because_it_is_hidden] : expected to be nil, got <<UILabel: 0x7f9dbc5970d0; frame = (0 0; 0 0); text = 'Username'; hidden = YES; userInteractionEnabled = NO; layer = <_UILabelLayer: 0x7f9dbc591b40>>>

Test Case '-[SuccinctTests.UIViewController_UILabelSpec finding_labels_with_exact_text__when_a_UILabel_is_not_displayed_to_the_user__cannot_find_the_label_because_it_is_hidden]' failed (0.258 seconds).

I've pushed this to the branch issue-58-only-visible-views. Care to make the test pass and write the next failing test? 😃

[rgravina]

This is awesome, thanks for adding this Derek! Canyon has an implementation already we can start with so I'll take a look at that. Will hopefully be able to take a look tonight or tomorrow morning.

On Wed, Jan 20, 2021 at 9:44 Derek Lee notifications@github.com wrote:

How about this as a first failing test?

UIViewController+UILabelSpec.swift

final class UIViewController_UILabelSpec: QuickSpec {

override func spec() {

    describe("finding labels with exact text") {

        ...

• fcontext("when a UILabel is not displayed to the user") {

• var viewController: UIViewController!

• beforeEach {

• viewController = UIViewControllerBuilder()

• .withSubview(

• UILabelBuilder().withTitleText("Username").isHidden().build()

• )

• .build()

• }

• it("cannot find the label because it is hidden") {

• expect(viewController.findLabel(withExactText: "Username")).to(beNil())

• }

• }

      ...

}

Getting the test to compile requires adding this to the UILabelBuilder:

import UIKit

struct UILabelBuilder {

private var label: UILabel

init() {

    label = UILabel()

}

func withTitleText(_ titleText: String) -> UILabelBuilder {

    label.text = titleText

    return self

}

• func isHidden() -> UILabelBuilder {

• label.isHidden = true

• return self

• }

  func build() -> UILabel {

  return label

  }

}

And I believe this generates the expected test result as the UILabel is found, but shouldn't have been:

Test Case '-[SuccinctTests.UIViewController_UILabelSpec finding_labels_with_exact_textwhen_a_UILabel_is_not_displayed_to_the_usercannot_find_the_label_because_it_is_hidden]' started.

/Users/dereklee/Dropbox/ios-dev/GitProjects/Succinct/SuccinctTests/UIViewController/UIViewController+UILabelSpec.swift:68: error: -[SuccinctTests.UIViewController_UILabelSpec finding_labels_with_exact_textwhen_a_UILabel_is_not_displayed_to_the_usercannot_find_the_label_because_it_is_hidden] : expected to be nil, got <<UILabel: 0x7f9dbc5970d0; frame = (0 0; 0 0); text = 'Username'; hidden = YES; userInteractionEnabled = NO; layer = <_UILabelLayer: 0x7f9dbc591b40>>>

Test Case '-[SuccinctTests.UIViewController_UILabelSpec finding_labels_with_exact_textwhen_a_UILabel_is_not_displayed_to_the_usercannot_find_the_label_because_it_is_hidden]' failed (0.258 seconds).

I've pushed this to the branch issue-58-only-visible-views. Care to make the test pass and write the next failing test? 😃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/derekleerock/Succinct/issues/58#issuecomment-763245502, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEVDVNMD66XOIDTSG22ZDS2YRP7ANCNFSM4MRCNVBQ .

[rgravina]

Thanks for the first test. After making it pass, I realise findLabel on UIView would also need a test, and then since isHidden is a UIView property, you might want any UIView to not be found too... which is the title of this issue now I re-read it :)

I'll start with hasLabel on UIView/UIViewController first though. You can see my progress over here https://github.com/derekleerock/Succinct/compare/master...rgravina:issue-58-only-visible-views

[theextremeprogrammer]

Thanks for this @rgravina! Yes, as the issue was requested, UIView should definitely be included. This issue I think covers quite a few different cases, and I have a feeling that the logic might be quite invasive given the different scenarios as this likely needs to apply to all types of UIView objects and anything that inherits from it. I was thinking starting with something like just focusing on UILabel might be helpful. Also, given that I'm not able to share context directly with you like I could with pairing, this should help to narrow down the areas that you need to look at the code for.

It might be helpful to think through some of the different test cases to drive out this functionality. Do you want to give this a first pass? I'm also happy to jot down some ideas. 👍🏻

Let mw know if you'd like me to share any context, or if you have any questions about how things are organized!

[rgravina]

I've moved the check over to isLabel and added a failure case for it. Seemed like it belonged there despite the method name not matching quite right. This makes it work for anything that calls findLabel (e.g. checking in the title bar). These many cases don't have their own tests but since they all use findLabel I guess it makes sense not to have additional redundant tests to maintain.

The next step is I think to handle the case where the label is visible but one of its parents is any kind of UIView that is hidden, therefore making it invisible. From there, I can expand to other types of UIView.

[theextremeprogrammer]

Thanks @rgravina! Yes I agree that including this logic in the isLabel(withExactText:) is a great place to put this logic. Thanks for adding the additional EvaluationResultType as well! The intention with this organization is to be used to output a helpful message to the user in the event that the user actually wants to find the label but isLabel returns false because it is hidden, for example. We can get to that part later as there's likely some other parts that need to be built out to get there.

The next step is I think to handle the case where the label is visible but one of its parents is any kind of UIView that is hidden, therefore making it invisible. From there, I can expand to other types of UIView.

Sounds good! Let me know if you want to ping-pong on this or if you're good moving forward. I'll assign this issue to you for now but I'm happy to tag in anytime!

[rgravina]

I added a guard clause in UIView.findInSubviews to return nil if it is hidden and added test in uiviewcontroller+label here. https://github.com/rgravina/Succinct/commit/3763d94af1cf6a9491b4db9ec9e107418ce87550

It would seem that because it is in UIView, it will work for findImageView or any other function that relies on it.

Here's an example. If I were to add this test, it would pass

it("can not find the image when the specified image is in the first view hierarchy but hidden") {
    let viewController = UIViewControllerBuilder()
        .withSubview(
            UIViewBuilder()
                .withImageView(searchImage)
                .isHidden()
                .build()
        )
        .build()

    expect(viewController.hasImageView(withImage: searchImage)).to(beFalse())
}

I assume you don't want to have a test for all of those. Wondering if some tests for UIView might be useful to cover it?

Do you know of any find method that does not go through UIView.findInSubviews?

I'm sure I must be missing something obvious.

[rgravina]

Yes I think I see what I'm missing now. By doing that, none of the methods that would return an EvaluationResult get called so you won't be able to generate good debugging messages. It also makes the failure case in finding a label redundant... maybe we need EvaluationResult for UIView if it's not there already?

[theextremeprogrammer]

Yes I think I see what I'm missing now. By doing that, none of the methods that would return an EvaluationResult get called so you won't be able to generate good debugging messages. It also makes the failure case in finding a label redundant...

I think you're on the right track! Yes, the reason I introduced the EvaluationResult was to come up with a (de-coupled) way to somehow get information back to the end-user for tests that they may have expected to pass but didn't - or maybe failed for an unknown reason. There's still some work to do to get the right information to the user for these scenarios, but the structure around EvaluationResult (and SuccinctCondition) is the direction I started to work in to try and solve this problem.

findInSubviews() is a method that I've been trying to be particularly careful about the Single Responsibility Principle to ensure that the logic here is really just looping through the subviews. I've managed to move any other logic outside of this method except for checking specific view types (table views) and logging (entering/exiting parent/child views). There are likely still further improvements that could be made here.

However, worth noting is that since I haven't converted all APIs to go through this method yet (due to other challenges..), and that work is still outstanding.

I assume you don't want to have a test for all of those. Wondering if some tests for UIView might be useful to cover it?

Maybe we don't need a test for all of them, but for the purpose of having confidence that the logic is being put in the proper place and that it is working properly I wouldn't be opposed to having a few tests to give some confidence around this and not to necessarily couple the tests to UIView specifically. What do you think?

maybe we need EvaluationResult for UIView if it's not there already?

I don't believe there is an EvaluationResult for UIView yet, but if there were then this logic would be a great candidate for it.

I really haven't given it much thought, but after looking at the SuccinctCondition, this basically encapsulates a function that takes a UIView and executes a condition against it. I wonder if this might be a good location to consider checking this kind of logic?