-tap should raise when the control has no width or height

[tjarratt]

Encountered this on a project today. It felt very strange that PCK would allow you to tap on controls that wouldn't be able to receive touch events in a real setting. This sparked a discussion around "should we be using -clipsToBounds=Yes ?" but decided that improving PCK to catch these errors earlier would be better.

For reference, the problem we needed to fix had to do with autolayout. We had forgot to set some autolayout constraints correctly and ended up with a zero width, zero height control that, while it looked okay, wasn't tappable.

[joemasilotti]

What about developers testing controllers with zero-frame views? For example, I use -tap for testing logic around my controllers, but that controller isn't necessarily attached to a window or even has a frame.

[akitchen]

There are other cases where it raises due to being untappable - I think we should be consistent.

Joe, I probably wouldn't use -tap in your case - I would either use a non-zero-frame view or separate the view from the controller completely.

On Wed, Nov 18, 2015 at 03:30 Joe Masilotti notifications@github.com wrote:

What about developers testing controllers with zero-frame views? For example, I use -tap for testing logic around my controllers, but that controller isn't necessarily attached to a window or even has a frame.

— Reply to this email directly or view it on GitHub https://github.com/pivotal/PivotalCoreKit/pull/172#issuecomment-157684338 .

[tjarratt]

Thanks for the feedback @joemasilotti. I was a little concerned about how this might negatively affect existing users, I was hoping this PR might spark some discussion, and I'm glad it did. My feeling is that since this should help catch errors earlier, that the potential breakage is worth it in the long run.

If you want to try this out in one of your projects and help me better understand how a UIControl ends up in a state with no width or height, maybe we can make this PR better together.

I too often find myself testing the behavior of view controllers outside of a UIWindow, but if you trigger -viewDidLoad on iOS 8 or 9, then the view should almost always be given a non-zero frame, especially if it was loaded from a nib, storyboard, uses autolayout, or if you created it programmatically. I can't think of any situation where I would want to call -tap on a control that didn't have a valid frame.

Well, actually. Maybe. Do you ever write tests for custom UIControl subclasses like this?

describe(@"MyAwesomeWidget", ^{
  __block MyAwesomeWidget *subject;

  beforeEach(^{
    subject = [[MyAwesomeWidget alloc] init]; // zero frame, naturally
  });

  it(@"wibbles the wobble", ^{
    id delegate = nice_fake_for(@protocol(Wobbler));
    subject.delegate = delegate;

    [subject tap];
    delegate should have_received(@selector(wobbledWidget:)).with(subject);
  });
});

In that case, I can see how that would be frustrating. Day to day, writing tests for iOS, I find myself tapping on UIButtons in view controllers, which always have a frame set. In the case above, I'd suggest that this test never made much sense, since you could never really trigger a touch up inside on a control that was setup like this.

[joemasilotti]

@tjarratt testing a UIControl subclass is my exact use-case. IMO it seems like overkill to require bringing in a window (and adding your view to it) for -tap to work. (Just initializing the control with a frame isn't enough to avoid the exception.)

These specific tests are only using the method as a shortcut to calling the action on the target. They don't necessarily care where or how the view renders.

[akitchen]

It should not need to be in a window, I would agree. However a non-zero frame and user interaction enabled are reasonable requirements.

On Mon, Nov 23, 2015 at 9:28 AM, Joe Masilotti notifications@github.com wrote:

@tjarratt testing a UIControl subclass is my exact use-case. IMO it seems like overkill to require bringing in a window (and adding your view to it) for -tap to work. (Just initializing the control with a frame isn't enough to avoid the exception.)

These specific tests are only using the method as a shortcut to calling the action on the target. They don't necessarily care where or how the view renders.

Reply to this email directly or view it on GitHub: https://github.com/pivotal/PivotalCoreKit/pull/172#issuecomment-159003446

[tjarratt]

I think if you don't want to verify that a UIControl has a frame, has user interaction enabled and is not disabled, you probably just want to invoke -sendActionsForControlEvents:UIControlEventTouchUpInside directly.

-tap, at least the direction it's been going in the last year, should really be about verifying that a control is in a good state and verify that a user can interact with it. This means verifying that it's currently enabled. If its presenting view controller disabled it, then -tap should be a no-op. If it has a non-zero frame, then it shouldn't actually allow touch events. It's useful for catching bugs before you actually start your app.

Is that a satisfying answer @joemasilotti? I don't want to sweep your concerns under the rug, but I also want to find a way to resolve the tension between making your tests harder to write and being able to catch these errors sooner.

[joemasilotti]

That's a good point.

The only reason my tests use -tap in both scenarios is to make them easier to read. I could easily extract the one line of code into a new helper method to achieve the same results.

Thanks for taking the time to listen to and reason with the community!

[tjarratt]

Thanks for helping us improve PCK and keep it stable @joemasilotti!

[tjarratt]

Merged from the command line.