-
Notifications
You must be signed in to change notification settings - Fork 913
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
Set report flow to form when launchiung hybrid site from the prompt #5324
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e95a1cd
to
d8bdb6c
Compare
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.
Left a couple of comments, but those are small nits. Looks good overall and works as expected.
@@ -96,7 +96,13 @@ class PrivacyDashboardHybridActivity : DuckDuckGoActivity() { | |||
onSubmitBrokenSiteReport = { payload -> | |||
val reportFlow = when (params) { |
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.
nit: you could assign params to local variable to avoid having to manually cast a few lines below
val reportFlow = when (params) { | |
val reportFlow = when (val params = params) { |
if ((params as BrokenSiteForm).reportFlow == BrokenSiteForm.BrokenSiteFormReportFlow.MENU) { | ||
ReportFlow.MENU | ||
} else { | ||
ReportFlow.PROMPT | ||
} |
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.
nit: You could map both values here explicitly, so that when someone adds new value to BrokenSiteForm.BrokenSiteFormReportFlow
enum, it won't get implicitly translated to ReportFlow.PROMPT
if ((params as BrokenSiteForm).reportFlow == BrokenSiteForm.BrokenSiteFormReportFlow.MENU) { | |
ReportFlow.MENU | |
} else { | |
ReportFlow.PROMPT | |
} | |
when (params.reportFlow) { | |
BrokenSiteForm.BrokenSiteFormReportFlow.MENU -> ReportFlow.MENU | |
BrokenSiteForm.BrokenSiteFormReportFlow.PROMPT -> ReportFlow.PROMPT | |
} |
46aa3e3
to
1c7dbfa
Compare
1c7dbfa
to
29e8164
Compare
Task/Issue URL: https://app.asana.com/0/1163321984198618/1208852264436742/f
Description
Set report flow to form (not menu) when launching hybrid site from the prompt
Rename pixels according to spec
Steps to test this PR
Feature 1
reportFlow
parameter onepbf
pixel isprompt
instead ofmenu
UI changes