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 default widget list for desktop #19708

Closed
simonhong opened this issue Nov 24, 2021 · 3 comments · Fixed by brave/brave-core#11266
Closed

Update default widget list for desktop #19708

simonhong opened this issue Nov 24, 2021 · 3 comments · Fixed by brave/brave-core#11266
Assignees
@simonhong
Labels
feature/widgets OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/include
Milestone
1.33.x - Release

Comments

@simonhong
Copy link
Member

simonhong commented Nov 24, 2021 •
edited by rebron
Loading

This is desktop part of #19478

  • For new users, all crypto widgets are hidden by default
    • User can control crypto widgets' visibility from gallery
  • For existing users, migrate current widget visibility
    • if current foremost widget is crypto widget, make it visible
    • if crypto widgets are not the foremost, make only authed widgets visible
    • After migration, user can control crypto widget's visibility from gallery
@simonhong simonhong self-assigned this Nov 24, 2021
@simonhong simonhong changed the title Hide crypto widgets by default Hide crypto widgets by default on desktop Nov 24, 2021
@simonhong simonhong mentioned this issue Nov 24, 2021
Closed
@simonhong simonhong mentioned this issue Nov 24, 2021
Merged
24 tasks
simonhong added a commit to brave/brave-core that referenced this issue Nov 24, 2021
@simonhong
Migrate crypto widgets visibility in NTP
da101b5 
fix brave/brave-browser#19708

Migration is done once in the first NTP loading.

For new users, all crypto widgets are hidden by default.
For existing users, if current foremost widget is crypto widget,
only it is visible and others are hidden.
After migration, user can control crypto widget's visibility from gallery.

To make all crypto widgets hidden by default, each pref's default
value is not changed. Their default prefs is still "true".
As their visibility are managed by prefs and widget order is managed in local
storage, it's difficult to start migration from c++. If default is changed
to false, it's very difficult to determine whether foremost widget is crypto
or not in the NTP webui.
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Nov 24, 2021
simonhong added a commit to brave/brave-core that referenced this issue Nov 25, 2021
@simonhong
Migrate crypto widgets visibility in NTP (#11266)
5b02b59 
* Migrate crypto widgets visibility in NTP

fix brave/brave-browser#19708

Migration is done once in the first NTP loading.

For new users, all crypto widgets are hidden by default.
For existing users, if current foremost widget is crypto widget,
only it is visible and others are hidden.
And, authed widgets are visible regardless of its position in widget stack
After migration, user can control crypto widget's visibility from gallery.

To make all crypto widgets hidden by default, each pref's default
value is not changed. Their default prefs is still "true".
As their visibility are managed by prefs and widget order is managed in local
storage, it's difficult to start migration from c++. If default is changed
to false, it's very difficult to determine whether foremost widget is crypto
or not in the NTP webui.
@simonhong simonhong added this to the 1.34.x - Nightly milestone Nov 25, 2021
simonhong added a commit to brave/brave-core that referenced this issue Nov 25, 2021
@simonhong
Migrate crypto widgets visibility in NTP (#11266)
8f4ef8e 
* Migrate crypto widgets visibility in NTP

fix brave/brave-browser#19708

Migration is done once in the first NTP loading.

For new users, all crypto widgets are hidden by default.
For existing users, if current foremost widget is crypto widget,
only it is visible and others are hidden.
And, authed widgets are visible regardless of its position in widget stack
After migration, user can control crypto widget's visibility from gallery.

To make all crypto widgets hidden by default, each pref's default
value is not changed. Their default prefs is still "true".
As their visibility are managed by prefs and widget order is managed in local
storage, it's difficult to start migration from c++. If default is changed
to false, it's very difficult to determine whether foremost widget is crypto
or not in the NTP webui.
@simonhong simonhong mentioned this issue Nov 25, 2021
Merged
7 tasks
@rebron rebron changed the title Hide crypto widgets by default on desktop Update default widget list for desktop Nov 29, 2021
@LaurenWags LaurenWags added OS/Android Fixes related to Android browser functionality QA/Test-All-Platforms feature/widgets labels Nov 29, 2021
@LaurenWags
Copy link
Member

Added labels that were on #19478 (duplicate issue)

@srirambv
Copy link
Contributor

Removing Android label as there is an Android specific issue #19546

@srirambv srirambv removed the OS/Android Fixes related to Android browser functionality label Nov 30, 2021
@srirambv
Copy link
Contributor

srirambv commented Dec 2, 2021

Verification passed on

Brave 1.33.94 Chromium: 96.0.4664.45 (Official Build) beta (64-bit)
Revision 76e4c1bb2ab4671b8beba3444e61c0f17584b2fc-refs/branch-heads/4664@{#947}
OS Linux
  • Verified test plan from Migrate crypto widgets visibility in NTP brave-core#11266
  • Verified on clean install only Brave rewards widget is shown
  • Verified all other widgets are disabled and can be enabled via dashboard settings
  • Verified enabled widgets are retained across browser relaunches
  • Verified on upgrade from previous version, only Brave rewards is shown if it was the foremost widget
  • Verified when a crypto widget(auth'd or not) is in foreground and rewards is the second one, post upgrade the same order is retained except all other widgets behind rewards is hidden
  • Verified when two crypto widgets are auth'd and rewards is third, the same order is retained post upgarde
  • Verified when rewards is the last widget, post upgrade all widgets are hidden and only rewards is shown
  • Verified post upgrade only rewards and Brave Talk widgets are retained when Talk is enabled on the older version
  • Verified if all widgets are hidden on older version, post upgrade none of them are enabled including rewards
  • Verified if rewards is disabled and Binance is the foremost widget, post upgrade only Binance widget is retained and rest all is hidden

Verification passed on

Brave 1.33.94 Chromium: 96.0.4664.45 (Official Build) beta (64-bit)
Revision 76e4c1bb2ab4671b8beba3444e61c0f17584b2fc-refs/branch-heads/4664@{#947}
OS Windows 11 Version Dev (Build 22509.1000)
  • Verified test plan from Migrate crypto widgets visibility in NTP brave-core#11266
  • Verified on clean install only Brave rewards widget is shown
  • Verified all other widgets are disabled and can be enabled via dashboard settings
  • Verified enabled widgets are retained across browser relaunches
  • Verified on upgrade from previous version, only Brave rewards is shown if it was the foremost widget
  • Verified when a crypto widget(auth'd or not) is in foreground and rewards is the second one, post upgrade the same order is retained except all other widgets behind rewards is hidden
  • Verified when two crypto widgets are auth'd and rewards is third, the same order is retained post upgarde
  • Verified when rewards is the last widget, post upgrade all widgets are hidden and only rewards is shown
  • Verified post upgrade only rewards and Brave Talk widgets are retained when Talk is enabled on the older version
  • Verified if all widgets are hidden on older version, post upgrade none of them are enabled including rewards
  • Verified if rewards is disabled and Binance is the foremost widget, post upgrade only Binance widget is retained and rest all is hidden

Verification passed on

Brave 1.33.94 Chromium: 96.0.4664.45 (Official Build) beta (arm64)
Revision 76e4c1bb2ab4671b8beba3444e61c0f17584b2fc-refs/branch-heads/4664@{#947}
OS macOS Version 12.0.1 (Build 21A559)
  • Verified test plan from Migrate crypto widgets visibility in NTP brave-core#11266
  • Verified on clean install only Brave rewards widget is shown
  • Verified all other widgets are disabled and can be enabled via dashboard settings
  • Verified enabled widgets are retained across browser relaunches
  • Verified on upgrade from previous version, only Brave rewards is shown if it was the foremost widget
  • Verified when a crypto widget(auth'd or not) is in foreground and rewards is the second one, post upgrade the same order is retained except all other widgets behind rewards is hidden
  • Verified when two crypto widgets are auth'd and rewards is third, the same order is retained post upgarde
  • Verified when rewards is the last widget, post upgrade all widgets are hidden and only rewards is shown
  • Verified post upgrade only rewards and Brave Talk widgets are retained when Talk is enabled on the older version
  • Verified if all widgets are hidden on older version, post upgrade none of them are enabled including rewards
  • Verified if rewards is disabled and Binance is the foremost widget, post upgrade only Binance widget is retained and rest all is hidden

@LaurenWags LaurenWags mentioned this issue Dec 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@simonhong simonhong

Labels
feature/widgets OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/include
Projects
None yet
Milestone
1.33.x - Release
Development

Successfully merging a pull request may close this issue.

Migrate crypto widgets visibility in NTP brave/brave-core
5 participants
@kjozwiak @rebron @simonhong @srirambv @LaurenWags