-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Move BrowserPopupMenu to Browser-ui #7117
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
Move BrowserPopupMenu to Browser-ui #7117
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Task/Issue URL: #7118 ### Description Translations for new Duck.ai Menu --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211921638789395
Task/Issue URL: https://app.asana.com/1/137249556945/project/1174433894299346/task/1211821284193236?focus=true ### Description This PR adds the new Duck.ai Menu ### Steps to test this PR _Duck.ai Menu_ - [x] Open app and enable fullscreen mode Duck.ai - [x] Open a new chat - [x] Open the Browser Menu - [x] Verify new menu is visible --------- Co-authored-by: Dax The Translator <daxmobile@duckduckgo.com>
app/src/androidTest/java/com/duckduckgo/espresso/privacy/SurrogatesTest.kt
Outdated
Show resolved
Hide resolved
| import com.duckduckgo.common.ui.view.gone | ||
| import com.duckduckgo.mobile.android.R.drawable | ||
|
|
||
| class BrowserMenu( |
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.
There’s a lot to review here, I noticed that !displayedInCustomTabScreen is no longer checked. Just making sure that that is intended?
| vpnMenuItem.isVisible = false | ||
| } | ||
| VpnMenuState.NotSubscribed -> { | ||
| vpnMenuItem.isVisible = 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.
As well as !browserShowing is not used anymore, these are now true
joshliebe
left a comment
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.
Works as expected, LGTM 🚀




Task/Issue URL: https://app.asana.com/1/137249556945/project/1174433894299346/task/1211921638789392?focus=true
Description
This PR moves the BrowserMenu to the browser-ui module for ease of use
Lint is going to complain because of strings, this will be fixed on a stacked PR
Steps to test this PR
Browser mode
New Tab mode
Custom Tabs
Malicious sites