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

Modify Brave Sync UI to allow one device in chain #6941

Closed
AlexeyBarabash opened this issue Nov 15, 2019 · 8 comments · Fixed by brave/brave-core#4239
Closed

Modify Brave Sync UI to allow one device in chain #6941

AlexeyBarabash opened this issue Nov 15, 2019 · 8 comments · Fixed by brave/brave-core#4239

Comments

@AlexeyBarabash
Copy link
Contributor

AlexeyBarabash commented Nov 15, 2019

Description

This issue is an required addition for PR brave/brave-core#3988 .

General notes:
Sync should allow to have just one device in the chain.

This will help to avoid the situation mentioned in #6504. Or with these hypothetical STRs:

1. enable sync on desktop
2. get busy doing something else
3. enable sync on mobile
4. shut down browser and go home
5. mobile never gets the data from the desktop because it was waiting for a second device to start syncing
6. User is angry because sync doesn't work as a user you would think that since you had enabled sync on the desktop a long time ago it should have synced everything already and then you can just connect your mobile device and get that data . 

**Required changes: **

  1. If brave://sync page is loading/reloading/sync state changed, and sync is enabled, but there are less than two devices, SyncUI page should show View Sync Code dialog.
  2. If View Sync Code is being shown, but there are less than two devices, then when the dialog is closed by cancel, SyncUI should not call Reset, and should just show the regular devices list screen and buttons.

@bradleyrichter, @darkdh , @bridiver could you please review the requested changes?

@cezaraugusto, you should work in branch of https://github.com/brave/brave-core/tree/sync_prefs_redo_issue_6504 , because it contains the service side changes to allow to have less than two devices in chain. The commit Changes in sync UI to allow test of service - subject to be redone - should be removed because it is just my attempt to allow page work when < 2 devices, so it is rough and not correct.

@darkdh
Copy link
Member

darkdh commented Nov 20, 2019

  1. Show this dialog every time brave://sync if there is only one device in chain to remind user to add second or more devices

Screen Shot 2019-11-20 at 14 45 24

2. Do not automatically reset sync chain unless "Leave sync chain" is clicked. That means remove this dialog when "Cancel" is clicked

Screen Shot 2019-11-20 at 14 48 42 1

Screen Shot 2019-11-20 at 14 49 46

@AlexeyBarabash
Copy link
Contributor Author

Since brave/brave-core#3988 had been merged into master, work on this issue should be done on a top of brave-core master.

@btlechowski
Copy link

#6941 (comment) the PR was merged to Beta.
This has to be fixed really soon as it brakes beta (1.3.x).

Steps to Reproduce

  1. Clean profile
  2. Open brave://sync
  3. Click Start a new Sync Chain
  4. Wait 1 minute

Actual result:

During initial sync creation "Choose device type" is automatically closed.
The sync chain is created with only one device.
sync_chain_fail

Note: You can still add devices after the sync was created

Brave 1.3.85 Chromium: 79.0.3945.88 (Official Build) beta (64-bit)
Revision c2a58a36b9411c80829b4b154bfcab97e581f1f3-refs/branch-heads/3945@{#954}
OS Ubuntu 18.04 LTS
Brave 1.3.85 Chromium: 79.0.3945.88 (Official Build) beta (64-bit)
Revision c2a58a36b9411c80829b4b154bfcab97e581f1f3-refs/branch-heads/3945@{#954}
OS Windows 7 Service Pack 1 (Build 7601.24530)

cc @rebron @bsclifton @brave/legacy_qa

@rebron
Copy link
Collaborator

rebron commented Jan 10, 2020

@cezaraugusto Is your pr solving this completely or are you stuck in review?

@rebron rebron added priority/P3 The next thing for us to work on. It'll ride the trains. priority/P2 A bad problem. We might uplift this to the next planned release. and removed priority/P3 The next thing for us to work on. It'll ride the trains. labels Jan 10, 2020
cezaraugusto added a commit to brave/brave-core that referenced this issue Jan 16, 2020
@cezaraugusto
Copy link
Contributor

@btlechowski thanks for reporting. on it right now.

@rebron yes there is a PR open for this but @AlexeyBarabash pointed me out that additional work was required, which I'm working on at this very moment.

@cezaraugusto
Copy link
Contributor

milestone changed to match commit channel

@btlechowski
Copy link

btlechowski commented Mar 3, 2020

Verification passed on

Brave 1.5.106 Chromium: 80.0.3987.122 (Official Build) beta (64-bit)
Revision cf72c4c4f7db75bc3da689cd76513962d31c7b52-refs/branch-heads/3987@{#943}
OS Ubuntu 18.04 LTS

Verified clicking Start a new Sync Chain creates sync chain.
Verified clicking Start a new Sync Chain shows Choose Device Type
Verified that Choose Device Type modal is always shown after restart when there is only one device in the chain.
Verified Sync Chain QR Code is shown after choosing Phone/Tablet
Verified Sync Chain Code is shown after choosing Computer
Verified ability to cancel adding device through QR Code
Verified ability to cancel adding device through Code Words
Verified ability to create sync through code words
Verified ability to create sync through QR code
Verified leaving sync chain by clicking Leave sync chain works
Verified leaving sync chain by clicking x next to the device's name works
Verified ability to add additional devices after sync creation

Verification passed on

Brave 1.5.108 Chromium: 80.0.3987.132 (Official Build) beta (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Verified clicking Start a new Sync Chain creates sync chain.
Verified clicking Start a new Sync Chain shows Choose Device Type
Verified that Choose Device Type modal is always shown after restart when there is only one device in the chain.
Verified Sync Chain QR Code is shown after choosing Phone/Tablet
Verified Sync Chain Code is shown after choosing Computer
Verified ability to cancel adding device through QR Code
Verified ability to cancel adding device through Code Words
Verified ability to create sync through code words
Verified ability to create sync through QR code
Verified leaving sync chain by clicking Leave sync chain works
Verified leaving sync chain by clicking x next to the device's name works
Verified ability to add additional devices after sync creation

Verification passed with

Brave 1.5.109 Chromium: 80.0.3987.132 (Official Build) beta (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS macOS Version 10.14.6 (Build 18G3020)

Verified clicking Start a new Sync Chain creates sync chain.
Verified clicking Start a new Sync Chain shows Choose Device Type
Verified that Choose Device Type modal is always shown after restart when there is only one device in the chain.
Verified Sync Chain QR Code is shown after choosing Phone/Tablet
Verified Sync Chain Code is shown after choosing Computer
Verified ability to cancel adding device through QR Code
Verified ability to cancel adding device through Code Words
Verified ability to create sync through code words
Verified ability to create sync through QR code
Verified leaving sync chain by clicking Leave sync chain works
Verified leaving sync chain by clicking x next to the device's name works
Verified ability to add additional devices after sync creation

Encountered #8314 while testing.

@LaurenWags
Copy link
Member

Marked as release-notes/exclude as per discussion with @btlechowski this was actually implemented with 1.3.x but this issue was not included in that milestone. cc @kjozwiak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants