Skip to content
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

Don't open in-context signup on input click #264

Conversation

alistairjcbrown
Copy link
Member

Reviewer: @shakyShane @GioSensation
Asana: https://app.asana.com/0/0/1204038361216229/f

Description

Use in-context signup isAvailable check to prevent tooltip from opening on input click

Steps to test

  1. Build into extension
  2. Go to https://duckduckgo.com/newsletter
  3. Click into input -- confirm tooltip does not show
  4. Click Dax icon -- confirm tooltip does show

Copy link
Member

@GioSensation GioSensation left a comment

Choose a reason for hiding this comment

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

Just a minor suggestion, but didn't have the time to test. Please test thoroughly.

src/InContextSignup.js Outdated Show resolved Hide resolved
@alistairjcbrown alistairjcbrown force-pushed the abrown/prevent-incontext-signup-on-local-pages branch from a5b808e to d6618af Compare February 27, 2023 15:46
@alistairjcbrown alistairjcbrown force-pushed the abrown/dont-open-incontext-signup-on-input-click branch from fa1bda9 to 78d50fa Compare February 27, 2023 18:42
@alistairjcbrown alistairjcbrown force-pushed the abrown/prevent-incontext-signup-on-local-pages branch from d6618af to 361d923 Compare February 28, 2023 12:31
@alistairjcbrown alistairjcbrown force-pushed the abrown/dont-open-incontext-signup-on-input-click branch from 78d50fa to 1c4c171 Compare February 28, 2023 12:32
@alistairjcbrown alistairjcbrown force-pushed the abrown/prevent-incontext-signup-on-local-pages branch from 361d923 to 813653f Compare February 28, 2023 12:37
Base automatically changed from abrown/prevent-incontext-signup-on-local-pages to abrown/changes-to-in-context-signup-treatment February 28, 2023 20:04
@alistairjcbrown alistairjcbrown force-pushed the abrown/dont-open-incontext-signup-on-input-click branch from 1c4c171 to ba3106b Compare February 28, 2023 20:05
@alistairjcbrown alistairjcbrown merged commit 67eec26 into abrown/changes-to-in-context-signup-treatment Feb 28, 2023
@alistairjcbrown alistairjcbrown deleted the abrown/dont-open-incontext-signup-on-input-click branch February 28, 2023 20:11
alistairjcbrown added a commit that referenced this pull request Mar 23, 2023
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants