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

Treat adding built-in items to sidebar differently than other pages #21124

Closed
simonhong opened this issue Feb 16, 2022 · 1 comment · Fixed by brave/brave-core#12311
Closed

Treat adding built-in items to sidebar differently than other pages #21124

simonhong opened this issue Feb 16, 2022 · 1 comment · Fixed by brave/brave-core#12311
Assignees
@simonhong
Labels
feature/sidebar Relating to Brave's Sidebar feature OS/Desktop QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/exclude
Milestone
1.37.x - Release

Comments

@simonhong
Copy link
Member

simonhong commented Feb 16, 2022 •
edited
Loading

When bookmarks manager is loaded in active tab, user can add it as a shortcut in sidebar.
But, sidebar provides bookmarks panel as a default items.
So, we should add bookmarks builtin item instead of shortcut if it's removed.
This can apply also to all other builtin items.(talk, wallet).

STR:

  1. Remove brave talk builtin item
  2. Load brave talk in active tab
  3. Try to add current tab(talk) via + dialog

Actual result - add item bubble shows two entries (one for current active tab and removed builtin item)
Screenshot 2022-02-17 101730

Expected result - add item bubble should show one entry (builtin talk item)
Screenshot 2022-02-17 101416
Also add item button should be disabled when builtin sidebar item is already used.
image
@simonhong simonhong self-assigned this Feb 16, 2022
@simonhong simonhong mentioned this issue Feb 17, 2022
Merged
25 tasks
simonhong added a commit to brave/brave-core that referenced this issue Feb 17, 2022
@simonhong
Don't add as a separate shortcut when bulitin provide related item
30326ff 
fix brave/brave-browser#21124

When bookmarks manager is loaded in active tab, user can add it
as a shortcut in sidebar. However, sidebar provides bookmarks panel
as a default items. So, we should add bookmarks builtin item
instead of shortcut if it's removed.
This can apply also to all other builtin items.(talk, wallet).
@bsclifton bsclifton changed the title Don't add as a separate item when bulitin items provide relavant item Treat adding built-in items differently than other pages Feb 22, 2022
@bsclifton bsclifton changed the title Treat adding built-in items differently than other pages Treat adding built-in items to sidebar differently than other pages Feb 22, 2022
simonhong added a commit to brave/brave-core that referenced this issue Feb 22, 2022
@simonhong
Don't add as a separate shortcut when bulitin provide related item
1ef6dbb 
fix brave/brave-browser#21124

When bookmarks manager is loaded in active tab, user can add it
as a shortcut in sidebar. However, sidebar provides bookmarks panel
as a default items. So, we should add bookmarks builtin item
instead of shortcut if it's removed.
This can apply also to all other builtin items.(talk, wallet).
@simonhong simonhong added this to the 1.37.x - Nightly milestone Feb 22, 2022
@stephendonner
Copy link

stephendonner commented Feb 22, 2022 •
edited by btlechowski
Loading

Verified PASSED using

Brave 1.37.65 Chromium: 99.0.4844.35 (Official Build) nightly (64-bit)
Revision f60a827ddb87f1c403e07713751a5551d5856ac0-refs/branch-heads/4844@{#579}
OS Windows 10 Version 21H2 (Build 19044.1566)

Steps:

  1. installed 1.37.65
  2. launched Brave
  3. loaded talk.brave.com
  4. hovered over the + icon

Confirmed no Add [Brave Talk] to Sidebar popup

21124-1

Case: remove/add Brave Talk icon

  1. installed 1.37.65
  2. launched Brave
  3. context-clicked on the Brave Talk icon
  4. clicked on Remove
  5. loaded talk.brave.com
  6. hovered over + button
  7. confirmed I was offered to add Brave Talk
  8. clicked to add Brave Talk

Confirmed the Brave Talk icon was added

example example
21124-3 21124-4

Case: Add history

  1. installed 1.37.65
  2. launched Brave
  3. loaded brave://history
  4. hovered over the + icon in Sidebar
  5. confirmed I saw Add history to sidebar dialog
  6. confirmed the + icon is disabled once history was added to Sidebar

21124-6

Case: no prompt for built-in Bookmarks

  1. installed 1.37.65
  2. launched Brave
  3. loaded brave://bookmarks
  4. hovered over the + icon

Confirmed no Add [bookmarks] to Sidebar popup

21124-5

Verified PASSED using

Brave 1.37.91 Chromium: 99.0.4844.51 (Official Build) beta (x86_64)
Revision d537ec02474b5afe23684e7963d538896c63ac77-refs/branch-heads/4844@{#875}
OS macOS Version 12.3 (Build 21E230)

Steps:

  1. installed 1.37.91
  2. launched Brave
  3. loaded talk.brave.com
  4. hovered over the + icon

Confirmed no Add [Brave Talk] to Sidebar popup

Screen Shot 2022-03-14 at 7 32 03 PM

Case: remove/add Brave Talk icon

  1. installed 1.37.91
  2. launched Brave
  3. context-clicked on the Brave Talk icon
  4. clicked on Remove
  5. loaded talk.brave.com
  6. hovered over + button
  7. confirmed I was offered to add Brave Talk
  8. clicked to add Brave Talk

Confirmed the Brave Talk icon was added

example example
Screen Shot 2022-03-14 at 7 33 08 PM Screen Shot 2022-03-14 at 7 33 14 PM

Case: Add history

  1. installed 1.37.91
  2. launched Brave
  3. loaded brave://history
  4. hovered over the + icon in Sidebar
  5. confirmed I saw Add history to sidebar dialog
  6. confirmed the + icon is disabled once history was added to Sidebar
example example example
Screen Shot 2022-03-14 at 7 34 00 PM Screen Shot 2022-03-14 at 7 34 02 PM Screen Shot 2022-03-14 at 7 34 08 PM

Case: no prompt for built-in Bookmarks

  1. installed 1.37.91
  2. launched Brave
  3. loaded brave://bookmarks
  4. hovered over the + icon

Confirmed no Add [bookmarks] to Sidebar popup

Screen Shot 2022-03-14 at 7 34 29 PM

Verification passed on

Brave 1.37.95 Chromium: 99.0.4844.74 (Official Build) beta (64-bit)
Revision fee9a47e86e981802390cb0d41c5ed7ea93c4f6f-refs/branch-heads/4844@{#1060}
OS Ubuntu 18.04 LTS

Steps:

  1. installed 1.37.95
  2. launched Brave
  3. loaded talk.brave.com
  4. hovered over the + icon

Confirmed no Add [Brave Talk] to Sidebar popup

image

Case: remove/add Brave Talk icon

  1. installed 1.37.95
  2. launched Brave
  3. context-clicked on the Brave Talk icon
  4. clicked on Remove
  5. loaded talk.brave.com
  6. hovered over + button
  7. confirmed I was offered to add Brave Talk
  8. clicked to add Brave Talk

Confirmed the Brave Talk icon was added
image
image

Case: Add history

  1. installed 1.37.95
  2. launched Brave
  3. loaded brave://history
  4. hovered over the + icon in Sidebar
  5. confirmed I saw Add history to sidebar dialog
  6. confirmed the + icon is disabled once history was added to Sidebar

image
image

Case: no prompt for built-in Bookmarks

  1. installed 1.37.95
  2. launched Brave
  3. loaded brave://bookmarks
  4. hovered over the + icon

Confirmed no Add [bookmarks] to Sidebar popup

image

@simonhong simonhong mentioned this issue Mar 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@simonhong simonhong

Labels
feature/sidebar Relating to Brave's Sidebar feature OS/Desktop QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.37.x - Release
Development

Successfully merging a pull request may close this issue.

Don't add as a separate shortcut when built-in provide related item brave/brave-core
4 participants
@stephendonner @simonhong @LaurenWags @btlechowski