MarketSquare / robotframework-browser

Robot Framework Browser library powered by Playwright.
Apache License 2.0
527 stars 106 forks source link

Drag And Drop By Coordinates: drop arg does not work for GUI element "virtual joystick" #1961

Closed UliSei closed 1 year ago

UliSei commented 2 years ago

Drag And Drop By Coordinates tested with chromium and webkit

Short overview:

it makes a difference to test the keyword • with a draggable element which stays where it was dropped • or with a virtual joystick which tends to return to its default position when it is dropped Suggested solution see below

Description of the system under test and a speculation why it behaves unexpectedly

There was a new drop argument introduced with v12.2.0 which defaults to True. It was tested with the draggable GUI element and obviously worked fine. However the keyword does not meet the expectations when the GUI element moves by itself to a default position when it is dropped. Testing with a virtual joystick showed, that a sequence of Drag And Drop By Coordinates statements with drop=False had the same behaviour as with drop=True. It looks as if the Browser Lib keyword Drag And Drop By Coordinates is not yet designed to continue a previous call of Drag And Drop By Coordinates with drop=False. The call of Drag And Drop By Coordinates seems to expect and initialize a dropped mouse.

For moving the draggable element it makes no difference whether drop is set to True or False. However when a virtual joystick is moved outside its center and the next Drag And Drop By Coordinates is called, the test shows:

Test case with expected behaviour

Drag a GUI element, the lightened disk, from its origin O to point D passing the points A, B and C.

Drag And Drop By Coordinates    ${Ox}    ${Oy}    ${Ax}    ${Ay}    10    False
Drag And Drop By Coordinates    ${Ax}    ${Ay}    ${Bx}    ${By}    10    False
Drag And Drop By Coordinates    ${Bx}    ${By}    ${Cx}    ${Cy}    10    False
Drag And Drop By Coordinates    ${Cx}    ${Cy}    ${Dx}    ${Dy}    10    False
Drag And Drop By Coordinates    ${Dx}    ${Dy}    ${Ox}    ${Oy}    10    True

The element shall move on a smooth/continuos path, without dropping. When the virtual joystick element is dropped at D the disk shall return to the origin O.

grafik (Mockup)

What the real test shows (chromium and webkit):

grafik

The orange line does start at the origin O instead of A. Every line starts at the origin although the drop parameter was set to False. Obviously the drop=False state is not kept from one keyword call to the next. Movement from A to B, wanted (green) and real (red): grafik

interaction.py, line 943 ff:

self.mouse_button(MouseButtonAction.down, x=from_x, y=from_y)
self.mouse_move(x=to_x, y=to_y, steps=steps)
if drop:
    self.mouse_button(MouseButtonAction.up)

I can't see what happens when self.mouse_button is run. I just suppose that the first line releases the mouse button of a previous move. Maybe the desired behaviour is not reached by the drop argument.

Suggested solution

A solution might be to run the keyword with optional args for additional points between "from" and "to" coordinates:

from_x <float> 
from_y <float> 
to_x <float>
to_y <float> 
steps = 1<int>
by_x1 <float> 
by_y1 <float> 
by_x2 <float> 
by_y2 <float> 
...
by_xN <float> 
by_yN <float>  # where N is number of points between "from" and "to"

line 944 would be run in a loop for all the N points and the final point. The functionality of argument steps would not change. It would control the number of mouse move events on each of the N distances.

Snooz82 commented 2 years ago

@allcontributors please add @UliSei for userTesting.

allcontributors[bot] commented 2 years ago

@Snooz82

I've put up a pull request to add @UliSei! :tada:

UliSei commented 2 years ago

I am goint to implement the suggested solution. Variant 1.) Argument drop will be deleted then. Instead it is possible to call the keyword with an optional argument, a list of coordinates: [ [x1, y1], [x2, y2], ...[xN, yN] ].

Variant 2.) A data structure which includes the start and end point would be more generic: [ [from_x, from_y], [x1, y1], [x2, y2], ...[xN, yN], [to_x, to_y] ]. But then it would be necessary to change the calls of the keyword at any place where it is used so far.

Variant 1 is to be preferred?

Snooz82 commented 2 years ago

@UliSei Could you give us a test page? Or implement a test page with a virtual joystick first into our test web app?

then lets discuss what should be the expected behavior. ok?

Snooz82 commented 2 years ago

I would actually expect to use a different keyword sequence.

Drag And Drop By Coordinates    ${Ox}    ${Oy}    ${Ax}    ${Ay}    10    False
Mouse Move    ${Bx}    ${By}    10
Mouse Move    ${Cx}    ${Cy}    10
Mouse Move    ${Dx}    ${Dy}    10
Mouse Move    ${Ox}    ${Oy}    10
Mouse Button    up
Snooz82 commented 2 years ago

Therefore i would say, we have to discuss the expected behavior.

i personally am not a big fan of that argument to not lift the mouse button. Because then it is not a Drag&Drop it is just a Drag.

Snooz82 commented 2 years ago

Or even better to understand:

${cntr}=    Get Element    joystick_panel.container    # this is the main object that stays static
Hover    ${cntr}
Mouse Button    down
Mouse Move Relative To    ${cntr}    100     0
Mouse Move Relative To    ${cntr}    0     -100
Mouse Move Relative To    ${cntr}    -100     0
Mouse Move Relative To    ${cntr}    0     100
Hover    ${cntr}
Mouse Button    up

it would be much more maintainable, because the drag and drop coordinates are no relative coordinates but absolute ones on the page. And you should only use these with calculated values from the current state of the page. And i think a good calculation anker in this case is the center of the boundingBox of the joystick 🕹 element itself.

Snooz82 commented 2 years ago

The described issue itself is, that sending a mouse down event is only possible, if the mouse is not already down.

so if you send a mouse down event (on relative coordinate x100 y0) and then moves 100 pixel up and 100 pixel left, the joystick just moves from the center point 100up and 100left.

other joystick implementations it could be that the joystick in that case first snaps to x100 y0 and then follows you mouse.

The only thing imho we could do is to remember the button state of that keyword and depending on the state do the mouse down or not. But in that case it is again just a mouse move...

and that would imho be too much magic.

UliSei commented 2 years ago

Thanks a lot! So I managed to move the joystick by this sequence ...

    Mouse Move    ${centerX}    ${centerY}
    Mouse Button    down
    Mouse Move    ${centerRightX}    ${centerRightY}    ${STEPS}
    Mouse Move    ${centerUpX}       ${centerUpY}       ${STEPS}
    Mouse Move    ${centerLeftX}     ${centerLeftY}     ${STEPS}
    Mouse Move    ${centerDownX}     ${centerDownY}     ${STEPS}
    Mouse Move    ${centerX}         ${centerY}         ${STEPS}
    Mouse Button    up

... with the desired result: grafik

From my point of view the remaining task is just to reset the "Drag And Drop By Coordinates" keyword as it was before - without the drop argument. Do you agree?

Snooz82 commented 2 years ago

Did you do the drop argument to that keyword some weeks ago?

if so, and if you agree that it is not helpful because of these circumstances, then i agree and we could remove that argument again. In the hope no one else wanted to use it. 😁

UliSei commented 2 years ago

Yes :-) Since the argument can not be used for the purpose it was designed for it's useless and better to remove it. I am going to prepare a PR.

UliSei commented 2 years ago

Unfortunately there was one job of 23 where a timeout occurred after 30sec: https://github.com/MarketSquare/robotframework-browser/actions/runs/2339630527 grafik

Snooz82 commented 2 years ago

This is normal. Sadly sometimes. Due to slow resources in Github Actions

i retriggered the job

UliSei commented 2 years ago

Thanks @Snooz82 - the CI pipeline now ended successfully!