Skip to content
This repository has been archived by the owner on Jan 30, 2025. It is now read-only.

Fix dblClick #1121

Merged
merged 4 commits into from
Dec 6, 2023
Merged

Fix dblClick #1121

merged 4 commits into from
Dec 6, 2023

Conversation

ankur22
Copy link
Collaborator

@ankur22 ankur22 commented Dec 5, 2023

What?

Fixes the behaviour of the dblClick API calls from locator, page, elementHandle and frame (they all share the same logic which is implemented in mouse).

Why?

When a dblClick API call is made, it needs to perform two things:

  1. Ensure a onDblClick event is initiated and any listeners receive the event. This was already part of the implementation.
  2. Ensure that double the mouse actions are performed so that an element is clicked on twice, e.g. a counter increments from 0 to 2. This was missing.

Checklist

  • I have performed a self-review of my code
  • I have added tests for my changes
  • I have commented on my code, particularly in hard-to-understand areas

Related PR(s)/Issue(s)

Playwright reference: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/input.ts#L199-L219

Updates: #469

The up function in mouse shouldn't override the mouse and clickCount
options. This is likely to have unwanted behaviour or missing behaviour
The behaviour of dblClick should comprise of:
1. Two pushes of a button which results in two clicks that can be
   recorded by a counter -- this behaviour was missing.
2. An onDblClick event which would result in any onDblClick listeners
   to be notified -- this behaviour already exists.

This fix ensures that the click up and down action is performed
multiple times based on the clickCount.
The mouse implementation of DblClick should work with the new fixed
click method. The old dblClick method only resolves one of the two
behaviours that are required (the counter will increase but the
onDblClick event will not fire).
This will help ensure that the DblClick behaviour matches the two
separate behaviours.
@ankur22 ankur22 requested review from inancgumus and ka3de December 5, 2023 12:32
@ankur22 ankur22 changed the title Fix/dbl click Fix dblClick Dec 5, 2023
Copy link
Member

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

Nice thinking of removing the double click and converting it into a click count option.

@ankur22 ankur22 merged commit 1ad8832 into main-next Dec 6, 2023
13 checks passed
@ankur22 ankur22 deleted the fix/dblClick branch December 6, 2023 11:26
@ankur22 ankur22 mentioned this pull request May 9, 2024
3 tasks
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants