From 785b41f54e97d3ee0875bc4473257000f6b30f10 Mon Sep 17 00:00:00 2001 From: Pete Miller Date: Mon, 25 Jul 2022 21:08:55 +0100 Subject: [PATCH] Toolbar SidePanel button only opens items added on the sidebar, and hides if none are added --- .../views/sidebar/sidebar_container_view.cc | 23 ++++++++++++++ .../ui/views/sidebar/sidebar_container_view.h | 5 +++ .../side_panel/side_panel_coordinator.cc | 31 +++++++++++++++++++ components/sidebar/sidebar_service.cc | 19 ++++++++++++ components/sidebar/sidebar_service.h | 3 ++ ...side_panel-side_panel_coordinator.cc.patch | 12 +++++-- 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/browser/ui/views/sidebar/sidebar_container_view.cc b/browser/ui/views/sidebar/sidebar_container_view.cc index 4561aa84dc9c..9e62f62627d8 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.cc +++ b/browser/ui/views/sidebar/sidebar_container_view.cc @@ -23,6 +23,7 @@ #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/side_panel/side_panel_entry.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/native_web_keyboard_event.h" #include "content/public/browser/web_contents.h" @@ -113,6 +114,7 @@ void SidebarContainerView::Init() { AddChildViews(); // Hide by default. Visibility will be controlled by show options later. DoHideSidebar(false); + UpdateToolbarButtonVisibility(); } void SidebarContainerView::SetSidebarShowOption( @@ -263,6 +265,16 @@ void SidebarContainerView::OnActiveIndexChanged(int old_index, int new_index) { InvalidateLayout(); } +void SidebarContainerView::OnItemAdded(const sidebar::SidebarItem& item, + int index, + bool user_gesture) { + UpdateToolbarButtonVisibility(); +} + +void SidebarContainerView::OnItemRemoved(int index) { + UpdateToolbarButtonVisibility(); +} + SidebarShowOptionsEventDetectWidget* SidebarContainerView::GetEventDetectWidget() { if (!show_options_widget_) { @@ -307,6 +319,17 @@ void SidebarContainerView::DoHideSidebar(bool show_event_detect_widget) { InvalidateLayout(); } +void SidebarContainerView::UpdateToolbarButtonVisibility() { + // Coordinate sidebar toolbar button visibility based on + // whether there are any sibar items with a sidepanel. + // This is similar to how chromium's side_panel_coordinator View + // also has some control on the toolbar button. + auto has_panel_item = + GetSidebarService(browser_)->GetDefaultPanelItem().has_value(); + auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser_); + browser_view->toolbar()->side_panel_button()->SetVisible(has_panel_item); +} + void SidebarContainerView::StartBrowserWindowEventMonitoring() { if (browser_window_event_monitor_) return; diff --git a/browser/ui/views/sidebar/sidebar_container_view.h b/browser/ui/views/sidebar/sidebar_container_view.h index 02905df537d8..f0a21809159b 100644 --- a/browser/ui/views/sidebar/sidebar_container_view.h +++ b/browser/ui/views/sidebar/sidebar_container_view.h @@ -74,7 +74,11 @@ class SidebarContainerView void ShowSidebar() override; // sidebar::SidebarModel::Observer overrides: + void OnItemAdded(const sidebar::SidebarItem& item, + int index, + bool user_gesture) override; void OnActiveIndexChanged(int old_index, int new_index) override; + void OnItemRemoved(int index) override; // SidePanelEntryObserver: void OnEntryShown(SidePanelEntry* entry) override; @@ -95,6 +99,7 @@ class SidebarContainerView void ShowSidebar(bool show_sidebar, bool show_event_detect_widget); SidebarShowOptionsEventDetectWidget* GetEventDetectWidget(); bool ShouldShowSidebar() const; + void UpdateToolbarButtonVisibility(); // On some condition(ex, add item bubble is visible), // sidebar should not be hidden even if mouse goes out from sidebar ui. diff --git a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc index 961e350cfa83..28acca3d55d2 100644 --- a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc +++ b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc @@ -3,8 +3,39 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this file, // you can obtain one at http://mozilla.org/MPL/2.0/. +#include "brave/browser/ui/sidebar/sidebar_service_factory.h" +#include "brave/browser/ui/views/sidebar/sidebar_side_panel_utils.h" +#include "brave/components/sidebar/sidebar_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/views/side_panel/side_panel_entry.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace { + +absl::optional GetDefaultEntryId(Profile* profile) { + auto* service = sidebar::SidebarServiceFactory::GetForProfile(profile); + auto panel_item = service->GetDefaultPanelItem(); + if (panel_item.has_value()) { + return SidePanelIdFromSideBarItem(panel_item.value()); + } + return absl::nullopt; +} + +} // namespace + // Brave has its own side panel navigation in the form of the SideBar, so // hide the Chromium combobox-style header. #define BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER header->SetVisible(false); + +// Choose Brave's own default, and exclude items that user has removed +// from sidebar. If none are enabled, do nothing. +#define BRAVE_SIDE_PANEL_COORDINATOR_SHOW \ + if (!entry_id.has_value()) { \ + entry_id = GetDefaultEntryId(browser_view_->GetProfile()); \ + if (!entry_id.has_value()) \ + return; \ + } + #include "src/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc" #undef BRAVE_SIDE_PANEL_COORDINATOR_CREATE_HEADER +#undef BRAVE_SIDE_PANEL_COORDINATOR_SHOW diff --git a/components/sidebar/sidebar_service.cc b/components/sidebar/sidebar_service.cc index 3f940f6406e9..1f850ce2db2a 100644 --- a/components/sidebar/sidebar_service.cc +++ b/components/sidebar/sidebar_service.cc @@ -23,6 +23,7 @@ #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" #include "components/prefs/scoped_user_pref_update.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/resource_bundle.h" @@ -349,6 +350,24 @@ SidebarService::ShowSidebarOption SidebarService::GetSidebarShowOption() const { return static_cast(prefs_->GetInteger(kSidebarShowOption)); } +absl::optional SidebarService::GetDefaultPanelItem() const { + const std::vector preferred_item_types{ + SidebarItem::BuiltInItemType::kReadingList, + SidebarItem::BuiltInItemType::kBookmarks}; + absl::optional default_item; + for (const auto& type : preferred_item_types) { + auto found_item_iter = std::find_if( + items_.begin(), items_.end(), + [type](SidebarItem item) { return (item.built_in_item_type == type); }); + if (found_item_iter != items_.end()) { + default_item = *found_item_iter; + DCHECK_EQ(default_item->open_in_panel, true); + break; + } + } + return default_item; +} + void SidebarService::SetSidebarShowOption(ShowSidebarOption show_options) { DCHECK_NE(ShowSidebarOption::kShowOnClick, show_options); prefs_->SetInteger(kSidebarShowOption, static_cast(show_options)); diff --git a/components/sidebar/sidebar_service.h b/components/sidebar/sidebar_service.h index 1f6263bdfa55..b09275530273 100644 --- a/components/sidebar/sidebar_service.h +++ b/components/sidebar/sidebar_service.h @@ -16,6 +16,7 @@ #include "components/keyed_service/core/keyed_service.h" #include "components/prefs/pref_change_registrar.h" #include "components/version_info/channel.h" +#include "third_party/abseil-cpp/absl/types/optional.h" class PrefRegistrySimple; class PrefService; @@ -63,6 +64,8 @@ class SidebarService : public KeyedService { ShowSidebarOption GetSidebarShowOption() const; void SetSidebarShowOption(ShowSidebarOption show_options); + absl::optional GetDefaultPanelItem() const; + SidebarService(const SidebarService&) = delete; SidebarService& operator=(const SidebarService&) = delete; diff --git a/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch b/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch index 51895ceee4d3..cc8b5ff6c04d 100644 --- a/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch +++ b/patches/chrome-browser-ui-views-side_panel-side_panel_coordinator.cc.patch @@ -1,8 +1,16 @@ diff --git a/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc b/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc -index 1efe9453a6d271fdb118f39f95630296de13a95e..3353a5aba43a3e8f36766c75e3adc16bbb34f314 100644 +index 1efe9453a6d271fdb118f39f95630296de13a95e..ef134beaf3bf407580e6fa84a3bbc82f6856984a 100644 --- a/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc +++ b/chrome/browser/ui/views/side_panel/side_panel_coordinator.cc -@@ -419,6 +419,7 @@ std::unique_ptr SidePanelCoordinator::CreateHeader() { +@@ -180,6 +180,7 @@ SidePanelCoordinator::~SidePanelCoordinator() { + } + + void SidePanelCoordinator::Show(absl::optional entry_id) { ++ BRAVE_SIDE_PANEL_COORDINATOR_SHOW + if (!entry_id.has_value()) + entry_id = GetLastActiveEntryId().value_or(kDefaultEntry); + +@@ -419,6 +420,7 @@ std::unique_ptr SidePanelCoordinator::CreateHeader() { ChromeLayoutProvider::Get()->GetDistanceMetric( ChromeDistanceMetric::DISTANCE_SIDE_PANEL_HEADER_VECTOR_ICON_SIZE)));