Skip to content

Commit

Permalink
Move Vertical tab settings to brave://settings (uplift to 1.51.x) (#1…
Browse files Browse the repository at this point in the history
…8183)

* Uplift of #18146 (squashed) to beta

* Resurrect "Use vertical tabs" Context menu

Base on our latest design guide, we need "Use vertical tabs" menu.

* Build fix trial

---------

Co-authored-by: sangwoo.ko <sangwoo108@gmail.com>
  • Loading branch information
brave-builds and sangwoo108 authored Apr 27, 2023
1 parent 79cd4f6 commit a2f27a1
Show file tree
Hide file tree
Showing 13 changed files with 168 additions and 77 deletions.
12 changes: 0 additions & 12 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -886,12 +886,6 @@ Or change later at <ph name="SETTINGS_EXTENIONS_LINK">$2<ex>brave://settings/ext
<message name="IDS_TAB_CXMENU_SHOW_VERTICAL_TABS" desc="The label of the tab context menu item for showing tabs vertically">
Use vertical 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>
<message name="IDS_TAB_CXMENU_USE_FLOATING_VERTICAL_TABS" desc="The label of the tab context menu item for enabling floating mode for vertical tab strip">
Float on mouseover
</message>
<message name="IDS_TAB_CXMENU_SOUND_MUTE_TAB" desc="The label of the tab context menu item for muting one or more tabs. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]">
{NUM_TABS, plural, =1 {Mute tab} other {Mute tabs}}
</message>
Expand All @@ -912,12 +906,6 @@ Or change later at <ph name="SETTINGS_EXTENIONS_LINK">$2<ex>brave://settings/ext
<message name="IDS_TAB_CXMENU_SHOW_VERTICAL_TABS" desc="The label of the tab context menu item for showing tabs vertically">
Use Vertical 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>
<message name="IDS_TAB_CXMENU_USE_FLOATING_VERTICAL_TABS" desc="The label of the tab context menu item for enabling floating mode for vertical tab strip">
Float on Mouseover
</message>
<message name="IDS_TAB_CXMENU_SOUND_MUTE_TAB" desc="The label of the tab context menu item for muting one or more tabs. NOTE: If having no explicit # makes the phrasing awkward, feel free to add a # as necessary. [ICU Syntax]">
{NUM_TABS, plural, =1 {Mute Tab} other {Mute Tabs}}
</message>
Expand Down
12 changes: 12 additions & 0 deletions app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_DEFAULT_IMAGES_OPTION_LABEL" desc="The label for choosing default images over super referral">
Brave default images
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_TABS_SECTION" desc="The label for section containing settings related to tabs">
Tabs
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_TABS_SHOW_VERTICAL_TABS" desc="The label for showing tabs vertically">
Use vertical tabs
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_TABS_SHOW_TITLE_BAR" desc="The label for showing window title when vertical tab strip is enabled.">
Show title bar
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_TABS_USE_FLOATING_VERTICAL_TABS" desc="The label for enabling floating mode for vertical tab strip">
Float on mouseover
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_TAB_HOVER_MODE" desc="The label for the settings select controlling what happens when a user hovers over a tab.">
Tab hover mode
</message>
Expand Down
14 changes: 14 additions & 0 deletions browser/extensions/api/settings_private/brave_prefs_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@

#include "brave/browser/extensions/api/settings_private/brave_prefs_util.h"

#include "base/feature_list.h"
#include "brave/browser/ethereum_remote_client/buildflags/buildflags.h"
#include "brave/browser/ui/tabs/brave_tab_prefs.h"
#include "brave/browser/ui/tabs/features.h"
#include "brave/components/brave_ads/common/pref_names.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "brave/components/brave_rewards/common/pref_names.h"
Expand Down Expand Up @@ -315,6 +317,18 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() {
// NFT pinning pref
(*s_brave_allowlist)[kAutoPinEnabled] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;

#if defined(TOOLKIT_VIEWS)
// Vertical tab strip prefs
if (base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs)) {
(*s_brave_allowlist)[brave_tabs::kVerticalTabsEnabled] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_allowlist)[brave_tabs::kVerticalTabsFloatingEnabled] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_allowlist)[brave_tabs::kVerticalTabsShowTitleOnWindow] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
}
#endif
return *s_brave_allowlist;
}

Expand Down
67 changes: 67 additions & 0 deletions browser/resources/settings/brave_appearance_page/tabs.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<!-- This file will be converted into tabs.html.js using //tools/grit/preprocess_if_expr -->
<style include="settings-shared iron-flex">
.border {
border-top: var(--cr-separator-line);
border-bottom: var(--cr-separator-line);
}

.settings-box {
padding: 0
}
</style>

<div class="cr-row">$i18n{appearanceSettingsTabsSection}</div>
<div class="list-frame">
<!-- Vertical Tab strip -->
<template is="dom-if" if="[[isVerticalTabStripFeatureEnabled()]]">
<settings-toggle-button
class="cr-row list-item"
pref="{{prefs.brave.tabs.vertical_tabs_enabled}}"
label="$i18n{appearanceSettingsTabsUseVerticalTabs}">
</settings-toggle-button>
<template
is="dom-if"
if="[[prefs.brave.tabs.vertical_tabs_enabled.value]]">
<div class="cr-row">
<settings-checkbox
class="cr-row list-item"
pref="{{prefs.brave.tabs.vertical_tabs_show_title_on_window}}"
label="$i18n{appearanceSettingsTabsShowWindowTitle}">
</settings-checkbox>
</div>
<div class="cr-row">
<settings-checkbox
class="cr-row list-item"
pref="{{prefs.brave.tabs.vertical_tabs_floating_enabled}}"
label="$i18n{appearanceSettingsTabsFloatOnMouseOver}">
</settings-checkbox>
</div>
</template> <!-- vertical_tabs_enabled.value -->
</template> <!-- isVerticalTabStripFeatureEnabled()-->

<!-- Tab search button visibility -->
<settings-toggle-button
pref="{{prefs.brave.tabs_search_show}}"
class="cr-row list-item"
label="$i18n{showSearchTabsBtn}">
</settings-toggle-button>

<!-- Tab speaker icons function -->
<settings-toggle-button
pref="{{prefs.brave.tabs.mute_indicator_not_clickable}}"
class="cr-row list-item"
label="$i18n{braveDisableClickableMuteIndicators}">
</settings-toggle-button>

<!-- Tab hover mode -->
<div class="cr-row list-item settings-box">
<div class="flex">
<div class="label">$i18n{appearanceSettingsTabHoverMode}</div>
</div>
<settings-dropdown-menu
pref="{{prefs.brave.tabs.hover_mode}}"
menu-options="[[tabTooltipModes_]]">
</settings-dropdown-menu>
</div>
</div> <!-- .list-frame -->

42 changes: 42 additions & 0 deletions browser/resources/settings/brave_appearance_page/tabs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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/.

import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'
import {I18nMixin, I18nMixinInterface} from 'chrome://resources/cr_elements/i18n_mixin.js'
import {PrefsMixin, PrefsMixinInterface} from '../prefs/prefs_mixin.js'
import '../settings_shared.css.js'
import '../settings_vars.css.js'
import {getTemplate} from './tabs.html.js'
import { loadTimeData } from '../i18n_setup.js'

const SettingsBraveAppearanceTabsElementBase = PrefsMixin(I18nMixin(PolymerElement)) as {
new (): PolymerElement & I18nMixinInterface & PrefsMixinInterface
}

export class SettingsBraveAppearanceTabsElement extends SettingsBraveAppearanceTabsElementBase {
static get is() {
return 'settings-brave-appearance-tabs'
}

static get template() {
return getTemplate()
}

private isVerticalTabStripFeatureEnabled() {
return loadTimeData.getBoolean('verticalTabStripFeatureEnabled')
}

private tabTooltipModes_ = [
{ value: 1, name: this.i18n('appearanceSettingsTabHoverModeCard') },
{
value: 2,
name: this.i18n('appearanceSettingsTabHoverModeCardWithPreview')
},
{ value: 0, name: this.i18n('appearanceSettingsTabHoverModeTooltip') }
]

}

customElements.define(SettingsBraveAppearanceTabsElement.is, SettingsBraveAppearanceTabsElement)
20 changes: 1 addition & 19 deletions browser/resources/settings/brave_appearance_page/toolbar.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
</div>
</template>
<if expr="toolkit_views">
<settings-brave-appearance-tabs prefs="{{prefs}}""></settings-brave-appearance-tabs>
<settings-brave-appearance-sidebar prefs="{{prefs}}"></settings-brave-appearance-sidebar>
</if>
<if expr="enable_brave_vpn">
Expand All @@ -59,22 +60,3 @@
</settings-toggle-button>
</template>
</if>
<settings-toggle-button
pref="{{prefs.brave.tabs_search_show}}"
class="cr-row"
label="$i18n{showSearchTabsBtn}">
</settings-toggle-button>
<settings-toggle-button
pref="{{prefs.brave.tabs.mute_indicator_not_clickable}}"
class="cr-row"
label="$i18n{braveDisableClickableMuteIndicators}">
</settings-toggle-button>
<div class="settings-box">
<div class="flex">
<div class="label">$i18n{appearanceSettingsTabHoverMode}</div>
</div>
<settings-dropdown-menu
pref="{{prefs.brave.tabs.hover_mode}}"
menu-options="[[tabTooltipModes_]]">
</settings-dropdown-menu>
</div>
10 changes: 1 addition & 9 deletions browser/resources/settings/brave_appearance_page/toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {getTemplate} from './toolbar.html.js'
import '../settings_shared.css.js'
import '../settings_vars.css.js'
import './sidebar.js'
import './tabs.js'

const SettingsBraveAppearanceToolbarElementBase = I18nMixin(PolymerElement) as {
new (): PolymerElement & I18nMixinInterface
Expand Down Expand Up @@ -48,15 +49,6 @@ class SettingsBraveAppearanceToolbarElement extends SettingsBraveAppearanceToolb
// }
// }

private tabTooltipModes_ = [
{ value: 1, name: this.i18n('appearanceSettingsTabHoverModeCard') },
{
value: 2,
name: this.i18n('appearanceSettingsTabHoverModeCardWithPreview')
},
{ value: 0, name: this.i18n('appearanceSettingsTabHoverModeTooltip') }
]

private showBraveVPNOption_() {
return loadTimeData.getBoolean('isBraveVPNEnabled')
}
Expand Down
5 changes: 4 additions & 1 deletion browser/resources/settings/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import("//brave/browser/shell_integrations/buildflags/buildflags.gni")
# webui build pipeline.
brave_settings_preprocess_deps =
[ "//brave/browser/resources/settings:preprocess" ]

brave_settings_web_component_files = [
"brave_appearance_page/bookmark_bar.ts",
"brave_appearance_page/brave_theme.ts",
"brave_appearance_page/sidebar.ts",
"brave_appearance_page/bookmark_bar.ts",
"brave_appearance_page/super_referral.ts",
"brave_appearance_page/tabs.ts",
"brave_appearance_page/toolbar.ts",
"brave_clear_browsing_data_dialog/brave_clear_browsing_data_on_exit_page.ts",
"brave_default_extensions_page/brave_default_extensions_page.ts",
Expand Down Expand Up @@ -54,6 +56,7 @@ brave_settings_web_component_files = [
"getting_started_page/getting_started.ts",
"social_blocking_page/social_blocking_page.ts",
]

brave_settings_non_web_component_files = [
"brave_appearance_page/brave_appearance_browser_proxy.ts",
"brave_clear_browsing_data_dialog/brave_clear_browsing_data_dialog_behavior.ts",
Expand Down
7 changes: 3 additions & 4 deletions browser/ui/tabs/brave_tab_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,11 @@ void BraveTabMenuModel::Build(int selected_tab_count) {
AddItemWithStringId(CommandRestoreTab, GetRestoreTabCommandStringId());
AddItemWithStringId(CommandBookmarkAllTabs, IDS_TAB_CXMENU_BOOKMARK_ALL_TABS);

if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs))
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs)) {
return;
}

AddSeparator(ui::NORMAL_SEPARATOR);
AddCheckItemWithStringId(CommandShowVerticalTabs,
IDS_TAB_CXMENU_SHOW_VERTICAL_TABS);
AddCheckItemWithStringId(CommandShowTitleBar, IDS_TAB_CXMENU_SHOW_TITLE_BAR);
AddCheckItemWithStringId(CommandUseFloatingVerticalTabStrip,
IDS_TAB_CXMENU_USE_FLOATING_VERTICAL_TABS);
}
2 changes: 0 additions & 2 deletions browser/ui/tabs/brave_tab_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ class BraveTabMenuModel : public TabMenuModel {
CommandRestoreTab,
CommandBookmarkAllTabs,
CommandShowVerticalTabs,
CommandShowTitleBar,
CommandUseFloatingVerticalTabStrip,
CommandToggleTabMuted,
CommandLast,
};
Expand Down
36 changes: 8 additions & 28 deletions browser/ui/views/tabs/brave_tab_context_menu_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,12 @@ void BraveTabContextMenuContents::RunMenuAt(const gfx::Point& point,
}

bool BraveTabContextMenuContents::IsCommandIdChecked(int command_id) const {
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs))
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs)) {
return ui::SimpleMenuModel::Delegate::IsCommandIdChecked(command_id);

if (command_id == BraveTabMenuModel::CommandShowVerticalTabs)
return tabs::utils::ShouldShowVerticalTabs(browser_);

if (command_id == BraveTabMenuModel::CommandShowTitleBar) {
return tabs::utils::ShouldShowWindowTitleForVerticalTabs(browser_);
}

if (command_id == BraveTabMenuModel::CommandUseFloatingVerticalTabStrip) {
return tabs::utils::IsFloatingVerticalTabsEnabled(browser_);
if (command_id == BraveTabMenuModel::CommandShowVerticalTabs) {
return tabs::utils::ShouldShowVerticalTabs(browser_);
}

return ui::SimpleMenuModel::Delegate::IsCommandIdChecked(command_id);
Expand All @@ -92,15 +86,12 @@ bool BraveTabContextMenuContents::IsCommandIdEnabled(int command_id) const {
}

bool BraveTabContextMenuContents::IsCommandIdVisible(int command_id) const {
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs))
if (!base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs)) {
return ui::SimpleMenuModel::Delegate::IsCommandIdVisible(command_id);
}

if (command_id == BraveTabMenuModel::CommandShowVerticalTabs)
if (command_id == BraveTabMenuModel::CommandShowVerticalTabs) {
return tabs::utils::SupportsVerticalTabs(browser_);

if (command_id == BraveTabMenuModel::CommandShowTitleBar ||
command_id == BraveTabMenuModel::CommandUseFloatingVerticalTabStrip) {
return tabs::utils::ShouldShowVerticalTabs(browser_);
}

return ui::SimpleMenuModel::Delegate::IsCommandIdVisible(command_id);
Expand Down Expand Up @@ -143,10 +134,6 @@ bool BraveTabContextMenuContents::IsBraveCommandIdEnabled(
chrome::CanBookmarkAllTabs(browser_);
}
break;
case BraveTabMenuModel::CommandShowTitleBar:
case BraveTabMenuModel::CommandShowVerticalTabs:
case BraveTabMenuModel::CommandUseFloatingVerticalTabStrip:
return true;
case BraveTabMenuModel::CommandToggleTabMuted: {
auto* model = static_cast<BraveTabStripModel*>(controller_->model());
for (const auto& index : model->GetTabIndicesForCommandAt(tab_index_)) {
Expand All @@ -155,6 +142,8 @@ bool BraveTabContextMenuContents::IsBraveCommandIdEnabled(
}
return false;
}
case BraveTabMenuModel::CommandShowVerticalTabs:
return true;
default:
NOTREACHED();
break;
Expand All @@ -176,15 +165,6 @@ void BraveTabContextMenuContents::ExecuteBraveCommand(int command_id) {
BrowserView::GetBrowserViewForBrowser(browser_)->InvalidateLayout();
return;
}
case BraveTabMenuModel::CommandShowTitleBar: {
brave::ToggleWindowTitleVisibilityForVerticalTabs(browser_);
BrowserView::GetBrowserViewForBrowser(browser_)->InvalidateLayout();
return;
}
case BraveTabMenuModel::CommandUseFloatingVerticalTabStrip: {
brave::ToggleVerticalTabStripFloatingMode(browser_);
return;
}
case BraveTabMenuModel::CommandToggleTabMuted: {
auto* model = static_cast<BraveTabStripModel*>(controller_->model());
auto indices = model->GetTabIndicesForCommandAt(tab_index_);
Expand Down
6 changes: 6 additions & 0 deletions browser/ui/webui/brave_settings_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "brave/browser/resources/settings/grit/brave_settings_resources.h"
#include "brave/browser/resources/settings/grit/brave_settings_resources_map.h"
#include "brave/browser/shell_integrations/buildflags/buildflags.h"
#include "brave/browser/ui/tabs/features.h"
#include "brave/browser/ui/webui/navigation_bar_data_provider.h"
#include "brave/browser/ui/webui/settings/brave_adblock_handler.h"
#include "brave/browser/ui/webui/settings/brave_appearance_handler.h"
Expand Down Expand Up @@ -141,6 +142,11 @@ void BraveSettingsUI::AddResources(content::WebUIDataSource* html_source,

html_source->AddBoolean("extensionsManifestV2Feature",
base::FeatureList::IsEnabled(kExtensionsManifestV2));
#if defined(TOOLKIT_VIEWS)
html_source->AddBoolean(
"verticalTabStripFeatureEnabled",
base::FeatureList::IsEnabled(tabs::features::kBraveVerticalTabs));
#endif
}

// static
Expand Down
Loading

0 comments on commit a2f27a1

Please sign in to comment.