Shot
Shot copied to clipboard
pedrovgs
Make focus consistent when taking screenshots
We've been experimenting some flakiness because of the focus at the moment of taking the screenshot. It would be nice to have some consistency out of the box.
In some project we "clear" the focus we iterate over all the views, starting from the root view, and then requestFocusFromTouch() to make it consistent. In some other projects we take a view id that will be the one that will request the focus.
Right now this feature can be implemented in every project manually thanks to the ScreenshoTest prepareUIForScreenshot you can override when needed. Even when I'm sure having this feature I'm wondering if it has to be performed before taking every screenshot or the library user should opt to it 🤔
I think it is better to let the user opt in, the same way as flakyComponents. Otherwise you are doing some likely expensive computation (iterating through the full view tree) just for some special screens where the focus is a source of flakiness.
I would gladly contribute to this doing that in a similar way to disable flaky components :)
@fmontesino I just opened a PR for this. Since you already implemented a similar solution, could you take a look at the PR?
I am wondering whether requestFocus() would be enough instead of iterating through the full view tree to clear focus of every view
Based on my experience I can say the best thing you can do with the focus when taking screenshots is to remove it, not to get it as part of any other view. That's why we added this behavior long ago before making the screenshot. If you are finding some inconsistencies @fmontesino it could be great if you could provide a reproducible example we can use to fix this scenario.
About opting in I don't think this is so expensive, I'd need to measure the time needed to clear the focus on an average screen before making any decission.
About the PR, I'm going to review it now.
Based on my experience I can say the best thing you can do with the focus when taking screenshots is to remove it, not to get it as part of any other view. That's why we added this behavior long ago before making the screenshot. If you are finding some inconsistencies @fmontesino it could be great if you could provide a reproducible example we can use to fix this scenario.
About opting in I don't think this is so expensive, I'd need to measure the time needed to clear the focus on an average screen before making any decission.
About the PR, I'm going to review it now.
@pedrovgs In my opinion, the mentioned behaviour disabling the cursors is different from making one view focused. Not all views that look different when focused have a cursor. Moreover, calling clearFocus() does not solve the issue because the first focusable view will take focus after that (and could be the one that called clearFocus().
On the other hand, it would be very helpful if @fmontesino could provide a more detailed example of the problem experienced.