Skip to content

Commit

Permalink
Adjusted split view tab's bounds/padding in vertical tab
Browse files Browse the repository at this point in the history
  • Loading branch information
simonhong committed Dec 19, 2024
1 parent e6c84c6 commit 5124a7d
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 18 deletions.
5 changes: 5 additions & 0 deletions browser/ui/views/tabs/brave_tab.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ std::u16string BraveTab::GetTooltipText(const gfx::Point& p) const {
return Tab::GetTooltipText(p);
}

void BraveTab::UpdateBorder() {
// In vertical tab, different border is used based on split view state.
SetBorder(views::CreateEmptyBorder(tab_style_views()->GetContentsInsets()));
}

int BraveTab::GetWidthOfLargestSelectableRegion() const {
// Assume the entire region except the area that alert indicator/close buttons
// occupied is available for click-to-select.
Expand Down
1 change: 1 addition & 0 deletions browser/ui/views/tabs/brave_tab.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class BraveTab : public Tab {

// Tab:
std::u16string GetTooltipText(const gfx::Point& p) const override;
void UpdateBorder() override;

// Overridden because we moved alert button to left side in the tab whereas
// upstream put it on right side. Need to consider this change for calculating
Expand Down
29 changes: 28 additions & 1 deletion browser/ui/views/tabs/brave_tab_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,6 @@ std::optional<BrowserRootView::DropIndex> BraveTabContainer::GetDropIndex(
continue;
}


const bool is_tab_pinned = tab->data().pinned;

// When dropping text or links onto pinned tabs, we need to take the
Expand Down Expand Up @@ -601,10 +600,12 @@ void BraveTabContainer::HandleDragExited() {
}

void BraveTabContainer::OnTileTabs(const TabTile& tile) {
UpdateTabsBorderInTile(tile);
SchedulePaint();
}

void BraveTabContainer::OnDidBreakTile(const TabTile& tile) {
UpdateTabsBorderInTile(tile);
SchedulePaint();
}

Expand Down Expand Up @@ -757,5 +758,31 @@ void BraveTabContainer::SetDropArrow(
drop_arrow_->SetWindowBounds(drop_bounds);
}

bool BraveTabContainer::IsPinnedTabContainer() const {
return tabs_view_model_.view_size() > 0 && tabs_view_model_.view_at(0)->data().pinned;
}

void BraveTabContainer::UpdateTabsBorderInTile(const TabTile& tile) {
auto* tab_strip_model = tab_slot_controller_->GetBrowser()->tab_strip_model();
const int offset =
IsPinnedTabContainer() ? 0 : tab_strip_model->IndexOfFirstNonPinnedTab();

auto tab1_index = tab_strip_model->GetIndexOfTab(tile.first) - offset;
auto tab2_index = tab_strip_model->GetIndexOfTab(tile.second) - offset;

if (!controller_->IsValidModelIndex(tab1_index) ||
!controller_->IsValidModelIndex(tab2_index)) {
// In case the tiled tab is not in this container, this can happen.
// For instance, this container is for pinned tabs but tabs in the tile
// are unpinned.
return;
}

auto* tab1 = GetTabAtModelIndex(tab1_index);
auto* tab2 = GetTabAtModelIndex(tab2_index);
tab1->UpdateBorder();
tab2->UpdateBorder();
}

BEGIN_METADATA(BraveTabContainer)
END_METADATA
3 changes: 3 additions & 0 deletions browser/ui/views/tabs/brave_tab_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class BraveTabContainer : public TabContainerImpl,
bool drop_in_group,
bool* is_beneath);

bool IsPinnedTabContainer() const;
void UpdateTabsBorderInTile(const TabTile& tile);

base::flat_set<Tab*> closing_tabs_;

raw_ptr<TabDragContextBase> drag_context_;
Expand Down
46 changes: 29 additions & 17 deletions browser/ui/views/tabs/brave_tab_style_views.inc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace {

using tabs::features::HorizontalTabsUpdateEnabled;

constexpr auto kPaddingForVerticalTabInTile = 4;

// Returns a value indicating if the browser frame view is "condensed", i.e.
// that its frame border is somehow collapsed, as in fullscreen or when
// maximized, or in Linux when caption buttons and the title bar are not
Expand Down Expand Up @@ -215,13 +217,13 @@ SkPath BraveVerticalTabStyle::GetPath(
}
}

if (IsTabTiled(tab()) && path_type != TabStyle::PathType::kHitTest) {
if (!is_pinned && IsTabTiled(tab()) && path_type != TabStyle::PathType::kHitTest) {
if (ShouldShowVerticalTabs()) {
constexpr auto kPaddingForVerticalTab = 4;
tab_top += scale * kPaddingForVerticalTab;
tab_bottom -= scale * kPaddingForVerticalTab;
tab_left += scale * kPaddingForVerticalTab;
tab_right -= scale * kPaddingForVerticalTab;
tab()->controller()->IsFirstTabInTile(tab())
? tab_top += scale* kPaddingForVerticalTabInTile
: tab_bottom -= scale * kPaddingForVerticalTabInTile;
tab_left += scale * kPaddingForVerticalTabInTile;
tab_right -= scale * kPaddingForVerticalTabInTile;
} else {
// Give 2 dip more padding when tab is in tile.
constexpr auto kPaddingForHorizontalTab = 2;
Expand Down Expand Up @@ -251,20 +253,30 @@ SkPath BraveVerticalTabStyle::GetPath(
}

gfx::Insets BraveVerticalTabStyle::GetContentsInsets() const {
if (!HorizontalTabsUpdateEnabled()) {
return BraveTabStyleViews::GetContentsInsets();
const bool is_pinned = tab()->data().pinned;
auto insets = tab_style()->GetContentsInsets();

if (!is_pinned && ShouldShowVerticalTabs() && IsTabTiled(tab())) {
const bool is_first_tab = tab()->controller()->IsFirstTabInTile(tab());
return insets + gfx::Insets::TLBR(
is_first_tab ? kPaddingForVerticalTabInTile : 0, 0,
is_first_tab ? 0 : kPaddingForVerticalTabInTile, 0);
}

// Ignore any stroke widths when determining the horizontal contents insets.
// To make contents vertically align evenly regardless of overlap in non
// vertical tab, use it as bottom inset in a tab as it's hidden by
// overlapping.
const int bottom_inset = ShouldShowVerticalTabs()
? 0
: GetLayoutConstant(TABSTRIP_TOOLBAR_OVERLAP);
if (HorizontalTabsUpdateEnabled()) {
// Ignore any stroke widths when determining the horizontal contents insets.
// To make contents vertically align evenly regardless of overlap in non
// vertical tab, use it as bottom inset in a tab as it's hidden by
// overlapping.
return insets +
gfx::Insets::TLBR(0, 0,
ShouldShowVerticalTabs()
? 0
: GetLayoutConstant(TABSTRIP_TOOLBAR_OVERLAP),
0);
}

return tab_style()->GetContentsInsets() +
gfx::Insets::TLBR(0, 0, bottom_inset, 0);
return BraveTabStyleViews::GetContentsInsets();
}

TabStyle::SeparatorBounds BraveVerticalTabStyle::GetSeparatorBounds(
Expand Down
1 change: 1 addition & 0 deletions chromium_src/chrome/browser/ui/views/tabs/tab.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class BraveTab;
kMinimumContentsWidthForCloseButtons = 55; \
friend class ::BraveTabTest; \
friend class ::BraveTab; \
virtual void UpdateBorder() {} \
static constexpr int kMinimumContentsWidthForCloseButtons_UnUsed

#define GetWidthOfLargestSelectableRegion \
Expand Down

0 comments on commit 5124a7d

Please sign in to comment.