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.

Sign up for GitHub

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

Jump to bottom

Update Settings design: Add new SettingsListItem #5318

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Update Settings design: Add new SettingsListItem #5318

wants to merge 7 commits into from

Conversation

mikescamell
Copy link
Contributor

@mikescamell mikescamell commented Nov 26, 2024 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/1207908166761516/1208785622935231/f

Description

  • Adds all new Icons for Settings (not all are implemented)
  • Adds new SettingsListItem
    • Also added to the ADS screen
    • Designs
  • Tackles the privacy section without any copy changes which will be done in a later PR

Steps to test this PR

Pre-requisites:

  • newSettings feature flag must be enabled
  • Easier to start with a fresh launch
  • Don't enable DDG as default browser

Default Browser On/Off

  • Open New Settings screen
  • Default browser should be off ⚪
  • Press default browser
  • Select DDG
  • Press back to get back to the New Settings screen
  • Default browser should be on 🟢
  • Press default browser
  • Select another browser
  • Press back to get back to the New Settings screen
  • Default browser should be off ⚪

Private Search

  • Should be on and is always on

Web Tracking Protection

  • Should be on and is always on

Cookie Pop-Up Protection

  • Open New Settings screen
  • Cookie Pop-Up Protection should be on 🟢
  • Press Cookie Pop-Up Protection
  • Toggle off "Let DuckDuckGo handle cookie pop-ups"
  • Press back
  • Cookie Pop-Up Protection should be off ⚪
  • Press Cookie Pop-Up Protection
  • Toggle on "Let DuckDuckGo handle cookie pop-ups"
  • Press back
  • Cookie Pop-Up Protection should be on 🟢

App Tracking Protection

  • Open New Settings screen
  • App Tracking Protection should be off ⚪
  • Press App Tracking Protection
  • Toggle on App Tracking Protection
  • Press back
  • App Tracking Protection should be on 🟢
  • Press App Tracking Protection
  • Toggle off App Tracking Protection
  • Press back
  • App Tracking Protection should be off ⚪

Email Protection

  • Open New Settings screen
  • Email Protection should be off ⚪
  • Press Email Protection
  • Follow steps to turn on Email Protection
  • Open New Settings screen
  • Email Protection should be on 🟢
  • Press Email Protection
  • Sign out
  • Open New Settings screen
  • Email Protection should be off ⚪

UI changes

Before After
Screenshot_20241126_145126 Screenshot_20241126_145056
Screenshot_20241126_145134 Screenshot_20241126_145109
Screenshot_20241126_145502 Screenshot_20241126_145543

Demo

Screen_recording_20241126_145018.mp4

@mikescamell Graphite App
Copy link
Contributor Author

mikescamell commented Nov 26, 2024 •
edited
Loading

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

@mikescamell mikescamell mentioned this pull request Nov 26, 2024
Merged
32 tasks
@mikescamell mikescamell marked this pull request as ready for review November 26, 2024 11:23
@mikescamell mikescamell marked this pull request as draft November 26, 2024 11:26
@mikescamell mikescamell force-pushed the feature/mike/update-settings/add-new-setting-list-item branch 2 times, most recently from 8d2f2e6 to abf33e2 Compare November 26, 2024 14:30
@mikescamell mikescamell marked this pull request as ready for review November 26, 2024 18:13
Base automatically changed from feature/mike/update-settings/add-feature-flag to develop November 27, 2024 09:47
@mikescamell
duplicating content settings and privacy
57c5d5a
@mikescamell
add new icons
81a615b
@mikescamell
add new SettingsListItem
0a702a2
@mikescamell
switch to SettingsListItem in privacy section
628f8d0 
Also adding all the new icons, removing secondary text and itemState, and adding the new isOn for search and web tracking settings

Next commit we'll ensure the indicators are on/off
@mikescamell
set items to on/off based on state
bc4bb02 
I've removed appTrackingProtectionOnboardingShown state as only have 2 states now, on/off, but I will follow up and ask Thomas if this is expected
@mikescamell
add SettingsListItem to ADS
1a7d27c 
I think this is worth having on the ADS screen, even if not officially a component. Happy to move it if needs be.
@mikescamell
formatting
38b133a
@mikescamell mikescamell force-pushed the feature/mike/update-settings/add-new-setting-list-item branch from abf33e2 to 38b133a Compare November 27, 2024 16:18
@mikescamell
Copy link
Contributor Author

Android CI Test failing (not related to this work) but a fix has been raised: #5316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malmstein malmstein Awaiting requested review from malmstein malmstein is a code owner

@nalcalag nalcalag Awaiting requested review from nalcalag nalcalag is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@mikescamell