Re-order brave menus at app menu

Insert & reorder brave menus based on corresponding commands enable status. If we want to add/remove from app menu, adjust enable commands status instead of adjusting it in app menu model directly. App menu model will add/delete based on commands status. Fix https://github.com/brave/brave-browser/issues/5552
2019-12-19 06:07:27 +09:00
parent 9894d5023a
commit dfa3dfb285
4 changed files with 147 additions and 76 deletions
@@ -141,10 +141,11 @@ void BraveBrowserCommandController::UpdateCommandForWebcompatReporter() {

 #if BUILDFLAG(ENABLE_TOR)
 void BraveBrowserCommandController::UpdateCommandForTor() {
-  const bool is_tor_enabled =
-      !brave::IsTorDisabledForProfile(browser_->profile());
-  UpdateCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE, is_tor_enabled);
-  UpdateCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR, is_tor_enabled);
+  // Enable new tor connection only for tor profile.
+  UpdateCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE,
+                       brave::IsTorProfile(browser_->profile()));
+  UpdateCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR,
+                       !brave::IsTorDisabledForProfile(browser_->profile()));
 }
 #endif

@@ -153,7 +154,9 @@ void BraveBrowserCommandController::UpdateCommandForBraveSync() {
 }

 void BraveBrowserCommandController::UpdateCommandForBraveWallet() {
-  UpdateCommandEnabled(IDC_SHOW_BRAVE_WALLET, true);
+  UpdateCommandEnabled(
+      IDC_SHOW_BRAVE_WALLET,
+      browser_->profile()->GetPrefs()->GetBoolean(kBraveWalletEnabled));
 }

 bool BraveBrowserCommandController::ExecuteBraveCommandWithDisposition(
@@ -5,24 +5,10 @@

 #include "brave/browser/ui/toolbar/brave_app_menu_model.h"

-#include "brave/app/brave_command_ids.h"
-#include "brave/browser/profiles/profile_util.h"
-#include "brave/common/pref_names.h"
-#include "brave/components/brave_wallet/browser/buildflags/buildflags.h"
-#include "brave/grit/brave_generated_resources.h"
 #include "chrome/app/chrome_command_ids.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/browser.h"
-#include "components/prefs/pref_service.h"
+#include "chrome/grit/generated_resources.h"

-BraveAppMenuModel::BraveAppMenuModel(
-    ui::AcceleratorProvider* provider,
-    Browser* browser,
-    AppMenuIconController* app_menu_icon_controller)
-    : AppMenuModel(provider, browser, app_menu_icon_controller),
-      browser_(browser) {}
-
-BraveAppMenuModel::~BraveAppMenuModel() {}
+BraveAppMenuModel::~BraveAppMenuModel() = default;

 void BraveAppMenuModel::Build() {
  // Insert brave items after build chromium items.
@@ -31,62 +17,144 @@ void BraveAppMenuModel::Build() {
 }

 void BraveAppMenuModel::InsertBraveMenuItems() {
-  // Sync & Rewards pages are redirected to normal window when it is loaded in
-  // private window. So, only hide them in guest(tor) window.
-  if (!browser_->profile()->IsGuestSession()) {
-    bool walletEnabled = browser_->profile()->GetPrefs()->
-        GetBoolean(kBraveWalletEnabled);
-    InsertItemWithStringIdAt(
-        GetIndexOfCommandId(IDC_SHOW_DOWNLOADS),
-        IDC_SHOW_BRAVE_REWARDS,
-        IDS_SHOW_BRAVE_REWARDS);
-    if (walletEnabled) {
-#if BUILDFLAG(BRAVE_WALLET_ENABLED)
-      InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_SHOW_BRAVE_REWARDS),
-                               IDC_SHOW_BRAVE_WALLET, IDS_SHOW_BRAVE_WALLET);
-#endif
-    }
-    InsertItemWithStringIdAt(
-#if BUILDFLAG(BRAVE_WALLET_ENABLED)
-        GetIndexOfCommandId(walletEnabled ? IDC_SHOW_BRAVE_WALLET :
-            IDC_SHOW_BRAVE_REWARDS),
-#else
-        GetIndexOfCommandId(IDC_SHOW_BRAVE_REWARDS),
-#endif
-        IDC_SHOW_BRAVE_SYNC, IDS_SHOW_BRAVE_SYNC);
-  }
+  // Insert & reorder brave menus based on corresponding commands enable status.
+  // If we you want to add/remove from app menu, adjust commands enable status
+  // at BraveBrowserCommandController.

-  // Insert Create New Profile item
-  auto profile_items_insert_index = GetIndexOfCommandId(IDC_ZOOM_MENU) - 1;
-  InsertItemWithStringIdAt(profile_items_insert_index,
-    IDC_OPEN_GUEST_PROFILE,
-    IDS_OPEN_GUEST_PROFILE);
-  InsertItemWithStringIdAt(
-    GetIndexOfCommandId(IDC_OPEN_GUEST_PROFILE),
-    IDC_ADD_NEW_PROFILE,
-    IDS_ADD_NEW_PROFILE);
-  InsertSeparatorAt(
-    GetIndexOfCommandId(IDC_ADD_NEW_PROFILE), ui::NORMAL_SEPARATOR);
-
-  InsertItemWithStringIdAt(
-      GetIndexOfCommandId(IDC_SHOW_DOWNLOADS),
-      IDC_SHOW_BRAVE_ADBLOCK,
-      IDS_SHOW_BRAVE_ADBLOCK);
-  if (brave::IsTorProfile(browser_->profile())) {
+  // Step 1. Configure tab & windows section.
+  if (IsCommandIdEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE)) {
    InsertItemWithStringIdAt(
        GetIndexOfCommandId(IDC_NEW_WINDOW),
        IDC_NEW_TOR_CONNECTION_FOR_SITE,
        IDS_NEW_TOR_CONNECTION_FOR_SITE);
  }
-
  if (IsCommandIdEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR)) {
    InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_NEW_INCOGNITO_WINDOW) + 1,
                             IDC_NEW_OFFTHERECORD_WINDOW_TOR,
                             IDS_NEW_OFFTHERECORD_WINDOW_TOR);
  }

-  InsertItemWithStringIdAt(
-      GetIndexOfCommandId(IDC_ABOUT),
-      IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER,
-      IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER);
+  // Step 2. Configure second section that includes history, downloads and
+  // bookmark. Then, insert brave items.
+
+  // First, reorder original menus We want to move them in order of bookmark,
+  // download and extensions.
+  const int bookmark_item_index = GetIndexOfCommandId(IDC_BOOKMARKS_MENU);
+  // If bookmark is not used, we don't need to adjust download item.
+  if (bookmark_item_index != -1) {
+    // Place download menu under bookmark.
+    DCHECK(IsCommandIdEnabled(IDC_SHOW_DOWNLOADS));
+    RemoveItemAt(GetIndexOfCommandId(IDC_SHOW_DOWNLOADS));
+    InsertItemWithStringIdAt(bookmark_item_index,
+                             IDC_SHOW_DOWNLOADS,
+                             IDS_SHOW_DOWNLOADS);
+  }
+  // Move extensions menu under download.
+  ui::SimpleMenuModel* model = static_cast<ui::SimpleMenuModel*>(
+      GetSubmenuModelAt(GetIndexOfCommandId(IDC_MORE_TOOLS_MENU)));
+  DCHECK(model);
+  model->RemoveItemAt(model->GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS));
+
+  if (IsCommandIdEnabled(IDC_MANAGE_EXTENSIONS)) {
+    InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_SHOW_DOWNLOADS) + 1,
+                             IDC_MANAGE_EXTENSIONS,
+                             IDS_SHOW_EXTENSIONS);
+  }
+
+  if (IsCommandIdEnabled(IDC_SHOW_BRAVE_REWARDS)) {
+    InsertItemWithStringIdAt(GetIndexOfBraveRewardsItem(),
+                             IDC_SHOW_BRAVE_REWARDS,
+                             IDS_SHOW_BRAVE_REWARDS);
+  }
+
+  // Insert wallet menu after download menu.
+  if (IsCommandIdEnabled(IDC_SHOW_BRAVE_WALLET)) {
+    InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_SHOW_DOWNLOADS) + 1,
+                             IDC_SHOW_BRAVE_WALLET,
+                             IDS_SHOW_BRAVE_WALLET);
+  }
+
+  // Insert sync menu
+  if (IsCommandIdEnabled(IDC_SHOW_BRAVE_SYNC)) {
+    InsertItemWithStringIdAt(GetIndexOfBraveSyncItem(),
+                             IDC_SHOW_BRAVE_SYNC,
+                             IDS_SHOW_BRAVE_SYNC);
+  }
+
+  // Insert adblock menu at last. Assumed this is always enabled.
+  DCHECK(IsCommandIdEnabled(IDC_SHOW_BRAVE_ADBLOCK));
+  InsertItemWithStringIdAt(GetIndexOfBraveAdBlockItem(),
+                           IDC_SHOW_BRAVE_ADBLOCK,
+                           IDS_SHOW_BRAVE_ADBLOCK);
+
+  // Insert Create New Profile item
+  if (IsCommandIdEnabled(IDC_OPEN_GUEST_PROFILE)) {
+    InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_ZOOM_MENU) - 1,
+                             IDC_OPEN_GUEST_PROFILE,
+                             IDS_OPEN_GUEST_PROFILE);
+    InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_OPEN_GUEST_PROFILE),
+                             IDC_ADD_NEW_PROFILE,
+                             IDS_ADD_NEW_PROFILE);
+    InsertSeparatorAt(GetIndexOfCommandId(IDC_ADD_NEW_PROFILE),
+                      ui::NORMAL_SEPARATOR);
+  }
+
+  // Insert webcompat reporter item.
+  InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_ABOUT),
+                           IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER,
+                           IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER);
+}
+
+int BraveAppMenuModel::GetIndexOfBraveAdBlockItem() const {
+  // Insert as a last item in second section.
+  int adblock_item_index = -1;
+  adblock_item_index = GetIndexOfCommandId(IDC_SHOW_BRAVE_SYNC);
+  if (adblock_item_index != -1)
+    return adblock_item_index + 1;
+
+  adblock_item_index = GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS);
+  if (adblock_item_index != -1)
+    return adblock_item_index + 1;
+
+  adblock_item_index = GetIndexOfCommandId(IDC_SHOW_BRAVE_WALLET);
+  if (adblock_item_index != -1)
+    return adblock_item_index + 1;
+
+  adblock_item_index = GetIndexOfCommandId(IDC_SHOW_DOWNLOADS);
+  DCHECK_NE(-1, adblock_item_index) << "No download item";
+  return adblock_item_index + 1;
+}
+
+int BraveAppMenuModel::GetIndexOfBraveRewardsItem() const {
+  // Insert rewards menu at first of this section. If history menu is not
+  // available, check below items.
+  int rewards_index = -1;
+  rewards_index = GetIndexOfCommandId(IDC_RECENT_TABS_MENU);
+  if (rewards_index != -1)
+    return rewards_index;
+
+  rewards_index = GetIndexOfCommandId(IDC_BOOKMARKS_MENU);
+  if (rewards_index != -1)
+    return rewards_index;
+
+  rewards_index = GetIndexOfCommandId(IDC_SHOW_DOWNLOADS);
+  DCHECK_NE(-1, rewards_index) << "No download item";
+  return rewards_index;
+}
+
+int BraveAppMenuModel::GetIndexOfBraveSyncItem() const {
+  // Insert sync menu under extensions menu. If extensions menu is not
+  // available, check above items.
+  int sync_index = -1;
+  sync_index = GetIndexOfCommandId(IDC_MANAGE_EXTENSIONS);
+  if (sync_index != -1)
+    return sync_index + 1;
+
+  sync_index = GetIndexOfCommandId(IDC_SHOW_BRAVE_WALLET);
+  if (sync_index != -1)
+    return sync_index + 1;
+
+  sync_index = GetIndexOfCommandId(IDC_SHOW_DOWNLOADS);
+  DCHECK_NE(-1, sync_index) << "No download item";
+  return sync_index + 1;
 }
@@ -10,20 +10,20 @@

 class BraveAppMenuModel : public AppMenuModel {
 public:
-  BraveAppMenuModel(ui::AcceleratorProvider* provider,
-      Browser* browser,
-      AppMenuIconController* app_menu_icon_controller = nullptr);
+  using AppMenuModel::AppMenuModel;
  ~BraveAppMenuModel() override;

+  BraveAppMenuModel(const BraveAppMenuModel&) = delete;
+  BraveAppMenuModel& operator=(const BraveAppMenuModel&) = delete;
+
 private:
  // AppMenuModel overrides:
  void Build() override;

  void InsertBraveMenuItems();
-
-  Browser* const browser_;  // weak
-
-  DISALLOW_COPY_AND_ASSIGN(BraveAppMenuModel);
+  int GetIndexOfBraveRewardsItem() const;
+  int GetIndexOfBraveAdBlockItem() const;
+  int GetIndexOfBraveSyncItem() const;
 };

 #endif  // BRAVE_BROWSER_UI_TOOLBAR_BRAVE_APP_MENU_MODEL_H_
@@ -52,8 +52,8 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, BasicTest) {
 #if BUILDFLAG(ENABLE_TOR)
  EXPECT_NE(
      -1, normal_model.GetIndexOfCommandId(IDC_NEW_OFFTHERECORD_WINDOW_TOR));
-  // Check tor browser commands are disabled.
-  EXPECT_TRUE(
+  // Check new tor browser command is only enabled for normal window.
+  EXPECT_FALSE(
      command_controller->IsCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE));
  EXPECT_TRUE(
      command_controller->IsCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR));