Skip to content

Commit e1970dd

Browse files
simonhongsimonhong
committed
Sidebar V2 - SidebarContainerViewNew
1 parent 3f3bc17 commit e1970dd

11 files changed

+697
-41
lines changed

‎browser/ui/BUILD.gn‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,6 +1124,8 @@ source_set("ui") {
11241124
"views/sidebar/sidebar_button_view.h",
11251125
"views/sidebar/sidebar_container_view.cc",
11261126
"views/sidebar/sidebar_container_view.h",
1127+
"views/sidebar/sidebar_container_view_new.cc",
1128+
"views/sidebar/sidebar_container_view_new.h",
11271129
"views/sidebar/sidebar_control_view.cc",
11281130
"views/sidebar/sidebar_control_view.h",
11291131
"views/sidebar/sidebar_edit_item_bubble_delegate_view.cc",

‎browser/ui/views/frame/brave_browser_view.cc‎

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "brave/browser/ui/commands/accelerator_service_factory.h"
2626
#include "brave/browser/ui/page_action/brave_page_action_icon_type.h"
2727
#include "brave/browser/ui/page_info/features.h"
28+
#include "brave/browser/ui/sidebar/features.h"
2829
#include "brave/browser/ui/sidebar/sidebar_controller.h"
2930
#include "brave/browser/ui/sidebar/sidebar_utils.h"
3031
#include "brave/browser/ui/sidebar/sidebar_web_panel_controller.h"
@@ -41,6 +42,7 @@
4142
#include "brave/browser/ui/views/location_bar/brave_location_bar_view.h"
4243
#include "brave/browser/ui/views/omnibox/brave_omnibox_view_views.h"
4344
#include "brave/browser/ui/views/sidebar/sidebar_container_view.h"
45+
#include "brave/browser/ui/views/sidebar/sidebar_container_view_new.h"
4446
#include "brave/browser/ui/views/tabs/vertical_tab_utils.h"
4547
#include "brave/browser/ui/views/toolbar/bookmark_button.h"
4648
#include "brave/browser/ui/views/toolbar/brave_toolbar_view.h"
@@ -49,6 +51,7 @@
4951
#include "brave/components/brave_wallet/common/buildflags/buildflags.h"
5052
#include "brave/components/commands/common/features.h"
5153
#include "brave/components/constants/pref_names.h"
54+
#include "brave/components/sidebar/browser/constants.h"
5255
#include "brave/components/sidebar/common/features.h"
5356
#include "brave/components/speedreader/common/buildflags/buildflags.h"
5457
#include "brave/ui/color/nala/nala_color_id.h"
@@ -349,14 +352,20 @@ BraveBrowserView::BraveBrowserView(Browser* browser) : BrowserView(browser) {
349352
// Only normal window (tabbed) should have sidebar.
350353
const bool can_have_sidebar = sidebar::CanUseSidebar(browser_);
351354
if (can_have_sidebar) {
352-
// Wrap chromium side panel with our sidebar container
353-
auto original_side_panel =
354-
RemoveChildViewT(contents_height_side_panel_.get());
355-
sidebar_container_view_ =
356-
AddChildView(std::make_unique<SidebarContainerView>(
357-
browser_, SidePanelCoordinator::From(browser_),
358-
std::move(original_side_panel)));
359-
contents_height_side_panel_ = sidebar_container_view_->side_panel();
355+
if (base::FeatureList::IsEnabled(sidebar::features::kSidebarV2)) {
356+
// V2: Don't wrap panel - let upstream manage it
357+
sidebar_container_view_new_ =
358+
AddChildView(std::make_unique<SidebarContainerViewNew>(browser_));
359+
} else {
360+
// V1: Wrap chromium side panel with our sidebar container
361+
auto original_side_panel =
362+
RemoveChildViewT(contents_height_side_panel_.get());
363+
sidebar_container_view_ =
364+
AddChildView(std::make_unique<SidebarContainerView>(
365+
browser_, SidePanelCoordinator::From(browser_),
366+
std::move(original_side_panel)));
367+
contents_height_side_panel_ = sidebar_container_view_->side_panel();
368+
}
360369

361370
if (IsBraveWebViewRoundedCornersEnabled()) {
362371
sidebar_separator_view_ =
@@ -415,12 +424,15 @@ void BraveBrowserView::OnPreferenceChanged(const std::string& pref_name) {
415424
}
416425

417426
void BraveBrowserView::UpdateSideBarHorizontalAlignment() {
418-
DCHECK(sidebar_container_view_);
419-
420427
const bool on_left = !GetProfile()->GetPrefs()->GetBoolean(
421428
prefs::kSidePanelHorizontalAlignment);
422429

423-
sidebar_container_view_->SetSidebarOnLeft(on_left);
430+
if (sidebar_container_view_new_) {
431+
sidebar_container_view_new_->SetSidebarOnLeft(on_left);
432+
} else {
433+
CHECK(sidebar_container_view_);
434+
sidebar_container_view_->SetSidebarOnLeft(on_left);
435+
}
424436

425437
if (multi_contents_view_ &&
426438
base::FeatureList::IsEnabled(sidebar::features::kSidebarWebPanel)) {
@@ -452,20 +464,26 @@ BraveBrowserView::~BraveBrowserView() {
452464

453465
sidebar::Sidebar* BraveBrowserView::InitSidebar() {
454466
// Start Sidebar UI initialization.
455-
DCHECK(sidebar_container_view_);
456-
sidebar_container_view_->Init();
457-
467+
sidebar::Sidebar* sidebar = nullptr;
468+
if (sidebar_container_view_new_) {
469+
sidebar = sidebar_container_view_new_;
470+
sidebar_container_view_new_->Init();
471+
} else {
472+
CHECK(sidebar_container_view_);
473+
sidebar = sidebar_container_view_;
474+
sidebar_container_view_->Init();
475+
}
458476
// Ask BraveMultiContentsView for preparing web panel feature.
459477
if (multi_contents_view_ &&
460478
base::FeatureList::IsEnabled(sidebar::features::kSidebarWebPanel)) {
461479
GetBraveMultiContentsView()->SetWebPanelWidth(
462-
sidebar_container_view_->side_panel()->GetPreferredSize().width());
480+
sidebar::kDefaultSidePanelWidth);
463481
GetBraveMultiContentsView()->UseContentsContainerViewForWebPanel();
464482
}
465483

466484
UpdateSideBarHorizontalAlignment();
467485

468-
return sidebar_container_view_;
486+
return sidebar;
469487
}
470488

471489
void BraveBrowserView::ToggleSidebar() {
@@ -729,6 +747,8 @@ void BraveBrowserView::AddedToWidget() {
729747

730748
GetBrowserViewLayout()->set_contents_background(contents_background_view_);
731749
GetBrowserViewLayout()->set_sidebar_container(sidebar_container_view_);
750+
GetBrowserViewLayout()->set_sidebar_container_new(
751+
sidebar_container_view_new_);
732752
GetBrowserViewLayout()->set_sidebar_separator(sidebar_separator_view_);
733753

734754
UpdateWebViewRoundedCorners();
@@ -916,6 +936,10 @@ void BraveBrowserView::OnWidgetActivationChanged(views::Widget* widget,
916936
if (sidebar_container_view_) {
917937
sidebar_container_view_->UpdateSidebarItemsState();
918938
}
939+
940+
if (sidebar_container_view_new_) {
941+
sidebar_container_view_new_->UpdateSidebarItemsState();
942+
}
919943
}
920944

921945
void BraveBrowserView::OnWidgetWindowModalVisibilityChanged(
@@ -1062,6 +1086,10 @@ void BraveBrowserView::UpdateSidebarBorder() {
10621086
if (sidebar_container_view_) {
10631087
sidebar_container_view_->UpdateBorder();
10641088
}
1089+
1090+
if (sidebar_container_view_new_) {
1091+
sidebar_container_view_new_->UpdateBorder();
1092+
}
10651093
}
10661094

10671095
void BraveBrowserView::OnActiveTabChanged(content::WebContents* old_contents,
@@ -1184,6 +1212,10 @@ void BraveBrowserView::HandleBrowserWindowMouseEvent(
11841212
sidebar_container_view_->ShowSidebarOnMouseOver(point_in_screen);
11851213
}
11861214

1215+
if (sidebar_container_view_new_) {
1216+
sidebar_container_view_new_->ShowSidebarOnMouseOver(point_in_screen);
1217+
}
1218+
11871219
if (vertical_tab_strip_widget_delegate_view_ &&
11881220
tabs::utils::ShouldShowBraveVerticalTabs(browser())) {
11891221
vertical_tab_strip_widget_delegate_view_->vertical_tab_strip_region_view()
@@ -1207,6 +1239,10 @@ bool BraveBrowserView::IsWebPanelContents(content::WebContents* contents) {
12071239
}
12081240

12091241
bool BraveBrowserView::IsSidebarVisible() const {
1242+
if (sidebar_container_view_new_) {
1243+
return sidebar_container_view_new_->IsSidebarVisible();
1244+
}
1245+
12101246
return sidebar_container_view_ && sidebar_container_view_->IsSidebarVisible();
12111247
}
12121248

‎browser/ui/views/frame/brave_browser_view.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class BraveHelpBubbleHostView;
6767
class BraveMultiContentsView;
6868
class ContentsLayoutManager;
6969
class SidebarContainerView;
70+
class SidebarContainerViewNew;
7071
class SidePanelEntry;
7172
class VerticalTabStripWidgetDelegateView;
7273
class ViewShadow;
@@ -257,6 +258,7 @@ class BraveBrowserView : public BrowserView,
257258
bool closing_confirm_dialog_activated_ = false;
258259
raw_ptr<BraveHelpBubbleHostView> brave_help_bubble_host_view_ = nullptr;
259260
raw_ptr<SidebarContainerView> sidebar_container_view_ = nullptr;
261+
raw_ptr<SidebarContainerViewNew> sidebar_container_view_new_ = nullptr;
260262
raw_ptr<views::View> sidebar_separator_view_ = nullptr;
261263
raw_ptr<views::View> contents_background_view_ = nullptr;
262264
raw_ptr<views::View> vertical_tab_strip_host_view_ = nullptr;

‎browser/ui/views/frame/brave_browser_view_layout.cc‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "brave/browser/ui/views/frame/brave_browser_view.h"
1515
#include "brave/browser/ui/views/frame/brave_contents_view_util.h"
1616
#include "brave/browser/ui/views/sidebar/sidebar_container_view.h"
17+
#include "brave/browser/ui/views/sidebar/sidebar_container_view_new.h"
1718
#include "brave/browser/ui/views/tabs/vertical_tab_utils.h"
1819
#include "chrome/browser/profiles/profile.h"
1920
#include "chrome/browser/ui/browser.h"
@@ -53,21 +54,23 @@ BraveBrowserViewLayout::BraveBrowserViewLayout(
5354
BraveBrowserViewLayout::~BraveBrowserViewLayout() = default;
5455

5556
int BraveBrowserViewLayout::GetIdealSideBarWidth() const {
56-
if (!sidebar_container_) {
57+
views::View* sidebar_container = GetSidebarContainer();
58+
if (!sidebar_container) {
5759
return 0;
5860
}
5961

6062
return GetIdealSideBarWidth(views().contents_container->width() +
6163
GetContentsMargins().width() +
62-
sidebar_container_->width());
64+
sidebar_container->width());
6365
}
6466

6567
int BraveBrowserViewLayout::GetIdealSideBarWidth(int available_width) const {
66-
if (!sidebar_container_) {
68+
views::View* sidebar_container = GetSidebarContainer();
69+
if (!sidebar_container) {
6770
return 0;
6871
}
6972

70-
int sidebar_width = sidebar_container_->GetPreferredSize().width();
73+
int sidebar_width = sidebar_container->GetPreferredSize().width();
7174

7275
// The sidebar can take up the entire space for fullscreen.
7376
if (sidebar_width == std::numeric_limits<int>::max()) {
@@ -228,7 +231,8 @@ bool BraveBrowserViewLayout::IsImmersiveModeEnabledWithoutToolbar() const {
228231
}
229232

230233
void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) {
231-
if (!sidebar_container_) {
234+
views::View* sidebar_container = GetSidebarContainer();
235+
if (!sidebar_container) {
232236
return;
233237
}
234238

@@ -248,7 +252,7 @@ void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) {
248252
#endif
249253

250254
gfx::Rect separator_bounds;
251-
const bool on_left = sidebar_container_->sidebar_on_left();
255+
bool on_left = GetSidebarOnLeft();
252256
if (on_left) {
253257
contents_bounds.set_x(contents_bounds.x() + sidebar_bounds.width());
254258

@@ -283,10 +287,12 @@ void BraveBrowserViewLayout::LayoutSideBar(gfx::Rect& contents_bounds) {
283287
// will have margins if needed.
284288
panel_margins.set_left_right(0, 0);
285289
}
286-
sidebar_container_->side_panel()->SetProperty(views::kMarginsKey,
287-
panel_margins);
290+
if (sidebar_container_) {
291+
sidebar_container_->side_panel()->SetProperty(views::kMarginsKey,
292+
panel_margins);
293+
}
288294

289-
sidebar_container_->SetBoundsRect(
295+
sidebar_container->SetBoundsRect(
290296
views().browser_view->GetMirroredRect(sidebar_bounds));
291297

292298
if (sidebar_separator_) {
@@ -416,6 +422,30 @@ gfx::Insets BraveBrowserViewLayout::GetInsetsConsideringVerticalTabHost()
416422
return insets;
417423
}
418424

425+
views::View* BraveBrowserViewLayout::GetSidebarContainer() const {
426+
if (sidebar_container_) {
427+
return sidebar_container_;
428+
}
429+
430+
if (sidebar_container_new_) {
431+
return sidebar_container_new_;
432+
}
433+
434+
return nullptr;
435+
}
436+
437+
bool BraveBrowserViewLayout::GetSidebarOnLeft() const {
438+
if (sidebar_container_) {
439+
return sidebar_container_->sidebar_on_left();
440+
}
441+
442+
if (sidebar_container_new_) {
443+
return sidebar_container_new_->sidebar_on_left();
444+
}
445+
446+
NOTREACHED();
447+
}
448+
419449
#if BUILDFLAG(IS_MAC)
420450
gfx::Insets BraveBrowserViewLayout::AddFrameBorderInsets(
421451
const gfx::Insets& insets) const {

‎browser/ui/views/frame/brave_browser_view_layout.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "chrome/browser/ui/views/frame/layout/browser_view_layout_impl_old.h"
1212

1313
class SidebarContainerView;
14+
class SidebarContainerViewNew;
1415

1516
class BraveBrowserViewLayout : public BrowserViewLayoutImplOld {
1617
public:
@@ -31,6 +32,10 @@ class BraveBrowserViewLayout : public BrowserViewLayoutImplOld {
3132
sidebar_container_ = sidebar_container;
3233
}
3334

35+
void set_sidebar_container_new(SidebarContainerViewNew* sidebar_container) {
36+
sidebar_container_new_ = sidebar_container;
37+
}
38+
3439
void set_sidebar_separator(views::View* sidebar_separator) {
3540
sidebar_separator_ = sidebar_separator;
3641
}
@@ -57,6 +62,8 @@ class BraveBrowserViewLayout : public BrowserViewLayoutImplOld {
5762
bool ShouldPushBookmarkBarForVerticalTabs();
5863
gfx::Insets GetInsetsConsideringVerticalTabHost() const;
5964
void UpdateContentsContainerInsets(gfx::Rect& contents_container_bounds);
65+
views::View* GetSidebarContainer() const;
66+
bool GetSidebarOnLeft() const;
6067

6168
#if BUILDFLAG(IS_MAC)
6269
gfx::Insets AddFrameBorderInsets(const gfx::Insets& insets) const;
@@ -65,6 +72,7 @@ class BraveBrowserViewLayout : public BrowserViewLayoutImplOld {
6572

6673
raw_ptr<views::View, DanglingUntriaged> vertical_tab_strip_host_ = nullptr;
6774
raw_ptr<SidebarContainerView, DanglingUntriaged> sidebar_container_ = nullptr;
75+
raw_ptr<SidebarContainerViewNew> sidebar_container_new_ = nullptr;
6876
raw_ptr<views::View> sidebar_separator_ = nullptr;
6977
raw_ptr<views::View> contents_background_ = nullptr;
7078
};

‎browser/ui/views/side_panel/brave_side_panel_coordinator.cc‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "base/debug/crash_logging.h"
1414
#include "base/debug/dump_without_crashing.h"
1515
#include "base/logging.h"
16+
#include "brave/browser/ui/sidebar/sidebar_controller.h"
1617
#include "brave/browser/ui/sidebar/sidebar_service_factory.h"
1718
#include "brave/browser/ui/sidebar/sidebar_utils.h"
1819
#include "brave/browser/ui/views/frame/brave_browser_view.h"
@@ -22,6 +23,7 @@
2223
#include "brave/components/sidebar/browser/sidebar_service.h"
2324
#include "brave/grit/brave_generated_resources.h"
2425
#include "chrome/browser/profiles/profile.h"
26+
#include "chrome/browser/ui/browser_window/public/browser_window_features.h"
2527
#include "chrome/browser/ui/views/side_panel/side_panel_entry.h"
2628

2729
namespace {
@@ -103,7 +105,14 @@ void BraveSidePanelCoordinator::OnViewVisibilityChanged(
103105
visible);
104106

105107
if (update_items_state) {
106-
GetBraveBrowserView()->sidebar_container_view()->UpdateActiveItemState();
108+
auto* controller =
109+
browser_view_->browser()->GetFeatures().sidebar_controller();
110+
std::optional<sidebar::SidebarItem::BuiltInItemType> current_type;
111+
if (auto entry_id =
112+
GetCurrentEntryId(SidePanelEntry::PanelType::kContent)) {
113+
current_type = sidebar::BuiltInItemTypeFromSidePanelId(*entry_id);
114+
}
115+
controller->UpdateActiveItemState(current_type);
107116
}
108117
}
109118

@@ -163,7 +172,9 @@ void BraveSidePanelCoordinator::PopulateSidePanel(
163172

164173
// Notify to give opportunity to observe another panel entries from
165174
// global or active tab's contextual registry.
166-
GetBraveBrowserView()->sidebar_container_view()->WillShowSidePanel();
175+
if (auto* sidebar = GetBraveBrowserView()->sidebar_container_view()) {
176+
sidebar->WillShowSidePanel();
177+
}
167178
SidePanelCoordinator::PopulateSidePanel(supress_animations, unique_key,
168179
std::move(open_trigger), entry,
169180
std::move(content_view));

‎browser/ui/views/sidebar/sidebar_container_view.cc‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -831,18 +831,6 @@ void SidebarContainerView::OnTabWillBeRemoved(tabs::TabInterface* tab,
831831
}
832832
}
833833

834-
void SidebarContainerView::UpdateActiveItemState() {
835-
DVLOG(1) << "Update active item state";
836-
837-
auto* controller = browser_->GetFeatures().sidebar_controller();
838-
std::optional<sidebar::SidebarItem::BuiltInItemType> current_type;
839-
if (auto entry_id = side_panel_coordinator_->GetCurrentEntryId(
840-
SidePanelEntry::PanelType::kContent)) {
841-
current_type = sidebar::BuiltInItemTypeFromSidePanelId(*entry_id);
842-
}
843-
controller->UpdateActiveItemState(current_type);
844-
}
845-
846834
void SidebarContainerView::OnTabStripModelChanged(
847835
TabStripModel* tab_strip_model,
848836
const TabStripModelChange& change,

‎browser/ui/views/sidebar/sidebar_container_view.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,6 @@ class SidebarContainerView : public sidebar::Sidebar,
131131
const TabStripSelectionChange& selection) override;
132132
void OnTabWillBeRemoved(tabs::TabInterface* tab, int index) override;
133133

134-
void UpdateActiveItemState();
135-
136134
private:
137135
friend class sidebar::SidebarBrowserTest;
138136

0 commit comments

Comments
 (0)