Skip to content

Commit

Permalink
Fix BraveCompoundTabContainer layout for vertical tab strip (#16885)
Browse files Browse the repository at this point in the history
In the latest Chromium, CompoundTabContainer doesn't use FlexLayout.
But in our use case, FlexLayout does good so make things work based
on the FlexLayout.

Related upstream changes:
https://chromium-review.googlesource.com/c/chromium/src/+/4081180
https://chromium-review.googlesource.com/c/chromium/src/+/3956548
  • Loading branch information
sangwoo108 authored and emerick committed Jan 31, 2023
1 parent bed7b40 commit 7baea5c
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 10 deletions.
82 changes: 75 additions & 7 deletions browser/ui/views/tabs/brave_compound_tab_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,23 @@ BraveCompoundTabContainer::BraveCompoundTabContainer(
void BraveCompoundTabContainer::SetAvailableWidthCallback(
base::RepeatingCallback<int()> available_width_callback) {
CompoundTabContainer::SetAvailableWidthCallback(available_width_callback);
if (!available_width_callback || tabs::features::ShouldShowVerticalTabs(
tab_slot_controller_->GetBrowser())) {
// Unlike Chromium Impl, Just pass the `available_width_callback` to
// child containers when it's vertical tabs or we're trying to clear
// the callback.
pinned_tab_container_->SetAvailableWidthCallback(available_width_callback_);
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs)) {
return;
}

if (tabs::features::ShouldShowVerticalTabs(
tab_slot_controller_->GetBrowser()) &&
available_width_callback) {
pinned_tab_container_->SetAvailableWidthCallback(
base::BindRepeating(&views::View::width, base::Unretained(this)));
unpinned_tab_container_->SetAvailableWidthCallback(
available_width_callback_);
base::BindRepeating(&views::View::width, base::Unretained(this)));
return;
}

// Upstream's compound tab container doesn't use this.
pinned_tab_container_->SetAvailableWidthCallback(base::NullCallback());
unpinned_tab_container_->SetAvailableWidthCallback(base::NullCallback());
}

BraveCompoundTabContainer::~BraveCompoundTabContainer() = default;
Expand Down Expand Up @@ -69,6 +77,13 @@ void BraveCompoundTabContainer::TransferTabBetweenContainers(
const bool was_pinned = to_model_index < NumPinnedTabs();
CompoundTabContainer::TransferTabBetweenContainers(from_model_index,
to_model_index);
if (!ShouldShowVerticalTabs()) {
return;
}

// Override transfer animation so that it goes well with vertical tab strip.
CompleteAnimationAndLayout();

const bool is_pinned = to_model_index < NumPinnedTabs();
bool layout_dirty = false;
if (is_pinned && !pinned_tab_container_->GetVisible()) {
Expand All @@ -78,6 +93,13 @@ void BraveCompoundTabContainer::TransferTabBetweenContainers(
layout_dirty = true;
}

// Animate tab from left to right.
Tab* tab = GetTabAtModelIndex(to_model_index);
tab->SetPosition({-tab->width(), 0});
TabContainer& to_container =
*(is_pinned ? pinned_tab_container_ : unpinned_tab_container_);
to_container.AnimateToIdealBounds();

if (was_pinned != is_pinned) {
// After transferring a tab from one to another container, we should layout
// the previous container.
Expand All @@ -92,5 +114,51 @@ void BraveCompoundTabContainer::TransferTabBetweenContainers(
Layout();
}

void BraveCompoundTabContainer::Layout() {
if (!ShouldShowVerticalTabs()) {
CompoundTabContainer::Layout();
return;
}

// We use flex layout manager so let it do its job.
views::View::Layout();
}

gfx::Size BraveCompoundTabContainer::CalculatePreferredSize() const {
if (!ShouldShowVerticalTabs()) {
return CompoundTabContainer::CalculatePreferredSize();
}

// We use flex layout manager so let it do its job.
return views::View::CalculatePreferredSize();
}

gfx::Size BraveCompoundTabContainer::GetMinimumSize() const {
if (!ShouldShowVerticalTabs()) {
return CompoundTabContainer::GetMinimumSize();
}

// We use flex layout manager so let it do its job.
return views::View::GetMinimumSize();
}

views::SizeBounds BraveCompoundTabContainer::GetAvailableSize(
const views::View* child) const {
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs) ||
!tabs::features::ShouldShowVerticalTabs(
tab_slot_controller_->GetBrowser())) {
return CompoundTabContainer::GetAvailableSize(child);
}

return views::SizeBounds(views::SizeBound(width()),
/*height=*/views::SizeBound());
}

bool BraveCompoundTabContainer::ShouldShowVerticalTabs() const {
return base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs) &&
tabs::features::ShouldShowVerticalTabs(
tab_slot_controller_->GetBrowser());
}

BEGIN_METADATA(BraveCompoundTabContainer, CompoundTabContainer)
END_METADATA
6 changes: 5 additions & 1 deletion browser/ui/views/tabs/brave_compound_tab_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@ class BraveCompoundTabContainer : public CompoundTabContainer {
base::RepeatingCallback<int()> available_width_callback) override;
void TransferTabBetweenContainers(int from_model_index,
int to_model_index) override;
void Layout() override;
gfx::Size CalculatePreferredSize() const override;
gfx::Size GetMinimumSize() const override;
views::SizeBounds GetAvailableSize(const views::View* child) const override;

private:
void UpdateLayout();
bool ShouldShowVerticalTabs() const;

base::raw_ref<TabSlotController> tab_slot_controller_;
};
Expand Down
4 changes: 2 additions & 2 deletions browser/ui/views/tabs/brave_tab_strip.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ void BraveTabStrip::UpdateTabContainer() {
}

if (should_use_compound_tab_container) {
tab_container_->SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical);
// Upstream's compound tab container lay out its sub containers manually.
tab_container_->SetLayoutManager(nullptr);
}
}
}
Expand Down

0 comments on commit 7baea5c

Please sign in to comment.