webdriverio
webdriverio copied to clipboard
webdriverio
Align click command consistency
Proposed changes
If the click command is called with no options, the click action on the element is executed, but when passing empty options or just a button left option, then the code first scrolls the button into view and then clicking. By just setting the options to an empty object if it is not passed in, the code follows the same pattern for all options. Resolves bug: 8628
Types of changes
- [ x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation update
Checklist
- [ x] I have read the CONTRIBUTING doc
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have added necessary documentation (if appropriate)
- [ ] I have added proper type definitions for new commands (if appropriate)
Further comments
By setting options to an empty object, it can follow the current code flow. I also looked into setting a scroll into view just before the elementClick command, but that introduces another place where you scroll and click, adding more complexity to the code. So I rather chose a simple fix that follows the existing code paths.
Reviewers: @webdriverio/project-committers
@erwinheitzman what do you think about this? My only concern is that a click now is 2 WebDriver commands rather than 1.
I did now was just to add the scrollIntoView before the click.
To be honest I am not happy about this. You add unnecessary scroll for hundreds of thousands users who don't need it.
For me $().click() scrolls as webdriver spec defines. Seems it will be better if you cover your corner case by custom click command in your own project or raise an issue in chrome\geckodriver repo.
Rather looking at it from a big project where we ran into this issue as well (the same Ionic upgrade), we had to take this into account where I expected WebdriverIO to be able to handle this.
Why scroll to something only sometimes but sometimes not? I would say that consistency is more important then the performance cost as it's so little that I doubt anyone will notice it.
I was honestly surprised that it won't scroll because it's completely accessible and possible, it's just that because of some edgecase, webdriver won't.
It's easy to create a wrapper so it's not like it's hard to work around it but I do feel like it would be nice if webdriver handled this by default :)
@erwinheitzman @DirkoOdendaal can you share a little bit more context? When you say you both have Ionic apps that means we are talking about mobile native apps, right? In this case I would suggest to only do the scrolling for mobile because WebDriver indeed scrolls the element into viewport.
@erwinheitzman @DirkoOdendaal can you share a little bit more context? When you say you both have Ionic apps that means we are talking about mobile native apps, right? In this case I would suggest to only do the scrolling for mobile because WebDriver indeed scrolls the element into viewport.
Hybrid app actually, Ionic started to use (open) shadow dom and it's my assumption that this is the reason why commands like click didn't scroll to the elements anymore. What I also noticed is that the getText command returned '' instead of the actual textContent of the element. Since it was an app of another team I only assisted in explaining what I thought was the reason as to why the click no longer acted the same after the change. The update we did was from Ionic v5 to v6.
Strange thing is that they could still access the elements like they weren't inside shadow dom. In other words they still used $ and $$ which work fine when you first scroll to the element and then execute the click or getText command.
I might be able to retrieve some additional information at some point but I do think this is all the info I have to share right now.
Examples:
elem.click() // doesn't click
// became
elem.scrollIntoView()
elem.click() // does click
elem.getText() // returns ''
// became
elem.scrollIntoView()
elem.getText() // returns 'foo'
Since this is my assuption, another suspicion I have is that it might be a virtual dom that might be the reason.
@erwinheitzman @DirkoOdendaal can you share a little bit more context? When you say you both have Ionic apps that means we are talking about mobile native apps, right? In this case I would suggest to only do the scrolling for mobile because WebDriver indeed scrolls the element into viewport.
Hybrid app actually, Ionic started to use (open) shadow dom and it's my assumption that this is the reason why commands like
clickdidn't scroll to the elements anymore. What I also noticed is that thegetTextcommand returned''instead of the actual textContent of the element. Since it was an app of another team I only assisted in explaining what I thought was the reason as to why the click no longer acted the same after the change. The update we did was from Ionic v5 to v6.Strange thing is that they could still access the elements like they weren't inside shadow dom. In other words they still used $ and $$ which work fine when you first scroll to the element and then execute the
clickorgetTextcommand.I might be able to retrieve some additional information at some point but I do think this is all the info I have to share right now.
Examples:
elem.click() // doesn't click // became elem.scrollIntoView() elem.click() // does clickelem.getText() // returns '' // became elem.scrollIntoView() elem.getText() // returns 'foo'Since this is my assuption, another suspicion I have is that it might be a virtual dom that might be the reason.
So what Erwin explains here is exactly how we started. I thought it might just be Ionic 6, but in ht eexample project I supplied, it used Ionic 5. Which is where I got confused.
If the correct thing to do is to try and raise a bug with webdriver then that's 100% fine.
I'll run that project on the other browsers too and see if it also happens.
@DirkoOdendaal I think it only appeared in v6 as they aren't using many ionic components, it might be that the components that they use where affected in v6 as two components where updated in that version. I can't say for sure though as I was contacted to provide a solution and provide an answer of why their tests where suddenly failing but I have little knowledge about what components are used and such.
@DirkoOdendaal I'ld suggest we only scroll into view if we get an element not intractable error as part of a retry. I also raised https://github.com/w3c/webdriver/pull/1680 to change the current behavior of browser scrolling the element just to the end of the viewport rather than to the center.
What do you think?
@christian-bromann
I've talked with @DirkoOdendaal about this last week, but based on the W3C specs this would be an external bug in ChromeDriver because, according to that spec, it should do this
The Element Click command scrolls into view the element if it is not already pointer-interactable, and clicks its in-view center point.
Solving it in WDIO would be good, but then we also need to update our docs on this. Reason for mentioning this is that WDIO also has a few commands from the W3C specs that have additional actions in it which are not clearly described in the docs.
What do you think?
@wswebcreation @DirkoOdendaal
Solving it in WDIO would be good
How?
has a few commands from the W3C specs that have additional actions in it which are not clearly described in the docs.
Which ones? WebdriverIO should clearly communicate protocol commands vs. framework commands which come with additional features and I am all in for making sure these are clearly documented.
I will go ahead and close the PR given there doesn't seem to be any movement anymore. I am all ears for suggestions and improvements on this topic.