Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support hiding window title when using vertical tab strip #15523

Merged
merged 12 commits into from
Oct 25, 2022
8 changes: 8 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -797,15 +797,23 @@ Or change later at <ph name="SETTINGS_EXTENIONS_LINK">$2<ex>brave://settings/ext
Brave is made available to you under the <ph name="BEGIN_LINK_MPL">&lt;a target="_blank" href="$1"&gt;</ph>Mozilla Public License 2.0<ph name="END_LINK_MPL">&lt;/a&gt;</ph> (MPL) and includes <ph name="BEGIN_LINK_OSS">&lt;a target="_blank" href="$2"&gt;</ph>open source software<ph name="END_LINK_OSS">&lt;/a&gt;</ph> under a variety of other licenses.
You can read <ph name="BEGIN_LINK_BUILD_INSTRUCTIONS">&lt;a target="_blank" href="$3"&gt;</ph>instructions on how to download and build for yourself<ph name="END_LINK_BUILD_INSTRUCTIONS">&lt;/a&gt;</ph> the specific <ph name="BEGIN_LINK_SOURCE_CODE">&lt;a target="_blank" href="$4"&gt;</ph>source code used to create this copy<ph name="END_LINK_SOURCE_CODE">&lt;/a&gt;</ph>.
</message>

<!-- Brave's tab context menu -->
<if expr="not use_titlecase">
<message name="IDS_TAB_CXMENU_BOOKMARK_ALL_TABS" desc="The label of the tab context menu item for creating a bookmark folder containing an entry for each open tab.">
Bookmark all tabs...
</message>
<message name="IDS_TAB_CXMENU_SHOW_TITLE_BAR" desc="The label of the tab context menu item for showing window title when vertical tab strip is enabled.">
Show title bar
</message>
</if>
<if expr="use_titlecase">
<message name="IDS_TAB_CXMENU_BOOKMARK_ALL_TABS" desc="In Title Case: The label of the tab context menu item for creating a bookmark folder containing an entry for each open tab.">
Bookmark All Tabs...
</message>
<message name="IDS_TAB_CXMENU_SHOW_TITLE_BAR" desc="The label of the tab context menu item for showing window title when vertical tab strip is enabled.">
Show title bar
</message>
</if>

<!-- Brave VPN -->
Expand Down
4 changes: 4 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ source_set("ui") {
"views/frame/brave_browser_view.h",
"views/frame/brave_browser_view_layout.cc",
"views/frame/brave_browser_view_layout.h",
"views/frame/brave_non_client_hit_test_helper.cc",
"views/frame/brave_non_client_hit_test_helper.h",
"views/frame/brave_opaque_browser_frame_view.cc",
"views/frame/brave_opaque_browser_frame_view.h",
"views/frame/brave_window_frame_graphic.cc",
Expand Down Expand Up @@ -290,6 +292,8 @@ source_set("ui") {

if (is_mac) {
sources += [
"views/frame/brave_browser_frame_mac.h",
"views/frame/brave_browser_frame_mac.mm",
"views/frame/brave_browser_non_client_frame_view_mac.h",
"views/frame/brave_browser_non_client_frame_view_mac.mm",
]
Expand Down
8 changes: 8 additions & 0 deletions browser/ui/browser_commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "brave/app/brave_command_ids.h"
#include "brave/browser/debounce/debounce_service_factory.h"
#include "brave/browser/net/brave_query_filter.h"
#include "brave/browser/ui/tabs/brave_tab_prefs.h"
#include "brave/browser/url_sanitizer/url_sanitizer_service_factory.h"
#include "brave/components/brave_vpn/buildflags/buildflags.h"
#include "brave/components/constants/pref_names.h"
Expand Down Expand Up @@ -225,4 +226,11 @@ void CopyLinkWithStrictCleaning(Browser* browser, const GURL& url) {
scw.WriteText(base::UTF8ToUTF16(final_url.spec()));
}

void ToggleWindowTitleVisibilityForVerticalTabs(Browser* browser) {
auto* prefs = browser->profile()->GetOriginalProfile()->GetPrefs();
prefs->SetBoolean(
brave_tabs::kVerticalTabsShowTitleOnWindow,
!prefs->GetBoolean(brave_tabs::kVerticalTabsShowTitleOnWindow));
}

} // namespace brave
2 changes: 2 additions & 0 deletions browser/ui/browser_commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ void CopySanitizedURL(Browser* browser, const GURL& url);
// - URLSanitizerService
void CopyLinkWithStrictCleaning(Browser* browser, const GURL& url);

void ToggleWindowTitleVisibilityForVerticalTabs(Browser* browser);

} // namespace brave


Expand Down
1 change: 1 addition & 0 deletions browser/ui/tabs/brave_tab_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,5 @@ void BraveTabMenuModel::Build() {
AddSeparator(ui::NORMAL_SEPARATOR);
AddItemWithStringId(CommandRestoreTab, GetRestoreTabCommandStringId());
AddItemWithStringId(CommandBookmarkAllTabs, IDS_TAB_CXMENU_BOOKMARK_ALL_TABS);
AddCheckItemWithStringId(CommandShowTitleBar, IDS_TAB_CXMENU_SHOW_TITLE_BAR);
}
1 change: 1 addition & 0 deletions browser/ui/tabs/brave_tab_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class BraveTabMenuModel : public TabMenuModel {
CommandStart = TabStripModel::CommandLast,
CommandRestoreTab,
CommandBookmarkAllTabs,
CommandShowTitleBar,
CommandLast,
};

Expand Down
3 changes: 3 additions & 0 deletions browser/ui/tabs/brave_tab_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ namespace brave_tabs {

const char kTabHoverMode[] = "brave.tabs.hover_mode";
const char kVerticalTabsCollapsed[] = "brave.tabs.vertical_tabs_collapsed";
const char kVerticalTabsShowTitleOnWindow[] =
"brave.tabs.vertical_tabs_show_title_on_window";

void RegisterBraveProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterIntegerPref(kTabHoverMode, TabHoverMode::CARD);
registry->RegisterBooleanPref(kVerticalTabsCollapsed, false);
registry->RegisterBooleanPref(kVerticalTabsShowTitleOnWindow, true);
}

bool AreTooltipsEnabled(PrefService* prefs) {
Expand Down
1 change: 1 addition & 0 deletions browser/ui/tabs/brave_tab_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum TabHoverMode { TOOLTIP = 0, CARD = 1, CARD_WITH_PREVIEW = 2 };

extern const char kTabHoverMode[];
extern const char kVerticalTabsCollapsed[];
extern const char kVerticalTabsShowTitleOnWindow[];

void RegisterBraveProfilePrefs(PrefRegistrySimple* registry);

Expand Down
26 changes: 26 additions & 0 deletions browser/ui/views/frame/brave_browser_frame_mac.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_FRAME_MAC_H_
#define BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_FRAME_MAC_H_

#include "chrome/browser/ui/views/frame/browser_frame_mac.h"

class Browser;

class BraveBrowserFrameMac : public BrowserFrameMac {
public:
BraveBrowserFrameMac(BrowserFrame* browser_frame, BrowserView* browser_view);
~BraveBrowserFrameMac() override;

// BrowserFrameMac:
void GetWindowFrameTitlebarHeight(bool* override_titlebar_height,
float* titlebar_height) override;

private:
raw_ptr<Browser> browser_;
};

#endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_FRAME_MAC_H_
29 changes: 29 additions & 0 deletions browser/ui/views/frame/brave_browser_frame_mac.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */

#include "brave/browser/ui/views/frame/brave_browser_frame_mac.h"

#include "brave/browser/ui/views/tabs/features.h"
#include "chrome/browser/ui/views/frame/browser_view.h"

BraveBrowserFrameMac::BraveBrowserFrameMac(BrowserFrame* browser_frame,
BrowserView* browser_view)
: BrowserFrameMac(browser_frame, browser_view),
browser_(browser_view->browser()) {}

BraveBrowserFrameMac::~BraveBrowserFrameMac() = default;

void BraveBrowserFrameMac::GetWindowFrameTitlebarHeight(
bool* override_titlebar_height,
float* titlebar_height) {
// Don't override titlebar height if the browser supports vertical tab strip
// so that it can overlay our client view. The visibility of title bar will be
// controlled by BrowserNonClientFrameViewMac::UpdateWindowTitleVisibility.
if (browser_->is_type_normal() && tabs::features::ShouldShowVerticalTabs())
simonhong marked this conversation as resolved.
Show resolved Hide resolved
return;

return BrowserFrameMac::GetWindowFrameTitlebarHeight(override_titlebar_height,
titlebar_height);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@ class BraveBrowserNonClientFrameViewMac : public BrowserNonClientFrameViewMac {
const BraveBrowserNonClientFrameViewMac&) = delete;

private:
bool ShouldShowWindowTitleForVerticalTabs() const;
void UpdateWindowTitleVisibility();

// BrowserNonClientFrameViewMac overrides:
void OnPaint(gfx::Canvas* canvas) override;
int GetTopInset(bool restored) const override;
int NonClientHitTest(const gfx::Point& point) override;

std::unique_ptr<BraveWindowFrameGraphic> frame_graphic_;

BooleanPrefMember show_title_bar_on_vertical_tabs_;
};

#endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_NON_CLIENT_FRAME_VIEW_MAC_H_
40 changes: 38 additions & 2 deletions browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@

#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/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/features.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "ui/base/hit_test.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/scoped_canvas.h"
Expand All @@ -20,6 +24,15 @@
: BrowserNonClientFrameViewMac(frame, browser_view) {
frame_graphic_ = std::make_unique<BraveWindowFrameGraphic>(
browser_view->browser()->profile());

if (tabs::features::ShouldShowVerticalTabs()) {
show_title_bar_on_vertical_tabs_.Init(
brave_tabs::kVerticalTabsShowTitleOnWindow,
browser_view->browser()->profile()->GetOriginalProfile()->GetPrefs(),
base::BindRepeating(
&BraveBrowserNonClientFrameViewMac::UpdateWindowTitleVisibility,
base::Unretained(this)));
}
}

BraveBrowserNonClientFrameViewMac::~BraveBrowserNonClientFrameViewMac() = default;
Expand All @@ -41,10 +54,33 @@
}

int BraveBrowserNonClientFrameViewMac::GetTopInset(bool restored) const {
if (tabs::features::ShouldShowVerticalTabs()) {
if (ShouldShowWindowTitleForVerticalTabs()) {
// Set minimum top inset to show caption buttons on frame.
return 18;
return 30;
}

return BrowserNonClientFrameViewMac::GetTopInset(restored);
}

bool BraveBrowserNonClientFrameViewMac::ShouldShowWindowTitleForVerticalTabs()
const {
return tabs::features::ShouldShowWindowTitleForVerticalTabs(
browser_view()->browser());
}

void BraveBrowserNonClientFrameViewMac::UpdateWindowTitleVisibility() {
if (!browser_view()->browser()->is_type_normal())
return;

frame()->SetWindowTitleVisibility(ShouldShowWindowTitleForVerticalTabs());
}

int BraveBrowserNonClientFrameViewMac::NonClientHitTest(
const gfx::Point& point) {
if (auto res = brave::NonClientHitTest(browser_view(), point);
res != HTNOWHERE) {
return res;
}

return BrowserNonClientFrameViewMac::NonClientHitTest(point);
}
2 changes: 1 addition & 1 deletion browser/ui/views/frame/brave_browser_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ bool BraveBrowserView::ShouldShowWindowTitle() const {
if (BrowserView::ShouldShowWindowTitle())
return true;

if (tabs::features::ShouldShowVerticalTabs() && browser_->is_type_normal())
if (tabs::features::ShouldShowWindowTitleForVerticalTabs(browser()))
return true;

return false;
Expand Down
21 changes: 21 additions & 0 deletions browser/ui/views/frame/brave_glass_browser_frame_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@

#include "brave/browser/ui/views/frame/brave_glass_browser_frame_view.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/features.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "ui/base/hit_test.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/scoped_canvas.h"
Expand Down Expand Up @@ -36,3 +39,21 @@ void BraveGlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
}
frame_graphic_->Paint(canvas, bounds_to_frame_graphic);
}

int BraveGlassBrowserFrameView::GetTopInset(bool restored) const {
if (tabs::features::ShouldShowVerticalTabs()) {
if (!tabs::features::ShouldShowWindowTitleForVerticalTabs(
browser_view()->browser()))
return 0;
}

return GlassBrowserFrameView::GetTopInset(restored);
}
int BraveGlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
if (auto res = brave::NonClientHitTest(browser_view(), point);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this check VerticalTabs feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd better to support this for others too. brave/brave-browser#25757

res != HTNOWHERE) {
return res;
}

return GlassBrowserFrameView::NonClientHitTest(point);
}
2 changes: 2 additions & 0 deletions browser/ui/views/frame/brave_glass_browser_frame_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class BraveGlassBrowserFrameView : public GlassBrowserFrameView {
private:
// GlassBrowserFrameView overrides:
void OnPaint(gfx::Canvas* canvas) override;
int GetTopInset(bool restored) const override;
int NonClientHitTest(const gfx::Point& point) override;

std::unique_ptr<BraveWindowFrameGraphic> frame_graphic_;
};
Expand Down
22 changes: 22 additions & 0 deletions browser/ui/views/frame/brave_non_client_hit_test_helper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */

#include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h"

#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "ui/base/hit_test.h"
#include "ui/views/window/hit_test_utils.h"

namespace brave {

int NonClientHitTest(BrowserView* browser_view, const gfx::Point& point) {
if (!browser_view->toolbar() || !browser_view->toolbar()->GetVisible())
return HTNOWHERE;

return views::GetHitTestComponent(browser_view->toolbar(), point);
}

} // namespace brave
24 changes: 24 additions & 0 deletions browser/ui/views/frame/brave_non_client_hit_test_helper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_
#define BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_

namespace gfx {
class Point;
} // namespace gfx

class BrowserView;

namespace brave {

// Helper function to set additional draggable area in client view.
// Returns HTNOWHERE if the point is not what we're interested in. In that
// case, caller should depend on the default behavior.
int NonClientHitTest(BrowserView* browser_view, const gfx::Point& point);

} // namespace brave

#endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */

#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/toolbar/reload_button.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/test/browser_test.h"
#include "ui/base/hit_test.h"

using BraveNonClientHitTestHelperBrowserTest = InProcessBrowserTest;

IN_PROC_BROWSER_TEST_F(BraveNonClientHitTestHelperBrowserTest, Toolbar) {
auto* browser_view = static_cast<BrowserView*>(browser()->window());
auto* toolbar = browser_view->toolbar();
auto* frame_view = browser_view->frame()->GetFrameView();

gfx::Point point;
views::View::ConvertPointToWidget(toolbar, &point);

// Dragging a window with the toolbar on it should work.
EXPECT_EQ(HTCAPTION, frame_view->NonClientHitTest(point));

// It shouldn't be perceived as a HTCAPTION when it's not visible.
toolbar->SetVisible(false);
EXPECT_NE(HTCAPTION, frame_view->NonClientHitTest(point));

// A coordinate on children of toolbar shouldn't be HTCAPTION so that users
// can interact with them. Checks a typical child of toolbar as a sanity
// check.
toolbar->SetVisible(true);
point = gfx::Point();
views::View::ConvertPointToWidget(toolbar->reload_button(), &point);
EXPECT_NE(HTCAPTION, frame_view->NonClientHitTest(point));
}
Loading