Final browser-laptop update

brave / browser-laptop

[DEPRECATED] Please see https://github.com/brave/brave-browser for the current version of Brave

https://www.brave.com

Other

7.95k stars 977 forks source link

Final browser-laptop update #15309

Closed bsclifton closed 5 years ago

bsclifton commented 5 years ago

The final update for browser-laptop

Fixes https://github.com/brave/browser-laptop/issues/15315

Test plan

See https://github.com/brave/brave-browser/issues/1536

diracdeltas commented 5 years ago

@bsclifton per discussion in slack, i would like to add some messaging/UI in this to help Linux users transition to the new repos (and also this should wait for https://github.com/brave/devops/issues/361).

diracdeltas commented 5 years ago

looks fine when i pretended my mac was linux to trigger the notification, except kind of incongruous that the token grant notification also pops up:

we probably want to either suppress it or prevent the old brave wallets from having any funds added to them?

diracdeltas commented 5 years ago

On Mac, the message looks great but I don't think the auto-install of b-c actually worked. I see this in the console:

Extracting brave-core binaries from "/Users/yan/repos/browser-laptop/node_modules/electron-prebuilt/dist/Brave.app/Contents/Resources/Brave-Browser.pkg" into temp directory "/var/folders/hl/761qjwzn4qb7slh2yvlgwfxr0000gn/T/brave-upgrade"
Could not open product archive: /Users/yan/repos/browser-laptop/node_modules/electron-prebuilt/dist/Brave.app/Contents/Resources/Brave-Browser.pkg
Removing temp directory "/var/folders/hl/761qjwzn4qb7slh2yvlgwfxr0000gn/T/brave-upgrade"

diracdeltas commented 5 years ago

/Users/yan/repos/browser-laptop/node_modules/electron-prebuilt/dist/Brave.app/Contents/Resources/Brave-Browser.pkg does not exist btw

bsclifton commented 5 years ago

@diracdeltas for the auto-install to work, you have to run packaging; I'm about to queue a build for Nightly channel if you're interested in trying it out? 😄

bsclifton commented 5 years ago

The only known issue that isn't fixed is captured with https://github.com/brave/brave-browser/issues/1914

Still looking through that... but I rebased this so that each commit is very meaningful. Latest is also pushed to Nightly channel for macOS + Windows

Ready for review!

diracdeltas commented 5 years ago

@bsclifton please ping us for a final security review when this is ready to merge, thanks

bsclifton commented 5 years ago

@diracdeltas @rebron in the latest commit, I basically disabled sync

deprecation notice shown
you can restore already existing groups (so that you can clear, if you'd like)
existing sync groups will show and can be cleared

Here's what that looks like. Basically, all controls are disabled except the clear

If you've never used sync:

If you already have a sync group:

bsclifton commented 5 years ago

@diracdeltas feedback addressed in https://github.com/brave/browser-laptop/pull/15309/commits/3f06a9998d77947f12680c18b620d1eb3fe8b10c 😄