Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue/342 kautomator not working properly on views with several scrollables #344

Conversation

sergio-sastre
Copy link
Contributor

@sergio-sastre sergio-sastre commented Dec 23, 2021

This PR is built on top of the issue-306/flaky_autoscroll_when_scrollview_has_padding

It adds a UiHorizontalScrollView to allow automator to explicitly scroll on HorizontalScrollViews, in order to workaround those cases in which Kautomator autoscroll do not work (i.e. numerous scrollable views on the screen)

Remark
I'd rather merge this into issue-306 before merging into master (cannot set target branch to issue-306 though), or rather merge this into master and close the PR for the issue-306, because the code is also in this PR that aims to resolve the scrolling issues with Kautomator and Kaspresso

Closes #342

Copy link
Contributor Author

@sergio-sastre sergio-sastre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should also add sth abut the scrolling issues with Kaspresso (i.e. padding) and Kautomator (i.e. numerous scrollable views in the screen under test) in the readme, so that users that encounter cut problems are aware of how to workaround such problems

Copy link
Collaborator

@eakurnikov eakurnikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please change the target pr branch from master to that one? It would be easier to see the difference.
As far as I can see here it is ok.

@sergio-sastre
Copy link
Contributor Author

sergio-sastre commented Feb 1, 2022

Could you please change the target pr branch from master to that one? It would be easier to see the difference.
As far as I can see here it is ok.

Hi @eakurnikov !
It seems I cannot set the target branch to sergio-sastre:issue-306-..., likely because it is from a forked repo.
So I did it in the forked repo for easier reviewing
sergio-sastre#2
hope it helps :)

@eakurnikov
Copy link
Collaborator

eakurnikov commented Feb 21, 2022

@sergio-sastre hey, I've just checked it in your fork, thanks, it is ok. But we need small fixes in the included #321 to merge it

@sergio-sastre
Copy link
Contributor Author

@sergio-sastre hey, I've just checked it in your fork, thanks, it is ok. But we need small fixes in the included #321 to merge it

Sure, I'll take a look next week. Thanks for reviewing @eakurnikov !

Kautomator scrolls through all the Scrollable views (also nested ones) in the hierarchy till it finds the view we want to perfom the action on
@sergio-sastre
Copy link
Contributor Author

sergio-sastre commented May 28, 2022

@sergio-sastre hey, I've just checked it in your fork, thanks, it is ok. But we need small fixes in the included #321 to merge it

@eakurnikov, @matzuk
So I was playing around with Kautomator, and I found a way to actually solve the autoscroll for Kautomator for real, going through all the scrollables UIAutomator finds on the screen :)

You can review this commit

I suggest to first merge the issue-306/flaky autoscroll, which I'll finish this weekend, and then I'll rebase this ticket from develop accordingly :)

Since the autoscroll goes through all the scrollables, the (on purpose) failing tests is now passing. Not necessary anymore :)
@eakurnikov
Copy link
Collaborator

eakurnikov commented Jun 2, 2022

Hey @sergio-sastre , thank you a lot, sorry for the delay, I will review the changes in the nearest few days

@sergio-sastre
Copy link
Contributor Author

sergio-sastre commented Jun 7, 2022

Hey @sergio-sastre , thank you a lot, sorry for the delay, I will review the changes in the nearest few days

I'd like to do still some small changes.
I've realised there are 3 situations in which it does not scroll till the target view:

  • ViewPagers: In general they also need to call UiScrollable#.setAsHorizontalScroll()
  • RecyclerViews with LinearLayoutOrientation.HORIZONTAL: Also need call to UiScrollable#.setAsHorizontalScroll()
  • Horizontal ScrollableViews in the middle of the main vertical scrolling view, not visible before starting to scroll. The changes I added do only work when those views are visible before starting any scrolling, due to the way uiAutomator works.

1 & 2 are easy to fix without affecting scrolling speed significantly. However, although I know how to solve 3, I think it might considerably decrease the scrolling speed, also for those layouts not containing horizontal ScrollableViews at all... I need to sleep on it

@RuslanMingaliev RuslanMingaliev changed the title Issue 342/kautomator not working properly on views with several scrollables #342 kautomator not working properly on views with several scrollables Jun 9, 2022
@RuslanMingaliev RuslanMingaliev changed the title #342 kautomator not working properly on views with several scrollables Closes #342 kautomator not working properly on views with several scrollables Jun 9, 2022
@RuslanMingaliev RuslanMingaliev changed the title Closes #342 kautomator not working properly on views with several scrollables Issue/342 kautomator not working properly on views with several scrollables Jun 9, 2022
@sergio-sastre
Copy link
Contributor Author

sergio-sastre commented Jul 14, 2022

Hey @sergio-sastre , thank you a lot, sorry for the delay, I will review the changes in the nearest few days

I'd like to do still some small changes. I've realised there are 3 situations in which it does not scroll till the target view:

  • ViewPagers: In general they also need to call UiScrollable#.setAsHorizontalScroll()
  • RecyclerViews with LinearLayoutOrientation.HORIZONTAL: Also need call to UiScrollable#.setAsHorizontalScroll()
  • Horizontal ScrollableViews in the middle of the main vertical scrolling view, not visible before starting to scroll. The changes I added do only work when those views are visible before starting any scrolling, due to the way uiAutomator works.

1 & 2 are easy to fix without affecting scrolling speed significantly. However, although I know how to solve 3, I think it might considerably decrease the scrolling speed, also for those layouts not containing horizontal ScrollableViews at all... I need to sleep on it

@RuslanMingaliev @eakurnikov @RuslanMingaliev
I've just realized that this PR does not depend on the other one regarding the error on ScrollViews with paddings...
I will close this and reopen a new one adressing this issue in the following days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Kautomator] auto-scrolling not working properly in screens with several Scrollables
2 participants