Fix double-click issue #306
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Note: Password rules outdatedYou can update the rules with the following command cd packages/password && npm run rules:updateOnce you've updated the rules, re-run the build from the root with |
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
GioSensation
commented
May 3, 2023
| * @returns {Promise<void>} | ||
| */ | ||
| async assertClickAndFocusMessages () { | ||
| async assertClickMessage () { |
There was a problem hiding this comment.
This stuff now only fires one message, either the click or focus, so I've updated the test cases.
GioSensation
commented
May 3, 2023
| @@ -187,6 +187,9 @@ export class HTMLTooltipUIController extends UIController { | |||
| e.preventDefault() | |||
| e.stopImmediatePropagation() | |||
|
|
|||
| const isMainMouseButton = e.button === 0 | |||
| if (!isMainMouseButton) return | |||
|
|
|||
There was a problem hiding this comment.
Unrelated, just makes it easier to open the devtools in debug builds.
GioSensation
commented
May 3, 2023
| @@ -75,15 +75,19 @@ export class OverlayUIController extends UIController { | |||
| }) | |||
| this._mutObs.observe(document.body, {childList: true, subtree: true}) | |||
|
|
|||
| const position = getPosition() | |||
There was a problem hiding this comment.
The position is now stored right away, so even if it changes after the click we already know the original position and the timeout below doesn't influence it.
GioSensation
commented
May 3, 2023
| input.scrollIntoView(true) | ||
| delay = 500 | ||
| } | ||
| this.#state = 'parentShown' |
There was a problem hiding this comment.
By setting the parentShown state right away we avoid multiple fires while the execution is suspended by the timeout.
alistairjcbrown
approved these changes
May 3, 2023
src/Form/Form.js
Outdated
| @@ -484,7 +485,7 @@ class Form { | |||
| storedClick.delete(input) | |||
| } | |||
|
|
|||
| if (this.shouldOpenTooltip(e, input)) { | |||
| if (this.shouldOpenTooltip(e, input) && isVisible(input)) { | |||
There was a problem hiding this comment.
Should the isVisible check on input be part of this.shouldOpenTooltip, or is there a reason we would want to open the tooltip for a non-visible input?
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
This was referenced May 4, 2023
This was referenced May 5, 2023
CDRussell
pushed a commit
to duckduckgo/Android
that referenced
this pull request
May 8, 2023
Task/Issue URL: https://app.asana.com/0/1204538113123459/1204538113123459 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/6.5.1 ## Description Updates Autofill to version [6.5.1](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/6.5.1). ### Autofill 6.5.1 release notes ## What's Changed * Disable pixel for all platforms but extension by @GioSensation in duckduckgo/duckduckgo-autofill#309 * Fix release script for BSK by @GioSensation in duckduckgo/duckduckgo-autofill#310 Included in 6.5.0: - Improve debugging by @GioSensation in duckduckgo/duckduckgo-autofill#293 - Reduce the calls on check position when there are many mutations by @alistairjcbrown in duckduckgo/duckduckgo-autofill#288 - Window release fetch tags before checkout by @alistairjcbrown in duckduckgo/duckduckgo-autofill#296 - Autofill compare generated test suites by @greyivy in duckduckgo/duckduckgo-autofill#283 - Update release scripts with latest Apple tooling by @GioSensation in duckduckgo/duckduckgo-autofill#299 - Add temporary incontext_eligible pixel by @GioSensation in duckduckgo/duckduckgo-autofill#304 - Fix double-click issue by @GioSensation in duckduckgo/duckduckgo-autofill#306 - Fix a whole bunch of existing failing test cases by @GioSensation in duckduckgo/duckduckgo-autofill#308 **Full Changelog**: duckduckgo/duckduckgo-autofill@6.5.0...6.5.1 ## Steps to test This release has been tested during autofill development. For smoke test steps see [this task](https://app.asana.com/0/1198964220583541/1200583647142330/f). Co-authored-by: GioSensation <GioSensation@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviewer: @alistairjcbrown
Asana: https://app.asana.com/0/0/1204520802024813/f
Description
Fixes a potential issue on desktop apps. More info on Asana.
Steps to test
See Asana task.