Added accelerator for toggle sidebar

fix brave/brave-browser#27994 Ctrl+B(Cmd+B on macOS) toggles sidebar.
brave · Feb 27, 2023 · 36dd742 · 36dd742
1 parent 72e0034
commit 36dd742
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 0 deletions.
diff --git a/app/brave_command_ids.h b/app/brave_command_ids.h
@@ -55,6 +55,7 @@
 #define IDC_TOGGLE_TAB_MUTE 56041
 #define IDC_SIDEBAR_TOGGLE_POSITION 56042
 #define IDC_CONTENT_CONTEXT_COPY_TEXT_FROM_IMAGE 56043
+#define IDC_TOGGLE_SIDEBAR 56044
 
 #define IDC_CONTENT_CONTEXT_IMPORT_IPNS_KEYS_START 56100
 #define IDC_CONTENT_CONTEXT_IMPORT_IPNS_KEYS_END 56199

diff --git a/app/command_utils.cc b/app/command_utils.cc
@@ -209,6 +209,7 @@ const base::flat_map<uint16_t, std::string>& GetCommandInfo() {
             ADD_UNTRANSLATED_COMMAND(TOGGLE_BRAVE_VPN),
             ADD_UNTRANSLATED_COMMAND(COPY_CLEAN_LINK),
             ADD_UNTRANSLATED_COMMAND(SIDEBAR_TOGGLE_POSITION),
+            ADD_UNTRANSLATED_COMMAND(TOGGLE_SIDEBAR),
             ADD_UNTRANSLATED_COMMAND(TOGGLE_TAB_MUTE)
       });
   return *kCommands;

diff --git a/browser/ui/brave_browser_command_controller.cc b/browser/ui/brave_browser_command_controller.cc
@@ -195,6 +195,7 @@ void BraveBrowserCommandController::UpdateCommandForSidebar() {
   if (sidebar::CanUseSidebar(browser_)) {
     UpdateCommandEnabled(IDC_SIDEBAR_SHOW_OPTION_MENU, true);
     UpdateCommandEnabled(IDC_SIDEBAR_TOGGLE_POSITION, true);
+    UpdateCommandEnabled(IDC_TOGGLE_SIDEBAR, true);
   }
 }
 
@@ -311,6 +312,9 @@ bool BraveBrowserCommandController::ExecuteBraveCommandWithDisposition(
     case IDC_SIDEBAR_TOGGLE_POSITION:
       brave::ToggleSidebarPosition(browser_);
       break;
+    case IDC_TOGGLE_SIDEBAR:
+      brave::ToggleSidebar(browser_);
+      break;
     case IDC_COPY_CLEAN_LINK:
       brave::CopySanitizedURL(
           browser_,

diff --git a/browser/ui/brave_browser_command_controller_browsertest.cc b/browser/ui/brave_browser_command_controller_browsertest.cc
@@ -129,6 +129,8 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
   EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_OPEN_GUEST_PROFILE));
   EXPECT_TRUE(
       command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER));
+
+  EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_TOGGLE_SIDEBAR));
 }
 
 // Create private browser and test its brave commands status.

diff --git a/browser/ui/brave_browser_window.cc b/browser/ui/brave_browser_window.cc
@@ -37,6 +37,8 @@ sidebar::Sidebar* BraveBrowserWindow::InitSidebar() {
   return nullptr;
 }
 
+void BraveBrowserWindow::ToggleSidebar() {}
+
 bool BraveBrowserWindow::HasSelectedURL() const {
   return false;
 }

diff --git a/browser/ui/brave_browser_window.h b/browser/ui/brave_browser_window.h
@@ -56,6 +56,7 @@ class BraveBrowserWindow : public BrowserWindow {
 
 #if defined(TOOLKIT_VIEWS)
   virtual sidebar::Sidebar* InitSidebar();
+  virtual void ToggleSidebar();
   virtual bool HasSelectedURL() const;
   virtual void CleanAndCopySelectedURL();
 #endif

diff --git a/browser/ui/browser_commands.cc b/browser/ui/browser_commands.cc
@@ -277,6 +277,17 @@ void ToggleSidebarPosition(Browser* browser) {
                     !prefs->GetBoolean(prefs::kSidePanelHorizontalAlignment));
 }
 
+void ToggleSidebar(Browser* browser) {
+  if (!browser) {
+    return;
+  }
+
+  if (auto* brave_browser_window =
+          BraveBrowserWindow::From(browser->window())) {
+    brave_browser_window->ToggleSidebar();
+  }
+}
+
 bool HasSelectedURL(Browser* browser) {
   if (!browser) {
     return false;

diff --git a/browser/ui/browser_commands.h b/browser/ui/browser_commands.h
@@ -37,6 +37,7 @@ void ToggleVerticalTabStrip(Browser* browser);
 void ToggleVerticalTabStripFloatingMode(Browser* browser);
 void ToggleActiveTabAudioMute(Browser* browser);
 void ToggleSidebarPosition(Browser* browser);
+void ToggleSidebar(Browser* browser);
 
 }  // namespace brave
 

diff --git a/browser/ui/sidebar/BUILD.gn b/browser/ui/sidebar/BUILD.gn
@@ -75,6 +75,7 @@ source_set("browser_tests") {
 
   deps = [
     "//base",
+    "//brave/app:command_ids",
     "//brave/browser/ui",
     "//brave/browser/ui/sidebar",
     "//brave/browser/ui/tabs",

diff --git a/browser/ui/sidebar/sidebar_browsertest.cc b/browser/ui/sidebar/sidebar_browsertest.cc
@@ -7,6 +7,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind.h"
 #include "base/test/scoped_feature_list.h"
+#include "brave/app/brave_command_ids.h"
 #include "brave/browser/ui/brave_browser.h"
 #include "brave/browser/ui/browser_commands.h"
 #include "brave/browser/ui/sidebar/sidebar_controller.h"
@@ -24,6 +25,7 @@
 #include "brave/components/sidebar/sidebar_service.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -39,6 +41,7 @@
 #include "ui/gfx/geometry/point.h"
 
 using ::testing::Eq;
+using ::testing::Ne;
 using ::testing::Optional;
 
 namespace sidebar {
@@ -145,6 +148,18 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTest, BasicTest) {
   // Check sidebar UI is initalized properly.
   EXPECT_TRUE(!!controller()->sidebar());
 
+  browser()->command_controller()->ExecuteCommand(IDC_TOGGLE_SIDEBAR);
+  WaitUntil(
+      base::BindLambdaForTesting([&]() { return !!model()->active_index(); }));
+  // Check active index is non-null.
+  EXPECT_THAT(model()->active_index(), Ne(absl::nullopt));
+
+  browser()->command_controller()->ExecuteCommand(IDC_TOGGLE_SIDEBAR);
+  WaitUntil(
+      base::BindLambdaForTesting([&]() { return !model()->active_index(); }));
+  // Check active index is null.
+  EXPECT_THAT(model()->active_index(), Eq(absl::nullopt));
+
   // Currently we have 4 default items.
   EXPECT_EQ(4UL, model()->GetAllSidebarItems().size());
   // Activate item that opens in panel.

diff --git a/browser/ui/views/accelerator_table_unittest.cc b/browser/ui/views/accelerator_table_unittest.cc
@@ -24,4 +24,5 @@ bool HasCommandID(int command_id) {
 
 TEST(AcceleratorTableTest, CheckBraveAccelerators) {
   EXPECT_TRUE(HasCommandID(IDC_NEW_OFFTHERECORD_WINDOW_TOR));
+  EXPECT_TRUE(HasCommandID(IDC_TOGGLE_SIDEBAR));
 }
diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc
@@ -38,6 +38,7 @@
 #include "chrome/browser/ui/frame/window_frame_util.h"
 #include "chrome/browser/ui/views/frame/contents_layout_manager.h"
 #include "chrome/browser/ui/views/frame/tab_strip_region_view.h"
+#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"
 #include "chrome/browser/ui/views/tabs/tab_search_button.h"
 #include "chrome/browser/ui/views/toolbar/browser_app_menu_button.h"
 #include "chrome/common/pref_names.h"
@@ -268,6 +269,10 @@ sidebar::Sidebar* BraveBrowserView::InitSidebar() {
   return sidebar_container_view_;
 }
 
+void BraveBrowserView::ToggleSidebar() {
+  side_panel_coordinator()->Toggle();
+}
+
 void BraveBrowserView::ShowBraveVPNBubble() {
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
   vpn_panel_controller_.ShowBraveVPNPanel();

diff --git a/browser/ui/views/frame/brave_browser_view.h b/browser/ui/views/frame/brave_browser_view.h
@@ -119,6 +119,7 @@ class BraveBrowserView : public BrowserView {
   BraveBrowser* GetBraveBrowser() const;
 
   sidebar::Sidebar* InitSidebar() override;
+  void ToggleSidebar() override;
   bool HasSelectedURL() const override;
   void CleanAndCopySelectedURL() override;
   void UpdateSideBarHorizontalAlignment();

diff --git a/chromium_src/chrome/browser/ui/views/accelerator_table.cc b/chromium_src/chrome/browser/ui/views/accelerator_table.cc
@@ -15,6 +15,8 @@ namespace {
 
 const AcceleratorMapping kBraveAcceleratorMap[] = {
     {ui::VKEY_M, ui::EF_CONTROL_DOWN, IDC_TOGGLE_TAB_MUTE},
+    // Ctrl+B(or Cmd+B)
+    {ui::VKEY_B, ui::EF_PLATFORM_ACCELERATOR, IDC_TOGGLE_SIDEBAR},
 #if BUILDFLAG(IS_MAC)
     // Command-Option-N
     {ui::VKEY_N, ui::EF_ALT_DOWN | ui::EF_PLATFORM_ACCELERATOR,