-
Notifications
You must be signed in to change notification settings - Fork 85
-tap should raise when the control has no width or height #172
Conversation
What about developers testing controllers with zero-frame views? For example, I use |
There are other cases where it raises due to being untappable - I think we Joe, I probably wouldn't use -tap in your case - I would either use a On Wed, Nov 18, 2015 at 03:30 Joe Masilotti notifications@github.com
|
3dceade
to
ba325c8
Compare
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 Well, actually. Maybe. Do you ever write tests for custom 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 |
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.
ba325c8
to
8aa8167
Compare
@tjarratt testing a 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. |
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
|
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
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. |
That's a good point. The only reason my tests use Thanks for taking the time to listen to and reason with the community! |
Thanks for helping us improve PCK and keep it stable @joemasilotti! |
Merged from the command line. |
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.