From f4c1efe5c4b83e0372e0db3ee0574c74df505c01 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 19 Sep 2023 13:12:21 +0900 Subject: [PATCH 1/2] Added header to bookmarks panel fix https://github.com/brave/brave-browser/issues/32952 Bookmarks panel's header includes icon, title and button for loading bookmarks manager. --- app/brave_generated_resources.grd | 10 ++ app/theme/brave_theme_resources.grd | 1 + .../brave/sidebar_bookmarks_panel_header.png | Bin 0 -> 791 bytes .../brave/sidebar_bookmarks_panel_header.png | Bin 0 -> 1703 bytes browser/ui/BUILD.gn | 4 + browser/ui/color/brave_color_id.h | 4 +- browser/ui/color/brave_color_mixer.cc | 8 + .../brave_bookmarks_side_panel_coordinator.cc | 41 +++++ .../brave_bookmarks_side_panel_coordinator.h | 50 ++++++ .../brave_bookmarks_side_panel_view.cc | 143 ++++++++++++++++++ .../brave_bookmarks_side_panel_view.h | 33 ++++ .../bookmarks_side_panel_coordinator.h | 18 +++ .../ui/views/side_panel/side_panel_util.cc | 4 + 13 files changed, 315 insertions(+), 1 deletion(-) create mode 100644 app/theme/default_100_percent/brave/sidebar_bookmarks_panel_header.png create mode 100644 app/theme/default_200_percent/brave/sidebar_bookmarks_panel_header.png create mode 100644 browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.cc create mode 100644 browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h create mode 100644 browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc create mode 100644 browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h create mode 100644 chromium_src/chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index 076d59fa1acc..9d032047ff3c 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -799,6 +799,16 @@ Or change later at $2brave://settings/ext Reading List + + + Bookmarks manager + + + + + Bookmarks Manager + + diff --git a/app/theme/brave_theme_resources.grd b/app/theme/brave_theme_resources.grd index ebfe896e5415..d6338bfe3cb1 100644 --- a/app/theme/brave_theme_resources.grd +++ b/app/theme/brave_theme_resources.grd @@ -48,6 +48,7 @@ + diff --git a/app/theme/default_100_percent/brave/sidebar_bookmarks_panel_header.png b/app/theme/default_100_percent/brave/sidebar_bookmarks_panel_header.png new file mode 100644 index 0000000000000000000000000000000000000000..11ff0bdf11eb5c93665b23c91bccc06fbcb2b894 GIT binary patch literal 791 zcmV+y1L*vTP)>O-yt7?;avOeRepK%Iv48j zV)O-vowMKWanbMJlxUjuDl3K(vB}Yns9OsOS^zx?&Z$i9Sm419_Eu3$2onK?CVi|O z9Z<4}C4nDBEAj`;IK>WPC_Z&;B4BC=kaVguOo?HlRcmklQ;{GwNB36~Y8V^-I44UK z>wt9)2~>|UOsTt>#)3Nn*QUvhErizP*|7!?8ptLB#^m(4lrq-gPf@b=8z%D|0f$!- zjW1)Is2FMpnSsA(NPn)CJhydwXK(&v=fc;!-CF#Lon7yW{pEpAAVB6P1In_iICePu zWejYPu(!N1{I;hpaBjm?45wPLWacM#SsD!G+?`~JI5y)s6?(6;wJm7sGo^ZoGEIpZ zBsGbAz|v#!8#Dzug3kvwoKj85Y};jKHGlDHZH%EX@zP@(EPkc|--+XuSeiM8%nzAJ z2VRnH!Uiwt%1B&d61yB+_Yd~_*NWmt`%M8SSZ3+|clB^_rS7ZdFf*o?8Eo}i&s#9< zY9O#2#~z1D!-Gq&TRHZ(Q>|MGkY1S%Avbgr42#`_h6(gAf~P&pP4ErPAnGdO9Qrl+ z^uubzsWhC~UF?yW!*lC5=#I8XP-=!Ftt=~(NJF2Ra+m|{9lgX!w3OsL-+}+NegmI9 VdMxILmB9c2002ovPDHLkV1k+qTDt%M literal 0 HcmV?d00001 diff --git a/app/theme/default_200_percent/brave/sidebar_bookmarks_panel_header.png b/app/theme/default_200_percent/brave/sidebar_bookmarks_panel_header.png new file mode 100644 index 0000000000000000000000000000000000000000..0db2aeee85abea9f5625bfdf4343f99744a91944 GIT binary patch literal 1703 zcmV;Y23YxtP)_a-AIo6-Z1_i?{|_k8@$xzCtK@{o&d zYPh+?9cOngTA~?O6voGBY#@`#UEVo8`yk&7lLbbffVN;9+I-pO(3TV7JD%km$#?V# z=^bOYT^KGOySvb4aQmkbGWR4!n-{Cn8tc`r?BkVi2<@Tx3NH17YWpN6#BMYQ12mjT%;)n%{Y!* zsu9srd3SP`et3o!rvb)7bA4>Ee-F5TEMy*BadxNsQTOrQTfGATezElTECx*{_0!dt z-#xyxzt6XC{5-V?+^~}mVG`=F{ ze$3QM1z;8#lLjm5Ig8{}ySOBOvBv}DMP6Zoo?h&97dnashij-ZE(EEGK% zK;^s0x;Q+$ew1v2wU22J8>v)HYym@k&{X|ZF=bdMLmN-1K{2r45E+qN0hk4ju-McV@6 zm)Wlu0dCC+u@iTh=!ZXMZd{V#!ijzqP?ep6;C%#0M0mSC+NwlNX)@h)z1Af-N)m>u zL%2c|=Qa9c$|9L$h{*3d7fwkZ=`WkA0z@=AF)`-J>(_GC#Z%r@fP;DsYCcV1Bqz_O z{?sQPo(G^gOJPel5c#5H5dkfMwO^tylz7z#8>A+R&0ez4n?|KbH@bL;qM;rf_ob8H zz1oxJKcD@4!*kGdnKX+&riNy)081EWNRxSeu(?|8gp|llgX9|NqkPpr@N zM(xM$Ug#CObQ<&AEc?{2ZY4;fNgCX#?NUoKi3>4lQq5%2g9u&a@6_#g1Zy~+=2fx9zU8x1d#MCMlvHInM0ctUtQe76qZ5$D&yp-hxQk9dN zTvA7uM^URqfS5|iQzK0L3U@g-1lIB#X4E!0lX}ofD#zh_q_rS4$mHT;c=NS8*~gz< z9}BQ$%5!Tfkvi!ttfdvF!w)KCISZHCvr?|-I6+|{$*CHVUY;d|r3mE`1o$o$r*cWD zC1W(zk+g3Wp4Sf6;BYUKh!*?I#oX~9FH{EpUm0FlSazP@5zyJnr;P`jYLI9}oeo5} zUwUHW@WF|=jHku)FwN$;epQ-5MQx=Df>~W*@0cSJn6@LeAZ5{x>sL`Nj$pmAL_)9( zXU!1_WnoJL#*fce6@XdDHW0DfDG0)q-R`w><_N@UnNJd6-Ld`ZK5jqv$tAGBdm6nO zEJ1$y^sS4jL~q5!{>p19LK%_Fd5YkaAxiGkh^4YGiRMhA7Mg-KI!ZytpdvSY3_EL= zS0DJBQTumm>ZjM2cb;C6zgJ6o483K)*8~hGjTd;A|wq7^XCbm$%+xLrSd6H{W$&L xDKkpk%QA8&U(R#CgR$Ah6ifRx_(&dl`4>sUMN@aeOoRXc002ovPDHLkV1hi57fk>F literal 0 HcmV?d00001 diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index 3f2745086678..af60dce6118c 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -330,6 +330,10 @@ source_set("ui") { "views/page_action/brave_page_action_icon_container_view.h", "views/rounded_separator.cc", "views/rounded_separator.h", + "views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.cc", + "views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h", + "views/side_panel/brave_bookmarks_side_panel_view.cc", + "views/side_panel/brave_bookmarks_side_panel_view.h", "views/side_panel/brave_read_later_side_panel_view.cc", "views/side_panel/brave_read_later_side_panel_view.h", "views/side_panel/brave_side_panel.cc", diff --git a/browser/ui/color/brave_color_id.h b/browser/ui/color/brave_color_id.h index 9ae91bf4a8a9..2c0662641d0f 100644 --- a/browser/ui/color/brave_color_id.h +++ b/browser/ui/color/brave_color_id.h @@ -57,7 +57,9 @@ E_CPONLY(kColorSidebarSeparator) \ E_CPONLY(kColorSidebarPanelHeaderSeparator) \ E_CPONLY(kColorSidebarPanelHeaderBackground) \ - E_CPONLY(kColorSidebarPanelHeaderTitle) + E_CPONLY(kColorSidebarPanelHeaderTitle) \ + E_CPONLY(kColorSidebarPanelHeaderButton) \ + E_CPONLY(kColorSidebarPanelHeaderButtonHovered) #if BUILDFLAG(ENABLE_SPEEDREADER) #define BRAVE_SPEEDREADER_COLOR_IDS \ diff --git a/browser/ui/color/brave_color_mixer.cc b/browser/ui/color/brave_color_mixer.cc index d7d9850f8c5b..0c60bcc2d725 100644 --- a/browser/ui/color/brave_color_mixer.cc +++ b/browser/ui/color/brave_color_mixer.cc @@ -362,6 +362,10 @@ void AddBraveLightThemeColorMixer(ui::ColorProvider* provider, leo::GetColor(leo::Color::kColorContainerBackground, leo::Theme::kLight)}; mixer[kColorSidebarPanelHeaderTitle] = { leo::GetColor(leo::Color::kColorTextSecondary, leo::Theme::kLight)}; + mixer[kColorSidebarPanelHeaderButton] = { + leo::GetColor(leo::Color::kColorIconDefault, leo::Theme::kLight)}; + mixer[kColorSidebarPanelHeaderButtonHovered] = { + leo::GetColor(leo::Color::kColorGray60, leo::Theme::kLight)}; if (HasCustomToolbarColor(key)) { // When custom color for toolbar is set, we can't depend on the color mode. @@ -478,6 +482,10 @@ void AddBraveDarkThemeColorMixer(ui::ColorProvider* provider, mixer[kColorSidebarPanelHeaderBackground] = {gfx::kGoogleGrey900}; mixer[kColorSidebarPanelHeaderTitle] = { leo::GetColor(leo::Color::kColorTextSecondary, leo::Theme::kDark)}; + mixer[kColorSidebarPanelHeaderButton] = { + leo::GetColor(leo::Color::kColorIconDefault, leo::Theme::kDark)}; + mixer[kColorSidebarPanelHeaderButtonHovered] = { + leo::GetColor(leo::Color::kColorGray60, leo::Theme::kDark)}; if (HasCustomToolbarColor(key)) { // When custom color for toolbar is set, we can't depend on the color mode. diff --git a/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.cc b/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.cc new file mode 100644 index 000000000000..23e5a065c2cd --- /dev/null +++ b/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.cc @@ -0,0 +1,41 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h" + +#include "base/functional/bind.h" +#include "brave/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h" +#include "chrome/browser/ui/views/side_panel/side_panel_entry.h" +#include "chrome/browser/ui/views/side_panel/side_panel_registry.h" +#include "chrome/grit/generated_resources.h" +#include "components/omnibox/browser/vector_icons.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/base/models/image_model.h" +#include "ui/views/vector_icons.h" + +BraveBookmarksSidePanelCoordinator::BraveBookmarksSidePanelCoordinator( + Browser* browser) + : BrowserUserData(*browser) {} + +BraveBookmarksSidePanelCoordinator::~BraveBookmarksSidePanelCoordinator() = + default; + +void BraveBookmarksSidePanelCoordinator::CreateAndRegisterEntry( + SidePanelRegistry* global_registry) { + global_registry->Register(std::make_unique( + SidePanelEntry::Id::kBookmarks, + l10n_util::GetStringUTF16(IDS_BOOKMARK_MANAGER_TITLE), + ui::ImageModel::FromVectorIcon(omnibox::kStarIcon, ui::kColorIcon), + base::BindRepeating( + &BraveBookmarksSidePanelCoordinator::CreateBookmarksPanelView, + base::Unretained(this)))); +} + +std::unique_ptr +BraveBookmarksSidePanelCoordinator::CreateBookmarksPanelView() { + return std::make_unique(&GetBrowser()); +} + +BROWSER_USER_DATA_KEY_IMPL(BraveBookmarksSidePanelCoordinator); diff --git a/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h b/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h new file mode 100644 index 000000000000..8eefa7e524a4 --- /dev/null +++ b/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h @@ -0,0 +1,50 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BOOKMARKS_BRAVE_BOOKMARKS_SIDE_PANEL_COORDINATOR_H_ +#define BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BOOKMARKS_BRAVE_BOOKMARKS_SIDE_PANEL_COORDINATOR_H_ + +#include + +#include "chrome/browser/ui/browser_user_data.h" + +class Browser; +class SidePanelRegistry; + +namespace views { +class View; +} // namespace views + +// Introduced to give custom contents view(BraveBookmarksSidePanelView) for +// bookmarks panel entry. That contents view includes bookmarks panel specific +// header view and web view. +// Note that this is not the subclass of upstream BookmarksSidePanelCoordinator. +// As it inherits BrowserUserData, subclassig doesn't work well when its +// instance is created via BrowserUserData<>::CreateForBrowser(). So, new +// coordinator class is introduced and +// BookmarksSidePanelCoordinator::CreateBookmarksWebView() is reused from +// BraveBookmarksSidePanelView. That's why BraveBookmarksSidePanelView is set +// as BookmarksSidePanelCoordinator's friend. +class BraveBookmarksSidePanelCoordinator + : public BrowserUserData { + public: + explicit BraveBookmarksSidePanelCoordinator(Browser* browser); + BraveBookmarksSidePanelCoordinator( + const BraveBookmarksSidePanelCoordinator&) = delete; + BraveBookmarksSidePanelCoordinator& operator=( + const BraveBookmarksSidePanelCoordinator&) = delete; + ~BraveBookmarksSidePanelCoordinator() override; + + void CreateAndRegisterEntry(SidePanelRegistry* global_registry); + + private: + friend class BrowserUserData; + + std::unique_ptr CreateBookmarksPanelView(); + + BROWSER_USER_DATA_KEY_DECL(); +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BOOKMARKS_BRAVE_BOOKMARKS_SIDE_PANEL_COORDINATOR_H_ diff --git a/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc new file mode 100644 index 000000000000..b6c28d86093a --- /dev/null +++ b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc @@ -0,0 +1,143 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h" + +#include + +#include "base/memory/raw_ref.h" +#include "brave/browser/ui/color/brave_color_id.h" +#include "brave/components/vector_icons/vector_icons.h" +#include "brave/grit/brave_generated_resources.h" +#include "brave/grit/brave_theme_resources.h" +#include "chrome/browser/ui/singleton_tabs.h" +#include "chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h" +#include "chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.h" +#include "chrome/browser/ui/views/side_panel/side_panel_content_proxy.h" +#include "chrome/browser/ui/views/side_panel/side_panel_util.h" +#include "chrome/common/webui_url_constants.h" +#include "chrome/grit/generated_resources.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/base/models/image_model.h" +#include "ui/base/resource/resource_bundle.h" +#include "ui/gfx/font_list.h" +#include "ui/gfx/geometry/insets.h" +#include "ui/views/background.h" +#include "ui/views/controls/button/image_button.h" +#include "ui/views/controls/image_view.h" +#include "ui/views/controls/label.h" +#include "ui/views/controls/separator.h" +#include "ui/views/layout/flex_layout.h" +#include "ui/views/layout/layout_types.h" + +namespace { + +// Renders icon, title and launch button. +class BookmarksSidePanelHeaderView : public views::View { + public: + explicit BookmarksSidePanelHeaderView(Browser* browser) { + constexpr int kHeaderInteriorMargin = 16; + SetLayoutManager(std::make_unique()) + ->SetOrientation(views::LayoutOrientation::kHorizontal) + .SetInteriorMargin(gfx::Insets(kHeaderInteriorMargin)) + .SetMainAxisAlignment(views::LayoutAlignment::kStart) + .SetCrossAxisAlignment(views::LayoutAlignment::kCenter); + + ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); + auto* header_image = AddChildView( + std::make_unique(ui::ImageModel::FromImageSkia( + *rb.GetImageSkiaNamed(IDR_SIDEBAR_BOOKMARKS_PANEL_HEADER)))); + constexpr int kSpacingBetweenHeaderImageAndLabel = 8; + header_image->SetProperty( + views::kMarginsKey, + gfx::Insets::TLBR(0, 0, 0, kSpacingBetweenHeaderImageAndLabel)); + header_image->SetProperty( + views::kFlexBehaviorKey, + views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred, + views::MaximumFlexSizeRule::kPreferred)); + + auto* header_label = AddChildView(std::make_unique( + l10n_util::GetStringUTF16(IDS_BOOKMARK_MANAGER_TITLE))); + header_label->SetFontList(gfx::FontList("Poppins, Semi-Bold 16px")); + header_label->SetEnabledColorId(kColorSidebarPanelHeaderTitle); + header_label->SetProperty( + views::kFlexBehaviorKey, + views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred, + views::MaximumFlexSizeRule::kPreferred)); + auto* spacer = AddChildView(std::make_unique()); + spacer->SetProperty( + views::kFlexBehaviorKey, + views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero, + views::MaximumFlexSizeRule::kUnbounded) + .WithOrder(2)); + // Safe to use Unretained(this) here as |button| will be destroyed before + // this class. + auto* button = AddChildView(std::make_unique( + base::BindRepeating(&BookmarksSidePanelHeaderView::OnButtonPressed, + base::Unretained(this), browser))); + button->SetTooltipText(l10n_util::GetStringUTF16( + IDS_SIDEBAR_READING_LIST_PANEL_HEADER_BOOKMARKS_BUTTON_TOOLTIP)); + + constexpr int kHeaderButtonSize = 20; + button->SetImageModel( + views::Button::STATE_NORMAL, + ui::ImageModel::FromVectorIcon( + kLeoLaunchIcon, kColorSidebarPanelHeaderButton, kHeaderButtonSize)); + button->SetImageModel( + views::Button::STATE_HOVERED, + ui::ImageModel::FromVectorIcon(kLeoLaunchIcon, + kColorSidebarPanelHeaderButtonHovered, + kHeaderButtonSize)); + + SetBackground( + views::CreateThemedSolidBackground(kColorSidebarPanelHeaderBackground)); + } + + ~BookmarksSidePanelHeaderView() override = default; + BookmarksSidePanelHeaderView(const BookmarksSidePanelHeaderView&) = delete; + BookmarksSidePanelHeaderView& operator=(const BookmarksSidePanelHeaderView&) = + delete; + + private: + void OnButtonPressed(Browser* browser, const ui::Event& event) { + ShowSingletonTab(browser, GURL(chrome::kChromeUIBookmarksURL)); + } +}; + +} // namespace + +BraveBookmarksSidePanelView::BraveBookmarksSidePanelView(Browser* browser) { + SetLayoutManager(std::make_unique()) + ->SetOrientation(views::LayoutOrientation::kVertical); + AddChildView(std::make_unique(browser)); + AddChildView(std::make_unique()) + ->SetColorId(kColorSidebarPanelHeaderSeparator); + + // Reuse upstream's bookmarks panl nwebui. + auto* web_view = + AddChildView(BookmarksSidePanelCoordinator::GetOrCreateForBrowser(browser) + ->CreateBookmarksWebView()); + web_view->SetProperty( + views::kFlexBehaviorKey, + views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred, + views::MaximumFlexSizeRule::kUnbounded)); + + // See the comment in + // BraveReadLaterSidePanelView::BraveReadLaterSidePanelView() about why we do + // this. + SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(false); + observation_.Observe(web_view); +} + +BraveBookmarksSidePanelView::~BraveBookmarksSidePanelView() = default; + +void BraveBookmarksSidePanelView::OnViewVisibilityChanged( + views::View* observed_view, + views::View* starting_view) { + if (observed_view->GetVisible()) { + SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(true); + observation_.Reset(); + } +} diff --git a/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h new file mode 100644 index 000000000000..ff3e8e836170 --- /dev/null +++ b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h @@ -0,0 +1,33 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_BOOKMARKS_SIDE_PANEL_VIEW_H_ +#define BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_BOOKMARKS_SIDE_PANEL_VIEW_H_ + +#include "base/functional/callback_forward.h" +#include "base/scoped_observation.h" +#include "ui/views/view.h" +#include "ui/views/view_observer.h" + +class Browser; + +// Gives bookmarks panel specific header view with web view. +class BraveBookmarksSidePanelView : public views::View, + public views::ViewObserver { + public: + explicit BraveBookmarksSidePanelView(Browser* browser); + ~BraveBookmarksSidePanelView() override; + BraveBookmarksSidePanelView(const BraveBookmarksSidePanelView&) = delete; + BraveBookmarksSidePanelView& operator=(const BraveBookmarksSidePanelView&) = + delete; + + private: + void OnViewVisibilityChanged(views::View* observed_view, + views::View* starting_view) override; + + base::ScopedObservation observation_{this}; +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_BOOKMARKS_SIDE_PANEL_VIEW_H_ diff --git a/chromium_src/chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h b/chromium_src/chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h new file mode 100644 index 000000000000..38ad1e987b04 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h @@ -0,0 +1,18 @@ +// Copyright (c) 2023 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_SIDE_PANEL_BOOKMARKS_BOOKMARKS_SIDE_PANEL_COORDINATOR_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_SIDE_PANEL_BOOKMARKS_BOOKMARKS_SIDE_PANEL_COORDINATOR_H_ + +#define CreateAndRegisterEntry \ + CreateAndRegisterEntry_UnUsed() {} \ + friend class BraveBookmarksSidePanelView; \ + void CreateAndRegisterEntry + +#include "src/chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h" // IWYU pragma: export + +#undef CreateAndRegisterEntry + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_SIDE_PANEL_BOOKMARKS_BOOKMARKS_SIDE_PANEL_COORDINATOR_H_ diff --git a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_util.cc b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_util.cc index dce47edb506e..933f8ef94b4f 100644 --- a/chromium_src/chrome/browser/ui/views/side_panel/side_panel_util.cc +++ b/chromium_src/chrome/browser/ui/views/side_panel/side_panel_util.cc @@ -4,13 +4,17 @@ // you can obtain one at http://mozilla.org/MPL/2.0/. #include "chrome/browser/ui/views/side_panel/side_panel_util.h" +#include "brave/browser/ui/views/side_panel/bookmarks/brave_bookmarks_side_panel_coordinator.h" #include "brave/browser/ui/views/side_panel/playlist/playlist_side_panel_coordinator.h" #include "brave/components/playlist/common/features.h" +#include "chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h" #define PopulateGlobalEntries PopulateGlobalEntries_ChromiumImpl +#define BookmarksSidePanelCoordinator BraveBookmarksSidePanelCoordinator #include "src/chrome/browser/ui/views/side_panel/side_panel_util.cc" +#undef BookmarksSidePanelCoordinator #undef PopulateGlobalEntries // static From 70255d1e013f109973a2ed2b7cfb291b74252eb4 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 20 Sep 2023 17:52:01 +0900 Subject: [PATCH 2/2] Introduced BraveSidePanelViewBase for sharing common part --- browser/ui/BUILD.gn | 2 + .../brave_bookmarks_side_panel_view.cc | 17 +------- .../brave_bookmarks_side_panel_view.h | 14 +------ .../brave_read_later_side_panel_view.cc | 26 +----------- .../brave_read_later_side_panel_view.h | 13 +----- .../side_panel/brave_side_panel_view_base.cc | 41 +++++++++++++++++++ .../side_panel/brave_side_panel_view_base.h | 31 ++++++++++++++ 7 files changed, 80 insertions(+), 64 deletions(-) create mode 100644 browser/ui/views/side_panel/brave_side_panel_view_base.cc create mode 100644 browser/ui/views/side_panel/brave_side_panel_view_base.h diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index af60dce6118c..2ffe5633bfcf 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -343,6 +343,8 @@ source_set("ui") { "views/side_panel/brave_side_panel_resize_widget.cc", "views/side_panel/brave_side_panel_resize_widget.h", "views/side_panel/brave_side_panel_utils.cc", + "views/side_panel/brave_side_panel_view_base.cc", + "views/side_panel/brave_side_panel_view_base.h", "views/tabs/brave_browser_tab_strip_controller.cc", "views/tabs/brave_browser_tab_strip_controller.h", "views/tabs/brave_compound_tab_container.cc", diff --git a/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc index b6c28d86093a..2b4d1af3040f 100644 --- a/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc +++ b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.cc @@ -15,8 +15,6 @@ #include "chrome/browser/ui/singleton_tabs.h" #include "chrome/browser/ui/views/side_panel/bookmarks/bookmarks_side_panel_coordinator.h" #include "chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.h" -#include "chrome/browser/ui/views/side_panel/side_panel_content_proxy.h" -#include "chrome/browser/ui/views/side_panel/side_panel_util.h" #include "chrome/common/webui_url_constants.h" #include "chrome/grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -124,20 +122,7 @@ BraveBookmarksSidePanelView::BraveBookmarksSidePanelView(Browser* browser) { views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred, views::MaximumFlexSizeRule::kUnbounded)); - // See the comment in - // BraveReadLaterSidePanelView::BraveReadLaterSidePanelView() about why we do - // this. - SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(false); - observation_.Observe(web_view); + StartObservingWebWebViewVisibilityChange(web_view); } BraveBookmarksSidePanelView::~BraveBookmarksSidePanelView() = default; - -void BraveBookmarksSidePanelView::OnViewVisibilityChanged( - views::View* observed_view, - views::View* starting_view) { - if (observed_view->GetVisible()) { - SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(true); - observation_.Reset(); - } -} diff --git a/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h index ff3e8e836170..54a56eed4b9c 100644 --- a/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h +++ b/browser/ui/views/side_panel/brave_bookmarks_side_panel_view.h @@ -6,28 +6,18 @@ #ifndef BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_BOOKMARKS_SIDE_PANEL_VIEW_H_ #define BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_BOOKMARKS_SIDE_PANEL_VIEW_H_ -#include "base/functional/callback_forward.h" -#include "base/scoped_observation.h" -#include "ui/views/view.h" -#include "ui/views/view_observer.h" +#include "brave/browser/ui/views/side_panel/brave_side_panel_view_base.h" class Browser; // Gives bookmarks panel specific header view with web view. -class BraveBookmarksSidePanelView : public views::View, - public views::ViewObserver { +class BraveBookmarksSidePanelView : public BraveSidePanelViewBase { public: explicit BraveBookmarksSidePanelView(Browser* browser); ~BraveBookmarksSidePanelView() override; BraveBookmarksSidePanelView(const BraveBookmarksSidePanelView&) = delete; BraveBookmarksSidePanelView& operator=(const BraveBookmarksSidePanelView&) = delete; - - private: - void OnViewVisibilityChanged(views::View* observed_view, - views::View* starting_view) override; - - base::ScopedObservation observation_{this}; }; #endif // BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_BOOKMARKS_SIDE_PANEL_VIEW_H_ diff --git a/browser/ui/views/side_panel/brave_read_later_side_panel_view.cc b/browser/ui/views/side_panel/brave_read_later_side_panel_view.cc index 25841064d29b..f05b9fcb264c 100644 --- a/browser/ui/views/side_panel/brave_read_later_side_panel_view.cc +++ b/browser/ui/views/side_panel/brave_read_later_side_panel_view.cc @@ -11,8 +11,6 @@ #include "brave/grit/brave_generated_resources.h" #include "brave/grit/brave_theme_resources.h" #include "chrome/browser/ui/views/side_panel/read_later_side_panel_web_view.h" -#include "chrome/browser/ui/views/side_panel/side_panel_content_proxy.h" -#include "chrome/browser/ui/views/side_panel/side_panel_util.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/resource_bundle.h" #include "ui/gfx/font_list.h" @@ -87,29 +85,7 @@ BraveReadLaterSidePanelView::BraveReadLaterSidePanelView( views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred, views::MaximumFlexSizeRule::kUnbounded)); - // Originally SidePanelEntry's Content was ReadLaterSidePanelWebView - // and it's availability is set to true when SidePanelWebUIView::ShowUI() - // and then proxy's availability callback is executed. - // However, we use parent view(BraveReadLaterSidePanelView) to have - // panel specific header view and this class becomes SidePanelEntry's Content. - // To make this content available when SidePanelWebUIVew::ShowUI() is called, - // this observes WebView's visibility because it's set as visible when - // ShowUI() is called. - // NOTE: If we use our own reading list page and it has loading spinner, maybe - // we can set `true` here. - SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(false); - observation_.Observe(web_view); -} - -void BraveReadLaterSidePanelView::OnViewVisibilityChanged( - views::View* observed_view, - views::View* starting_view) { - // Once it becomes available, stop observing becuase its availablity is - // not changed from now on. - if (observed_view->GetVisible()) { - SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(true); - observation_.Reset(); - } + StartObservingWebWebViewVisibilityChange(web_view); } BraveReadLaterSidePanelView::~BraveReadLaterSidePanelView() = default; diff --git a/browser/ui/views/side_panel/brave_read_later_side_panel_view.h b/browser/ui/views/side_panel/brave_read_later_side_panel_view.h index d68e1489fb41..0bbb48257c8f 100644 --- a/browser/ui/views/side_panel/brave_read_later_side_panel_view.h +++ b/browser/ui/views/side_panel/brave_read_later_side_panel_view.h @@ -7,15 +7,12 @@ #define BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_READ_LATER_SIDE_PANEL_VIEW_H_ #include "base/functional/callback_forward.h" -#include "base/scoped_observation.h" -#include "ui/views/view.h" -#include "ui/views/view_observer.h" +#include "brave/browser/ui/views/side_panel/brave_side_panel_view_base.h" class Browser; // Gives reading list specific header view with web view. -class BraveReadLaterSidePanelView : public views::View, - public views::ViewObserver { +class BraveReadLaterSidePanelView : public BraveSidePanelViewBase { public: BraveReadLaterSidePanelView(Browser* browser, base::RepeatingClosure close_cb); @@ -23,12 +20,6 @@ class BraveReadLaterSidePanelView : public views::View, BraveReadLaterSidePanelView(const BraveReadLaterSidePanelView&) = delete; BraveReadLaterSidePanelView& operator=(const BraveReadLaterSidePanelView&) = delete; - - private: - void OnViewVisibilityChanged(views::View* observed_view, - views::View* starting_view) override; - - base::ScopedObservation observation_{this}; }; #endif // BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_READ_LATER_SIDE_PANEL_VIEW_H_ diff --git a/browser/ui/views/side_panel/brave_side_panel_view_base.cc b/browser/ui/views/side_panel/brave_side_panel_view_base.cc new file mode 100644 index 000000000000..693c5fd77cc0 --- /dev/null +++ b/browser/ui/views/side_panel/brave_side_panel_view_base.cc @@ -0,0 +1,41 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/side_panel/brave_side_panel_view_base.h" + +#include "chrome/browser/ui/views/side_panel/side_panel_content_proxy.h" +#include "chrome/browser/ui/views/side_panel/side_panel_util.h" + +BraveSidePanelViewBase::BraveSidePanelViewBase() { + // Originally SidePanelEntry's Content was + // [ReadLater|Bookmarks]SidePanelWebView and it's availability is set to true + // when SidePanelWebUIView::ShowUI() and then proxy's availability callback is + // executed. However, we use parent view(BraveReadLaterSidePanelView) to have + // panel specific header view and this class becomes SidePanelEntry's Content. + // To make this content available when SidePanelWebUIVew::ShowUI() is called, + // this observes WebView's visibility because it's set as visible when + // ShowUI() is called. + // NOTE: If we use our own reading list page and it has loading spinner, maybe + // we can set `true` here. + SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(false); +} + +BraveSidePanelViewBase::~BraveSidePanelViewBase() = default; + +void BraveSidePanelViewBase::StartObservingWebWebViewVisibilityChange( + views::View* web_view) { + observation_.Observe(web_view); +} + +void BraveSidePanelViewBase::OnViewVisibilityChanged( + views::View* observed_view, + views::View* starting_view) { + // Once it becomes available, stop observing becuase its availablity is + // not changed from now on. + if (observed_view->GetVisible()) { + SidePanelUtil::GetSidePanelContentProxy(this)->SetAvailable(true); + observation_.Reset(); + } +} diff --git a/browser/ui/views/side_panel/brave_side_panel_view_base.h b/browser/ui/views/side_panel/brave_side_panel_view_base.h new file mode 100644 index 000000000000..4584f0a51301 --- /dev/null +++ b/browser/ui/views/side_panel/brave_side_panel_view_base.h @@ -0,0 +1,31 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_SIDE_PANEL_VIEW_BASE_H_ +#define BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_SIDE_PANEL_VIEW_BASE_H_ + +#include "base/scoped_observation.h" +#include "ui/views/view.h" +#include "ui/views/view_observer.h" + +class BraveSidePanelViewBase : public views::View, public views::ViewObserver { + public: + BraveSidePanelViewBase(); + ~BraveSidePanelViewBase() override; + BraveSidePanelViewBase(const BraveSidePanelViewBase&) = delete; + BraveSidePanelViewBase& operator=(const BraveSidePanelViewBase&) = delete; + + protected: + void StartObservingWebWebViewVisibilityChange(views::View* web_view); + + private: + // views::ViewObserver overrides: + void OnViewVisibilityChanged(views::View* observed_view, + views::View* starting_view) override; + + base::ScopedObservation observation_{this}; +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_SIDE_PANEL_BRAVE_SIDE_PANEL_VIEW_BASE_H_