Skip to content

Commit

Permalink
Merge pull request #20044 from brave/ksmith-h-tabs
Browse files Browse the repository at this point in the history
Update horizontal tabs styling
  • Loading branch information
zenparsing authored Sep 25, 2023
2 parents 3b7f9ca + b5a338c commit cd701f7
Show file tree
Hide file tree
Showing 36 changed files with 624 additions and 91 deletions.
28 changes: 18 additions & 10 deletions browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,16 +361,24 @@
#endif // BUILDFLAG(IS_ANDROID)

#if !BUILDFLAG(IS_ANDROID)
#define BRAVE_SHARED_PINNED_TABS \
EXPAND_FEATURE_ENTRIES({ \
"brave-shared-pinned-tabs", \
"Shared pinned tab", \
"Pinned tabs are shared across windows", \
kOsWin | kOsMac | kOsLinux, \
FEATURE_VALUE_TYPE(tabs::features::kBraveSharedPinnedTabs), \
})
#define BRAVE_TABS_FEATURE_ENTRIES \
EXPAND_FEATURE_ENTRIES( \
{ \
"brave-shared-pinned-tabs", \
"Shared pinned tab", \
"Pinned tabs are shared across windows", \
kOsWin | kOsMac | kOsLinux, \
FEATURE_VALUE_TYPE(tabs::features::kBraveSharedPinnedTabs), \
}, \
{ \
"brave-horizontal-tabs-update", \
"Updated horizontal tabs design", \
"Updates the look and feel or horizontal tabs", \
kOsWin | kOsMac | kOsLinux, \
FEATURE_VALUE_TYPE(tabs::features::kBraveHorizontalTabsUpdate), \
})
#else
#define BRAVE_SHARED_PINNED_TABS
#define BRAVE_TABS_FEATURE_ENTRIES
#endif

#if BUILDFLAG(ENABLE_AI_CHAT)
Expand Down Expand Up @@ -887,7 +895,7 @@
BRAVE_BACKGROUND_VIDEO_PLAYBACK_ANDROID \
BRAVE_SAFE_BROWSING_ANDROID \
BRAVE_CHANGE_ACTIVE_TAB_ON_SCROLL_EVENT_FEATURE_ENTRIES \
BRAVE_SHARED_PINNED_TABS \
BRAVE_TABS_FEATURE_ENTRIES \
BRAVE_AI_CHAT \
BRAVE_AI_CHAT_HISTORY \
LAST_BRAVE_FEATURE_ENTRIES_ITEM // Keep it as the last item.
Expand Down
13 changes: 12 additions & 1 deletion browser/ui/brave_layout_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@

#include "brave/browser/ui/brave_layout_constants.h"

#include "brave/browser/ui/tabs/features.h"
#include "chrome/browser/ui/layout_constants.h"
#include "ui/base/pointer/touch_ui_controller.h"

using tabs::features::HorizontalTabsUpdateEnabled;

// Returns a |nullopt| if the UI color is not handled by Brave.
absl::optional<int> GetBraveLayoutConstant(LayoutConstant constant) {
const bool touch = ui::TouchUiController::Get()->touch_ui();
Expand All @@ -18,7 +21,15 @@ absl::optional<int> GetBraveLayoutConstant(LayoutConstant constant) {
// ui::MaterialDesignController::IsNewerMaterialUi();
switch (constant) {
case TAB_HEIGHT: {
return (touch ? 41 : 30) + GetLayoutConstant(TABSTRIP_TOOLBAR_OVERLAP);
const int tab_height = HorizontalTabsUpdateEnabled() ? 36 : 30;
return (touch ? 41 : tab_height) +
GetLayoutConstant(TABSTRIP_TOOLBAR_OVERLAP);
}
case TABSTRIP_TOOLBAR_OVERLAP: {
if (!HorizontalTabsUpdateEnabled()) {
return absl::nullopt;
}
return 0;
}
case TAB_SEPARATOR_HEIGHT: {
return 24;
Expand Down
2 changes: 2 additions & 0 deletions browser/ui/color/brave_color_mixer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ void AddChromeLightThemeColorMixer(ui::ColorProvider* provider,
mixer[kColorTabForegroundActiveFrameActive] = {kLightToolbarIcon};
mixer[kColorTabForegroundInactiveFrameActive] = {
kColorTabForegroundActiveFrameActive};
mixer[kColorTabStrokeFrameActive] = {SkColorSetA(SK_ColorBLACK, 0.07 * 255)};
mixer[kColorTabStrokeFrameInactive] = {kColorTabStrokeFrameActive};
mixer[kColorToolbar] = {kLightToolbar};
mixer[kColorToolbarButtonIcon] = {kColorTabForegroundActiveFrameActive};
mixer[kColorToolbarButtonIconInactive] = {
Expand Down
2 changes: 2 additions & 0 deletions browser/ui/tabs/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ source_set("tabs") {

if (!is_android) {
sources = [
"brave_tab_layout_constants.h",
"brave_tab_menu_model.cc",
"brave_tab_menu_model.h",
"brave_tab_prefs.cc",
"brave_tab_prefs.h",
"brave_tab_strip_model.cc",
"brave_tab_strip_model.h",
"brave_tab_style.h",
"brave_vertical_tab_color_mixer.cc",
"brave_vertical_tab_color_mixer.h",
"features.cc",
Expand Down
54 changes: 54 additions & 0 deletions browser/ui/tabs/brave_tab_layout_constants.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* 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_TABS_BRAVE_TAB_LAYOUT_CONSTANTS_H_
#define BRAVE_BROWSER_UI_TABS_BRAVE_TAB_LAYOUT_CONSTANTS_H_

namespace brave_tabs {

// Horizontal tab layout:
//
// The upstream tab implemenation assumes that tab view bounds overlap. In order
// to create a gap between tabs without violating these assumptions, tabs views
// are given a small overlap. Rounded tab rectangles are drawn centered and
// inset horizontally by an amount that will create the required visual gap.

// The amount of space before the first tab.
constexpr int kHorizontalTabStripLeftMargin = 8;

// The amount of vertical spacing between the top and bottom of tabs and the
// bounds of the tab strip region. The portion of this space below tabs will be
// occupied by tab group underlines.
constexpr int kHorizontalTabStripVerticalSpacing = 4;

// The visual gap between tabs.
constexpr int kHorizontalTabGap = 4;

// The amount of overlap between tabs. Based on upstream assumptions, tab views
// must have a non-negative overlap. Furthermore, tab separators will not render
// correctly if the tab overlap is zero.
constexpr int kHorizontalTabOverlap = 2;

// The horizontal difference between the edge of the tab view and the visual
// edge of the rendered tab.
constexpr int kHorizontalTabInset =
(kHorizontalTabGap + kHorizontalTabOverlap) / 2;

// The content padding within a tab.
constexpr int kHorizontalTabPadding = 6;

// The horizontal difference between the visual edge of a tab group and the
// bounds of the group underline.
constexpr int kHorizontalGroupUnderlineInset = 2;

// The tab border radius.
constexpr int kTabBorderRadius = 8;

// The size of the group header slot when the title is empty.
constexpr int kEmptyGroupTitleSize = 22;

} // namespace brave_tabs

#endif // BRAVE_BROWSER_UI_TABS_BRAVE_TAB_LAYOUT_CONSTANTS_H_
57 changes: 57 additions & 0 deletions browser/ui/tabs/brave_tab_style.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/* 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_TABS_BRAVE_TAB_STYLE_H_
#define BRAVE_BROWSER_UI_TABS_BRAVE_TAB_STYLE_H_

#include "brave/browser/ui/tabs/brave_tab_layout_constants.h"
#include "brave/browser/ui/tabs/features.h"
#include "chrome/browser/ui/layout_constants.h"
#include "ui/gfx/geometry/insets.h"

// A subclass of TabStyle used to customize tab layout and visuals. It is
// implemented as a template because it must be included in the source file
// override before the base class definition.
template <typename TabStyleBase>
class BraveTabStyle : public TabStyleBase {
public:
int GetTabOverlap() const override {
if (!tabs::features::HorizontalTabsUpdateEnabled()) {
return TabStyleBase::GetTabOverlap();
}
return brave_tabs::kHorizontalTabOverlap;
}

int GetTopCornerRadius() const override {
if (!tabs::features::HorizontalTabsUpdateEnabled()) {
return TabStyleBase::GetTopCornerRadius();
}
return brave_tabs::kTabBorderRadius;
}

int GetBottomCornerRadius() const override {
if (!tabs::features::HorizontalTabsUpdateEnabled()) {
return TabStyleBase::GetBottomCornerRadius();
}
return brave_tabs::kTabBorderRadius;
}

gfx::Insets GetContentsInsets() const override {
if (!tabs::features::HorizontalTabsUpdateEnabled()) {
return TabStyleBase::GetContentsInsets();
}
return gfx::Insets::VH(
0, brave_tabs::kHorizontalTabPadding + brave_tabs::kHorizontalTabInset);
}

int GetPinnedWidth() const override {
if (!tabs::features::HorizontalTabsUpdateEnabled()) {
return TabStyleBase::GetPinnedWidth();
}
return GetLayoutConstant(TAB_HEIGHT) + brave_tabs::kHorizontalTabInset * 2;
}
};

#endif // BRAVE_BROWSER_UI_TABS_BRAVE_TAB_STYLE_H_
8 changes: 8 additions & 0 deletions browser/ui/tabs/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,12 @@ BASE_FEATURE(kBraveSharedPinnedTabs,
"BraveSharedPinnedTabs",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kBraveHorizontalTabsUpdate,
"BraveHorizontalTabsUpdate",
base::FEATURE_DISABLED_BY_DEFAULT);

bool HorizontalTabsUpdateEnabled() {
return base::FeatureList::IsEnabled(kBraveHorizontalTabsUpdate);
}

} // namespace tabs::features
4 changes: 4 additions & 0 deletions browser/ui/tabs/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ BASE_DECLARE_FEATURE(kBraveChangeActiveTabOnScrollEvent);

BASE_DECLARE_FEATURE(kBraveSharedPinnedTabs);

BASE_DECLARE_FEATURE(kBraveHorizontalTabsUpdate);

bool HorizontalTabsUpdateEnabled();

} // namespace tabs::features

#endif // BRAVE_BROWSER_UI_TABS_FEATURES_H_
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "brave/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h"

#include "brave/browser/ui/tabs/brave_tab_prefs.h"
#include "brave/browser/ui/tabs/features.h"
#include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h"
#include "brave/browser/ui/views/frame/brave_window_frame_graphic.h"
#include "brave/browser/ui/views/tabs/vertical_tab_utils.h"
Expand Down Expand Up @@ -66,7 +67,13 @@
return 30;
}

return BrowserNonClientFrameViewMac::GetTopInset(restored);
if (!tabs::features::HorizontalTabsUpdateEnabled()) {
return BrowserNonClientFrameViewMac::GetTopInset(restored);
}

// The tab region view maintains its own padding, but insert a small gap to
// give a bit more room for the frame resize handle.
return 2;
}

bool BraveBrowserNonClientFrameViewMac::ShouldShowWindowTitleForVerticalTabs()
Expand Down
2 changes: 1 addition & 1 deletion browser/ui/views/frame/vertical_tab_strip_region_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ class VerticalTabNewTabButton : public BraveNewTabButton {
float scale,
bool extend_to_top) const override {
auto contents_bounds = GetContentsBounds();
const float radius = tabs::kUnpinnedTabBorderRadius * scale;
const float radius = GetCornerRadius() * scale;
SkPath path;
const gfx::Rect path_rect(origin.x(), origin.y(),
contents_bounds.width() * scale,
Expand Down
1 change: 1 addition & 0 deletions browser/ui/views/tabs/brave_compound_tab_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ void BraveCompoundTabContainer::SetScrollEnabled(bool enabled) {

if (enabled) {
scroll_view_ = AddChildView(std::make_unique<CustomScrollView>());
scroll_view_->SetBackgroundThemeColorId(kColorToolbar);
auto* contents_view =
scroll_view_->SetContents(std::make_unique<ContentsView>(this));
contents_view->AddChildView(base::to_address(unpinned_tab_container_));
Expand Down
40 changes: 33 additions & 7 deletions browser/ui/views/tabs/brave_new_tab_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,26 @@
#include "brave/browser/ui/views/tabs/brave_new_tab_button.h"

#include <algorithm>
#include <memory>
#include <utility>

#include "brave/browser/ui/color/brave_color_id.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/ui/layout_constants.h"
#include "brave/browser/ui/tabs/features.h"
#include "brave/components/vector_icons/vector_icons.h"
#include "chrome/browser/ui/views/tabs/new_tab_button.h"
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "ui/gfx/geometry/skia_conversions.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/scoped_canvas.h"
#include "ui/views/view_class_properties.h"

using tabs::features::HorizontalTabsUpdateEnabled;

// static
const gfx::Size BraveNewTabButton::kButtonSize{24, 24};
gfx::Size BraveNewTabButton::GetButtonSize() {
if (!HorizontalTabsUpdateEnabled()) {
return {24, 24};
}
return {28, 28};
}

// static
SkPath BraveNewTabButton::GetBorderPath(const gfx::Point& origin,
Expand All @@ -44,7 +51,7 @@ SkPath BraveNewTabButton::GetBorderPath(const gfx::Point& origin,

gfx::Size BraveNewTabButton::CalculatePreferredSize() const {
// Overriden so that we use Brave's custom button size
gfx::Size size = kButtonSize;
gfx::Size size = GetButtonSize();
const auto insets = GetInsets();
size.Enlarge(insets.width(), insets.height());
return size;
Expand All @@ -59,12 +66,31 @@ SkPath BraveNewTabButton::GetBorderPath(const gfx::Point& origin,

BraveNewTabButton::BraveNewTabButton(TabStrip* tab_strip,
PressedCallback callback)
: NewTabButton(tab_strip, std::move(callback)) {}
: NewTabButton(tab_strip, std::move(callback)) {
if (HorizontalTabsUpdateEnabled()) {
// Ensure that the new tab button is vertically centered within its flex
// layout container.
SetProperty(views::kCrossAxisAlignmentKey, views::LayoutAlignment::kCenter);
}
}

BraveNewTabButton::~BraveNewTabButton() = default;

void BraveNewTabButton::PaintIcon(gfx::Canvas* canvas) {
gfx::ScopedCanvas scoped_canvas(canvas);

if (HorizontalTabsUpdateEnabled()) {
// Instead of letting `NewTabButton` draw a "plus", paint a vector icon to
// the canvas in the center of the view.
constexpr int kIconSize = 16;
gfx::Rect bounds = GetContentsBounds();
canvas->Translate(gfx::Vector2d((bounds.width() - kIconSize) / 2,
(bounds.height() - kIconSize) / 2));
gfx::PaintVectorIcon(canvas, kLeoPlusAddIcon, kIconSize,
GetForegroundColor());
return;
}

// Shim base implementation's painting
// Overriden to fix chromium assumption that border radius
// will be 50% of width.
Expand Down
5 changes: 2 additions & 3 deletions browser/ui/views/tabs/brave_new_tab_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ class BraveNewTabButton : public NewTabButton {
// These static members are shared with BraveTabSearchButton
// TODO(sko) If we could make TabSearchButton inherit BraveNewTabButton,
// we might not need these any more.
static const gfx::Size kButtonSize;
static gfx::Size GetButtonSize();
static SkPath GetBorderPath(const gfx::Point& origin,
float scale,
bool extend_to_top,
int border_radius,
const gfx::Size& contents_bounds);

BraveNewTabButton(TabStrip* tab_strip, PressedCallback callback);
BraveNewTabButton(const BraveNewTabButton&) = delete;
BraveNewTabButton& operator=(const BraveNewTabButton&) = delete;
~BraveNewTabButton() override;

protected:
Expand Down
Loading

0 comments on commit cd701f7

Please sign in to comment.