Update show bookmarks setting #15577

spylogsster · 2022-10-20T20:24:22Z

Resolves brave/brave-browser#26072

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Check bookmark option in settings brave://settings/appearance, it should have menu with 3 items
Check bookmark option in the app menu on mac, View submenu, it should have menu with 3 items
Check bookmark option in the main browser menu, Bookmarks submenu, it should have menu with 3 items
Check context menu for bookmark bar, it should have menu with 3 items

brave-builds · 2022-10-20T21:57:33Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-10-21T09:35:04Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-10-21T13:25:38Z

A Storybook has been deployed to preview UI for the latest push

rebron · 2022-10-26T07:23:18Z

I'm going to update the issue. I think we should keep the existing default for new users which should be "only on the new tab page".

Also, we need to carry these options to the context menu when right clicking the bookmarks bar.

rebron · 2022-10-26T07:32:03Z

I updated the issue. My fault on the typo. Should be "Only on the new tab page"

spylogsster · 2022-10-26T08:18:46Z

Marked as draft to add selectors to some context menus as well

brave-builds · 2022-10-26T09:03:33Z

A Storybook has been deployed to preview UI for the latest push

petemill

Just some minor syntax changes required

browser/resources/settings/brave_appearance_page/bookmark_bar.ts

brave-builds · 2022-10-27T20:23:18Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2022-10-28T10:45:58Z

A Storybook has been deployed to preview UI for the latest push

goodov

lgtm. please rebase before merging.

brave-builds · 2023-01-16T15:20:29Z

A Storybook has been deployed to preview UI for the latest push

github-actions · 2023-01-16T16:34:16Z

⚠️ PR head is an unsigned commit
commit: 62230d152d447639570e356a736773268f1aff94
reason: unsigned
Please follow the handbook to configure commit signing
cc: @spylogsster

brave-builds · 2023-01-16T17:14:05Z

A Storybook has been deployed to preview UI for the latest push

iefremov · 2023-01-16T18:49:38Z

browser/ui/bookmark/BUILD.gn

@@ -32,3 +40,16 @@ source_set("bookmark") {
    "//ui/webui/resources/js/browser_command:mojo_bindings",
  ]
 }
+
+source_set("unittest") {


i'm a bit outdated, so don't really know what is the current convention - put tests into smaller BUILD.gn`s or continue stockpiling them in brave/test/BUILD.gn ?

something wrong here?

github-actions · 2023-01-16T18:49:50Z

⚠️ PR head is an unsigned commit
commit: 62230d152d447639570e356a736773268f1aff94
reason: unsigned
Please follow the handbook to configure commit signing
cc: @spylogsster

iefremov · 2023-01-16T18:58:11Z

browser/ui/toolbar/bookmark_bar_sub_menu_model.cc

+#include "brave/browser/ui/bookmark/bookmark_helper.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"


github-actions · 2023-01-16T18:58:23Z

⚠️ PR head is an unsigned commit
commit: 62230d152d447639570e356a736773268f1aff94
reason: unsigned
Please follow the handbook to configure commit signing
cc: @spylogsster

iefremov · 2023-01-16T19:02:36Z

browser/ui/toolbar/brave_bookmark_context_menu_controller.h

+#include <memory>
+#include <vector>
+
+#include "base/callback_forward.h"


pls check for unused includes

github-actions · 2023-01-16T19:02:48Z

⚠️ PR head is an unsigned commit
commit: 62230d152d447639570e356a736773268f1aff94
reason: unsigned
Please follow the handbook to configure commit signing
cc: @spylogsster

kjozwiak · 2023-01-16T21:59:37Z

Removing from 1.47.x as this hasn't landed on master. However, 1.47.x is already on the release channel. Please set the appropriate milestone once the above lands.

brave-builds · 2023-01-17T10:14:57Z

A Storybook has been deployed to preview UI for the latest push

github-actions · 2023-01-17T10:24:09Z

⚠️ PR head is an unsigned commit
commit: 06f5ddd36a17d73af1b02501855e5f5ad32045bb
reason: unsigned
Please follow the handbook to configure commit signing
cc: @spylogsster

brave-builds · 2023-01-17T11:05:56Z

A Storybook has been deployed to preview UI for the latest push

github-actions · 2023-01-17T11:40:21Z

⚠️ PR head is an unsigned commit
commit: 06f5ddd36a17d73af1b02501855e5f5ad32045bb
reason: unsigned
Please follow the handbook to configure commit signing
cc: @spylogsster

brave-builds · 2023-01-17T13:05:05Z

A Storybook has been deployed to preview UI for the latest push

github-actions · 2024-09-30T14:06:57Z

⚠️ PR head is an unsigned commit
commit: d630f45
reason: unknown_key
Please follow the handbook to configure commit signing
cc: @spylogsster

spylogsster requested review from petemill and bsclifton October 20, 2022 20:24

spylogsster self-assigned this Oct 20, 2022

github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Oct 20, 2022

spylogsster added this to the 1.47.x - Nightly milestone Oct 20, 2022

spylogsster force-pushed the brave-26072 branch from 61519bf to fc439b0 Compare October 21, 2022 08:49

spylogsster force-pushed the brave-26072 branch from fc439b0 to 4cb3922 Compare October 21, 2022 12:16

spylogsster force-pushed the brave-26072 branch from 4cb3922 to babd88f Compare October 26, 2022 07:54

spylogsster marked this pull request as draft October 26, 2022 08:18

petemill requested changes Oct 26, 2022

View reviewed changes

spylogsster force-pushed the brave-26072 branch 7 times, most recently from 43910f2 to 07333ca Compare October 27, 2022 15:55

spylogsster marked this pull request as ready for review October 27, 2022 19:22

spylogsster requested a review from a team as a code owner October 27, 2022 19:22

spylogsster force-pushed the brave-26072 branch from e3ef28e to 64ac8cc Compare October 27, 2022 19:22

spylogsster force-pushed the brave-26072 branch from 64ac8cc to e7f5b46 Compare October 28, 2022 10:07

goodov approved these changes Jan 16, 2023

View reviewed changes

spylogsster force-pushed the brave-26072 branch from 9910010 to be8cae0 Compare January 16, 2023 14:33

spylogsster force-pushed the brave-26072 branch from be8cae0 to 62230d1 Compare January 16, 2023 16:34

iefremov reviewed Jan 16, 2023

View reviewed changes

kjozwiak removed this from the 1.47.x - Release milestone Jan 16, 2023

spylogsster force-pushed the brave-26072 branch from 0cf8ba6 to 1f902b2 Compare January 17, 2023 09:25

spylogsster force-pushed the brave-26072 branch from 1f902b2 to 06f5ddd Compare January 17, 2023 10:23

iefremov approved these changes Jan 17, 2023

View reviewed changes

spylogsster force-pushed the brave-26072 branch from 06f5ddd to 80ab9d3 Compare January 17, 2023 11:52

Update show bookmarks setting

d630f45

spylogsster force-pushed the brave-26072 branch from 80ab9d3 to d630f45 Compare January 17, 2023 12:07

spylogsster merged commit b01395c into master Jan 17, 2023

spylogsster deleted the brave-26072 branch January 17, 2023 13:27

github-actions bot added this to the 1.49.x - Nightly milestone Jan 17, 2023

dgealow mentioned this pull request May 4, 2023

Ctrl+Shift+B changes settings unpredictably brave/brave-browser#29293

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update show bookmarks setting #15577

Update show bookmarks setting #15577

spylogsster commented Oct 20, 2022 •

edited

Loading

brave-builds commented Oct 20, 2022

brave-builds commented Oct 21, 2022

brave-builds commented Oct 21, 2022

rebron commented Oct 26, 2022 •

edited

Loading

rebron commented Oct 26, 2022

spylogsster commented Oct 26, 2022

brave-builds commented Oct 26, 2022

petemill left a comment

brave-builds commented Oct 27, 2022

brave-builds commented Oct 28, 2022

goodov left a comment

brave-builds commented Jan 16, 2023

github-actions bot commented Jan 16, 2023

brave-builds commented Jan 16, 2023

iefremov Jan 16, 2023

spylogsster Jan 17, 2023

github-actions bot commented Jan 16, 2023

iefremov Jan 16, 2023

spylogsster Jan 17, 2023

github-actions bot commented Jan 16, 2023

iefremov Jan 16, 2023

spylogsster Jan 17, 2023

github-actions bot commented Jan 16, 2023

kjozwiak commented Jan 16, 2023

brave-builds commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

brave-builds commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

brave-builds commented Jan 17, 2023

github-actions bot commented Sep 30, 2024

Update show bookmarks setting #15577

Update show bookmarks setting #15577

Conversation

spylogsster commented Oct 20, 2022 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

brave-builds commented Oct 20, 2022

brave-builds commented Oct 21, 2022

brave-builds commented Oct 21, 2022

rebron commented Oct 26, 2022 • edited Loading

rebron commented Oct 26, 2022

spylogsster commented Oct 26, 2022

brave-builds commented Oct 26, 2022

petemill left a comment

Choose a reason for hiding this comment

brave-builds commented Oct 27, 2022

brave-builds commented Oct 28, 2022

goodov left a comment

Choose a reason for hiding this comment

brave-builds commented Jan 16, 2023

github-actions bot commented Jan 16, 2023

brave-builds commented Jan 16, 2023

iefremov Jan 16, 2023

Choose a reason for hiding this comment

spylogsster Jan 17, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 16, 2023

iefremov Jan 16, 2023

Choose a reason for hiding this comment

spylogsster Jan 17, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 16, 2023

iefremov Jan 16, 2023

Choose a reason for hiding this comment

spylogsster Jan 17, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 16, 2023

kjozwiak commented Jan 16, 2023

brave-builds commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

brave-builds commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

brave-builds commented Jan 17, 2023

github-actions bot commented Sep 30, 2024

spylogsster commented Oct 20, 2022 •

edited

Loading

rebron commented Oct 26, 2022 •

edited

Loading