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

Hide sync option in settings if it's disabled #4572

Merged
merged 1 commit into from
Feb 14, 2020

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Feb 10, 2020

fix brave/brave-browser#8159

Submitter Checklist:

Test Plan:

See brave/brave-browser#8159

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@simonhong simonhong added this to the 1.6.x - Nightly milestone Feb 10, 2020
@simonhong simonhong requested a review from petemill February 10, 2020 08:19
@simonhong simonhong self-assigned this Feb 10, 2020
@simonhong simonhong force-pushed the hide_sync_menu_from_settings branch from ac61e43 to 7bad3da Compare February 10, 2020 08:29
@bsclifton bsclifton force-pushed the hide_sync_menu_from_settings branch from 7bad3da to b2ef143 Compare February 13, 2020 07:35
@simonhong simonhong requested a review from bsclifton February 13, 2020 08:02
@bsclifton bsclifton requested a review from darkdh February 13, 2020 22:31
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Changes LGTM (pending addressing feedback by @darkdh)

For others reading this, user would need to enable flag via brave://flags
Screen Shot 2020-02-13 at 3 49 33 PM

That would set the isSyncEnabled to true. I verified this works great locally 😄 👍

@simonhong simonhong force-pushed the hide_sync_menu_from_settings branch from b2ef143 to 8e5cfb1 Compare February 13, 2020 23:31
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

++

@simonhong
Copy link
Member Author

simonhong commented Feb 14, 2020

Got unrelated build error on Win64.
https://ci.brave.com/job/brave-browser-build-pr/job/hide_sync_menu_from_settings/4/execution/node/287/log/

09:38:25  In file included from ../../brave/browser/tor/tor_profile_service_impl.cc:14:
09:38:25  ../..\brave/browser/brave_browser_process_impl.h(14,10): fatal error: 'brave/components/brave_referrals/buildflags/buildflags.h' file not found
09:38:25  #include "brave/components/brave_referrals/buildflags/buildflags.h"
09:38:25           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
09:38:25  1 error generated.

Started to rebuild only on Win64.

@simonhong simonhong added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Feb 14, 2020
@simonhong
Copy link
Member Author

CI success on all platform.

@simonhong simonhong merged commit 1ffce92 into master Feb 14, 2020
@simonhong simonhong deleted the hide_sync_menu_from_settings branch February 14, 2020 04:33
brave-builds pushed a commit that referenced this pull request Feb 14, 2020
brave-builds pushed a commit that referenced this pull request Feb 14, 2020
@LaurenWags
Copy link
Member

Verified using

Brave 1.6.30 Chromium: 80.0.3987.106 (Official Build) nightly (64-bit)
Revision f68069574609230cf9b635cd784cfb1bf81bb53a-refs/branch-heads/3987@{#882}
OS macOS Version 10.14.6 (Build 18G103)

Verified STR from brave/brave-browser#8159

With sync flag set as Default/Disabled:
Screen Shot 2020-02-14 at 9 08 10 AM

With sync flag set as Enabled:
Screen Shot 2020-02-14 at 9 10 16 AM

@bbondy bbondy modified the milestones: 1.6.x - Beta, 1.7.x - Dev Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sync still shows as an option in settings
5 participants