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

Sabrina/sync setup update #2198

Merged
merged 29 commits into from
Dec 1, 2023
Merged

Sabrina/sync setup update #2198

merged 29 commits into from
Dec 1, 2023

Conversation

SabrinaTardio
Copy link
Contributor

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

Description: Update the sync UI flow according to https://www.figma.com/file/5wIXwQLKJ0dZVsP5XWcgbT/%F0%9F%8E%AF-Sync-9?type=design&node-id=8748-306047&mode=design&t=5IygRcQcvvNAJ7xg-0

Steps to test this PR:

  1. Go to Sync Settings check UI and copy looks as expected

Sync This Device Flow

  1. Go to Sync Settings check UI and copy looks as expected
  2. Click on Sync and Back Up this device check “Sync and Back Up this device” sheet appears and design and copy are as expected
  3. Click on “Turn on Sync & Backup”
  4. Check preparing to sync sheet appears briefly before Save Recovery Code appears (check copy and UI)
  5. Check save as PDF and copy code work as expected
  6. Press Next and check Your Data is Synced! sheet appears.
  7. Press done and check sync enabled view appears (check looks as expected)
  8. Delete account

Recover Synced Data flow

  1. Click on Recover Synced Data and check Recover Synced Data sheets appear and looks as expected
  2. Click on Get Started and check the recover synced data camera mode appears
  3. Try to scan a recovery QR code or other device code and check the flow appears as in the previous case
  4. Try the same with Enter text Code manually

Sync with Another device flow

  1. Click on Sync with Another Device
  2. Check the Scan QR code camera view appears
  3. Try to point to a QR code of both a connected device and later an unconnected device and check the flow
  4. Same by manually inserting the code
  5. Same when scanning the QR code presented from another device
  6. Same when copying the text in another device.

Sync enabled View
Check sync with another device work as expected: the same ScanQR code view should appear and you should be able to connect a new device.

<!—
Before submitting a PR, please ensure you have tested the combinations you expect the reviewer to test, then delete configurations you know do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
—>

Copy Testing:

  • Use of correct apostrophes in new copy, ie ’ rather than ’

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16

Theme Testing:

  • Light theme
  • Dark theme

—

Internal references:

Software Engineering Expectations
Technical Design Template

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 27, 2023 •
edited
Loading

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 46f3b2e

@SabrinaTardio SabrinaTardio requested a review from brindy November 27, 2023 17:15
@SabrinaTardio SabrinaTardio marked this pull request as ready for review November 27, 2023 17:15
brindy
brindy requested changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments for you. I still need to do the actual testing.

DuckDuckGo/SyncSettingsViewController+PDFRendering.swift Outdated Show resolved Hide resolved
DuckDuckGo/SyncSettingsViewController.swift Outdated Show resolved Hide resolved
LocalPackages/SyncUI/Sources/SyncUI/Views/PasteCodeView.swift Outdated Show resolved Hide resolved
LocalPackages/SyncUI/Sources/SyncUI/Views/PreparingToSync.swift Outdated Show resolved Hide resolved
LocalPackages/SyncUI/Sources/SyncUI/Views/RecoverSyncedData.swift Outdated Show resolved Hide resolved
LocalPackages/SyncUI/Sources/SyncUI/Views/ScanOrEnterCodeToRecoverSyncedData.swift Outdated Show resolved Hide resolved
@brindy
Copy link
Contributor

brindy commented Nov 30, 2023

Still testing, but here's a few things:

1/ Alignment of text "Your data is end to end encrypted..." is wrong
Screenshot 2023-11-30 at 09 27 27

2/ Tap "Sync with another device" button:
* Tap manually enter the code
* Tap back - QR Code is wrong. Scanning this with another devices causes an assertion.
Simulator Screenshot - iPhone 15 Pro - 2023-11-30 at 09 43 03

3/ Tap "Sync with another device" button:
* Tap manually enter the code
* Paste a invalid code
* Bug: Shows "validating code" and doesn't move on (used to show an error)
Simulator Screenshot - iPhone 15 Pro - 2023-11-30 at 10 00 04

4/ Tap "Sync with another device" button:
* Tap manually enter the code
* Paste a valid code
* Bug: Shows "setting up sync and backup screen" "connecting..." and doesn't move on. Sliding it away shows that sync connected.
Simulator Screenshot - iPhone 15 Pro - 2023-11-30 at 09 40 31

@SabrinaTardio SabrinaTardio requested a review from brindy December 1, 2023 09:34
brindy
brindy approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 comment but otherwise LGTM

@SabrinaTardio SabrinaTardio merged commit 7c44458 into develop Dec 1, 2023
10 checks passed
@SabrinaTardio SabrinaTardio deleted the sabrina/sync-setup-update branch December 1, 2023 12:07
samsymons added a commit that referenced this pull request Dec 1, 2023
@samsymons
Merge branch 'develop' into graeme/remove-port-from-server-ip
30fce88 
# By Anh Do (1) and others
# Via GitHub
* develop:
  NetP: Fix list row colours (#2213)
  Reset VPN waitlist T&C (#2212)
  Report macOS active/new user for netP (#2207)
  Sabrina/sync setup update (#2198)
  new pixels for toolbars and share sheet  (#2208)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request Dec 4, 2023
@samsymons
Merge branch 'develop' into sam/netp-waitlist-final-touches
2ce9639 
# By Graeme Arthur (3) and others
# Via GitHub
* develop:
  NetP: Remove port from server address (#2214)
  NetP: Fix list row colours (#2213)
  Reset VPN waitlist T&C (#2212)
  Report macOS active/new user for netP (#2207)
  Sabrina/sync setup update (#2198)
  new pixels for toolbars and share sheet  (#2208)
  Use design system fonts throughout NetP (#2211)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request Dec 5, 2023
@samsymons
Merge branch 'develop' into jacek/xcode15
10ef86e 
* develop: (40 commits)
  Address Bar Spoofing Tests + Remediation (#2181)
  Update Sync e2e tests to fit the new UI (#2215)
  NetP waitlist final touches (#2209)
  NetP: Remove port from server address (#2214)
  NetP: Fix list row colours (#2213)
  Reset VPN waitlist T&C (#2212)
  Report macOS active/new user for netP (#2207)
  Sabrina/sync setup update (#2198)
  new pixels for toolbars and share sheet  (#2208)
  Use design system fonts throughout NetP (#2211)
  moving the toggle to the top of the dashboard (#2166)
  Allow automated fetching of synced bookmarks' favicons (#2163)
  NetP Geoswitching Design Review feedback (#2206)
  update theme to use system colours (#2180)
  Release 7.99.0 (#2205)
  Update iOS privacy defaults (#2185)
  Remove disabled switches from VPN Settings screen (#2203)
  Update BSK for VPN settings (#2165)
  Fix migrating from Bookmarks V2 and older (#2196)
  Autofill pixel parameter removed (#2182)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brindy brindy brindy approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@SabrinaTardio @brindy