-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ema/prevent focus extensions #270
Ema/prevent focus extensions #270
Conversation
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
@shakyShane I haven't added tests yet, but I'd appreciate a first pass. Please advise if you can think of how to test the Dax click thingy. I was thinking of just specifying click coords in the Playwright tests, but if you have a simpler idea I'm all ears 👂🙂👂. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work :)
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
…b.com:duckduckgo/duckduckgo-autofill into ema/prevent-focus-extensions Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> # Conflicts: # dist/autofill-debug.js # dist/autofill.js # swift-package/Resources/assets/autofill-debug.js # swift-package/Resources/assets/autofill.js
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
@@ -45,6 +45,7 @@ function createDevice () { | |||
} | |||
return new AppleDeviceInterface(globalConfig, deviceApi, settings) | |||
} | |||
globalConfig.isExtension = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up setting it here. All other similar properties are in the globalConfig
so it makes sense, but extensions don't inject strings on the fly so we can do it like this here. Any egregious downside?
FYI, using instanceof ExtensionInterface
couldn't be used because imports were not failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say for now this is fine given how we're determining that we're in the extension - as you can imagine there are other ways to be more deterministic, but given what we have currently I don't think this is an issue at all :)
* Prevent in-context signup showing on local or invalid domain pages (#263) * Don't open in-context signup on input click (#264) * Show grayscale Dax icon for in-context signup (#265) * Remove initial dismissal in favor of single permanent dismissal (#266) * Only show in-context signup if recently installed (#267) * Adds a close X button to the top right of the tooltip (#268) * Fix grayscale dax logo (#271) * Ema/prevent focus extensions (#270) * Update icon asset to fix grayscale version (#276) * Improve narrow width iframes (#273) * Add Email Protection tooltip pixel (#277) * Fade Dax icon in on hover for in-context signup (#278) * Fade Dax icon in on hover for in-context signup * Keep Dax icon with active styles when tooltip open * Prevent icon flicker when clicking Dax icon for open tooltip * Move style management into form * Position tooltip above input when no space below (#282) * Position tooltip above input when no space below * Preload webfonts before showing tooltip This prevents the fonts loading in once the tooltip is shown and causing a layout shift. This is more important when the tooltip is showing above the input, as it can cause the tooltip to jump up. * Check tooltip position after revealed and bail if called before When running on slower devices, the tooltip position may be checked before the tooltip is revealed. When this happens, we now bail early without updating the position. Check position is now called after styles have loaded, when we know the tooltip is visible. * Hide caret initially and reduce position calculation calls * Read hidden attribute from DOM Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Fix linting issue with playwright report Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Rename typoed file Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> --------- Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> --------- Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Reviewer: @shakyShane
Asana: https://app.asana.com/0/1198964220583541/1204088031977626/f
Description
This prevents the focus event when clicking on Dax on extensions to avoid overlap with Chrome's native tooltips. It also fixes a couple of related bugs.
Steps to test
I'll add tests this week. Please advise if you can think of how to test the Dax click thingy. I was thinking of just specifying click coords in the Playwright tests, but if you have a simpler idea I'm all ears 👂🙂👂.