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
Fire pixel when autoprompt is dismissed #124
Conversation
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Sources/BrowserServicesKit/Autofill/AutofillUserScript+SecureVault.swift
Outdated
Show resolved
Hide resolved
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>
…ema/autoprompt-pixel
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
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.
LGTM. Code looks fine and I've tested this via the client-side PRs.
@samsymons Did you manage to look into the failing tests? |
@GioSensation Yep, I've just pushed a commit to resolve the legit failure on this branch. There are some unrelated failures which can happen on macOS 12, we know about those and suspect they're an OS bug. The issue with the legit failure was that the test suite was treating an asynchronous test as synchronous, and was completing too early, before it could set the values that the test itself was checking for. I've updated it to wait for a predicate expectation and to track which callbacks it receives – works better now. 🙂 |
Awesome, thanks Sam! 🚀 |
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/0/1202674424656875/f
iOS PR: duckduckgo/iOS#1262
macOS PR: https://github.com/more-duckduckgo-org/macos-browser/pull/669
What kind of version bump will this require?: Major (because it requires native updates to work properly)
Description:
This fires a pixel when autoprompt is dismissed.
Steps to test this PR:
Pixel fired on iOS
m_autofill_logins_autoprompt_dismissed
.No pixel on iOS
m_autofill_logins_autoprompt_dismissed
.No pixel on macOS
m_autofill_logins_autoprompt_dismissed
.OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template