Add offset support to click and hover#57
Conversation
ernestas-zekas
commented
Jul 2, 2020
ernestas-zekas
commented
- Added new method getCoordinates in Elements class;
- Refactored clicks and hover methods to use new method getCoordinates;
- Added new tests to cover changes in framework;
- Fixed false positive testcase;
- Added duration parameter to abortRequestsAfterAction method in interceptor class.
- Refactored clicks and hover methods to use new method getCoordinates; - Adde new tests to cover changes in framework; - Fixed false postive testcase; - Added duration parameter to abortRequestsAfterActionmethod in interceptor class.
ernestas-zekas
requested review from
Martynienas,
dariusnemeiksis and
vaidasmaciulis
# Conflicts: # example/tests/elementActions.test.js # example/tests/interceptor.test.js
framework/Element.js
Outdated
| const yCoordinate = rect.y + y; | ||
| console.log(`Action on page at position x: ${xCoordinate}, y: ${yCoordinate}`); | ||
| console.log(`Action on element rectangle at position x: ${x}, y: ${y}`); | ||
| return [xCoordinate, yCoordinate]; |
There was a problem hiding this comment.
For readability I suggest returning this as an object, like return { x: xCoordinate, y: yCoordinate };
Then it can be called as coordinates.x, coordinates.y in line 63 and others
framework/Element.js
Outdated
| } | ||
|
|
||
| async click() { | ||
| async getCoordinates(xOffset, yOffset, elementHandle) { |
There was a problem hiding this comment.
I was thinking how this might be used from test, as it will be public. User might expect to call it as element.getCoordinates() and simply get the coordinates of the element. But for this to work we need waiter inside, and we should not pass elementHandle.
So, this method can have lines inside:
const elementHandle = await this.wait();
await elementHandle.focus();
Then in other methods i.e. click(), these lines can be removed, as only coordinates is required to do the action. And we know element will be already loaded as wait() have already been called in this method.
There was a problem hiding this comment.
As this is new public method we need to add separate test for it.
| } | ||
|
|
||
| async abortRequestsDuringAction(action, requestUrlFragment = "") { | ||
| async abortRequestsAfterAction(action, requestUrlFragment = "", waitDuration = 500) { |
There was a problem hiding this comment.
Just a note: this is breaking change, due to rename and next release will need to have major version updated. (We have merged some breaking changes already, so it will be anyways)
# Conflicts: # example/tests/interceptor.test.js
ernestas-zekas
requested review from
jevgenijusmarinuskinas and
vaidasmaciulis
4 participants
