Fix bug with table view elements that are matched but offscreen #1070
Conversation
justinseanmartin
requested review from
RoyalPineapple,
harleyjcooper and
mikelupo
| } | ||
|
|
||
| container = container.superview; | ||
| } while (container != nil); |
There was a problem hiding this comment.
Do you never break out of this? It seems like you're going to keep looping. Unless there's an implicit return in the call to [containerScrollView scrollRectToVisible:animated:]?
There was a problem hiding this comment.
container = container.superview; will eventually hit the top of the view hierarchy and it's superview will be nil.
There was a problem hiding this comment.
So no matter what, we always want to run through the element's entire view hierarchy, calling [containerScrollView scrollRectToVisible:animated:] on everything we can? We're okay with potentially calling that multiple times?
There was a problem hiding this comment.
Yep, we'll call it on all of the scroll views up to the root of the hierarchy. If you have a scrollview within a scrollview, all of them should be positioned such that the element to be tapped on should scroll to be visible.
|
I will try to finish this up to apply Justin’s comments today or tomorrow. Sorry this is taking so long. |
|
I'm not sure that this is the right place to fix this. I know there is a bug here, the question is more on what the right way to address it. Going to leave this open for a while longer and hope I get time to actually investigate this. |
justinseanmartin
force-pushed
the
jmartin/scrollviews-before-tapping
branch
from
36db1ea to
cf7ece5
Compare
The TableViewTests exposed an inadequacy in the algorithm that searched for cells that are off the screen. This commit addresses that by scrolling to the last visible row then scrolling the section by CGRect. Both are needed to make this hack work.
This bug has existed for a while, but I hadn't gotten around to fixing it until verifying the fix in #1020, which started hitting this in our app.
The issue is that if a table view cell contains a matching element, and that element is fully offscreen but the tableViewCell is partially onscreen, we won't scroll it into view during the matching operation (in
-[UIView(KIFAdditions) accessibilityElementMatchingBlock:notHidden:]) and tapping the element will fail.The fix here is that right before attempting to tap an element, if it isn't currently tappable on the screen and is contained within one or more UIScrollView's, attempt to reposition the scroll views such that the element to tap is visible on the screen.