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

Update Settings design: Add feature flag & new Settings screen #5293

Closed

Conversation

mikescamell
Copy link
Contributor

@mikescamell mikescamell commented Nov 20, 2024

Task/Issue URL: https://app.asana.com/0/488551667048375/1208785611228225/f

Description

Adds a feature flag and new screen for the new updated Settings screen designs.

For now it's the same layout as before, this is just laying the groundwork for the rest of the implementation.

So you can see that the flag is working, the new Settings screen has a temporary title of "New Settings" which will be removed when the UI changes enough that it's perceivable.

I also added the ability to trigger notifications from dev settings so it's easier to test the ClearData notification route to opening settings.

Steps to test this PR

  • Turn on the feature flag "updatedSettings"

Opening Settings from the Browser menu

  • Click overflow menu
  • Press Settings button
  • Ensure "New Settings" is displayed at the top

Opening Settings from ADS shortcut

  • Add the ADS shortcut via phone homescreen
  • Press Shortcut
  • ADS screen should open
  • Press back
  • Ensure "New Settings" is displayed at the top

Clear Data Notification

  • Open new Notifications screen in Dev Settings
  • Click "Data clearing set to manual"
  • Click on the notification
  • Ensure "New Settings" is displayed at the top

VPN Disabled Notification

  • Open new Notifications screen in Dev Settings
  • Click "NetP Disabled"
  • Click on the notification
  • Ensure "New Settings" is displayed at the top

Clear Data Notification

  • Open new Notifications screen in Dev Settings
  • Click "Data clearing set to manual"
  • Click on the notification
  • Ensure "New Settings" is displayed at the top

New Tab Page

  • Ensure "pluginNewTabPage" feature flag is enabled
  • Open a new tab
  • Click the "Settings" shortcut
  • Ensure "New Settings" is displayed at the top

Tab Switcher

  • Open Tab Switcher screen
  • Click overflow menu
  • Click "Settings"
  • Ensure "New Settings" is displayed at the top

Privacy Pro

  • Simplest way to see it is to edit ln 126 in SpecialUrlDetector to true and load a web page
  • Press back
  • New settings should be visible

UI changes

N/A

Note we've also duplicated the xml layout and are now referring to it's binding in the Activity

I used a new title of "New Settings" for the activity so it's easy to differentiate and notice if you're using the flagged version. I will remove this once we have the new UI for the items and it's more obvious it's the updated version
NotificationsActivity in the next commit
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

You can see in the viewmodel that I had some issues with the SurveyAvailableNotification

1. The SurveyAvailableNotification needs a notification in the database, otherwise we get a NoElement exception as we call "last()" on the list of surveys when getting them in the intent
2. The intent hits a repo that hits a DAO, that is not a suspend function, so we have to be off the main thread

Next we'll add VPN notifications
@mikescamell mikescamell changed the title Update Settings design: Add feature flag Update Settings design: Add feature flag & new Settings screen Nov 21, 2024
@mikescamell mikescamell marked this pull request as ready for review November 21, 2024 09:24
@mikescamell mikescamell force-pushed the feature/mike/update-settings/add-feature-flag branch from 16d4729 to bebaf52 Compare November 21, 2024 09:27
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.

1 participant