Skip to content

Commit

Permalink
Grouping sidebar related options together in settings
Browse files Browse the repository at this point in the history
fix brave/brave-browser#25717

Several sidebar options are grouped and located under autocomplete suggestion option.
Always show ful urls option is relocated under Use wide address bar
  • Loading branch information
simonhong committed Nov 30, 2022
1 parent ad89e32 commit 1ac1209
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 32 deletions.
3 changes: 3 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,9 @@ Or change later at <ph name="SETTINGS_EXTENIONS_LINK">$2<ex>brave://settings/ext
<message name="IDS_SETTINGS_SIDEBAR_SHOW_OPTION_TITLE" desc="Title of sidebar show option menu in settings">
Show Sidebar
</message>
<message name="IDS_SETTINGS_SIDEBAR_PART_TITLE" desc="Title of sidebar show option menu in settings">
Sidebar
</message>
<message name="IDS_SIDEBAR_SHOW_OPTION_ALWAYS" desc="Label for always show sidebar option">
Always
</message>
Expand Down
12 changes: 9 additions & 3 deletions app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_BOOKMARKS_BUTTON" desc="The label for whether the bookmarks button should be shown or not">
Show bookmarks button
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_SIDE_PANEL_BUTTON" desc="The label for whether the side panel button should be shown or not">
Show side panel button
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_LOCATION_BAR_IS_WIDE" desc="The label for whether the location bar should display wide or not">
Use wide address bar
</message>
Expand Down Expand Up @@ -117,6 +114,15 @@
Articles automatically load in reader mode, saving you time
</message>

<if expr="enable_sidebar">
<message name="IDS_SETTINGS_APPEARNCE_SETTINGS_SIDEBAR_PART_TITLE" desc="Title of sidebar part in settings">
Sidebar
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_SIDEBAR_BUTTON" desc="The label for whether the side panel button should be shown or not">
Show Sidebar button
</message>
</if>

<!-- Settings / New tab page -->
<message name="IDS_SETTINGS_NEW_TAB" desc="The text label for the New Tab settings page">
New Tab Page
Expand Down
2 changes: 2 additions & 0 deletions browser/resources/settings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import("//brave/browser/shell_integrations/buildflags/buildflags.gni")
import("//brave/build/config.gni")
import("//brave/components/brave_vpn/buildflags/buildflags.gni")
import("//brave/components/brave_wayback_machine/buildflags/buildflags.gni")
import("//brave/components/sidebar/buildflags/buildflags.gni")
import("//brave/components/tor/buildflags/buildflags.gni")
import("//brave/resources/brave_grit.gni")
import("//chrome/common/features.gni")
Expand Down Expand Up @@ -72,6 +73,7 @@ preprocess_if_expr("preprocess") {
"enable_brave_vpn=$enable_brave_vpn",
"enable_extensions=$enable_extensions",
"enable_pin_shortcut=$enable_pin_shortcut",
"enable_sidebar=$enable_sidebar",
]
in_folder = "./"
out_folder =
Expand Down
55 changes: 41 additions & 14 deletions browser/resources/settings/brave_appearance_page/sidebar.html
Original file line number Diff line number Diff line change
@@ -1,21 +1,48 @@
<style include="settings-shared iron-flex">
.settings-box.bottom-border {
border-top: none;
.border {
border-top: var(--cr-separator-line);
border-bottom: var(--cr-separator-line);
}
</style>
<div class="settings-box bottom-border">
<div class="flex" id="labelWrapper" aria-hidden="true">
<div class="label">$i18n{appearanceSettingsShowOptionTitle}</div>
<div class="secondary label" id="sub-label">
<span id="sub-label-text" aria-hidden="true">
[[sidebarShowEnabledLabel_]]
</span>

<div class="cr-row">$i18n{sideBar}</div>
<div class="list-frame">
<div class="cr-row list-item">
<div class="flex" id="labelWrapper" aria-hidden="true">
<div class="label">$i18n{appearanceSettingsShowOptionTitle}</div>
<div class="secondary label" id="sub-label">
<span id="sub-label-text" aria-hidden="true">
[[sidebarShowEnabledLabel_]]
</span>
</div>
</div>
<settings-dropdown-menu
pref="{{prefs.brave.sidebar.sidebar_show_option}}"
on-settings-control-change="onShowOptionChanged_"
menu-options="[[sidebarShowOptions_]]">
</settings-dropdown-menu>
</div>
<settings-dropdown-menu
pref="{{prefs.brave.sidebar.sidebar_show_option}}"
on-settings-control-change="onShowOptionChanged_"
menu-options="[[sidebarShowOptions_]]">
</settings-dropdown-menu>
<settings-toggle-button
class="cr-row border list-item"
pref="{{prefs.brave.show_side_panel_button}}"
label="$i18n{appearanceSettingsShowSidebarButton}">
</settings-toggle-button>
<settings-radio-group
pref="{{prefs.side_panel.is_right_aligned}}"
group-aria-label="$i18n{sideBar}">
<controlled-radio-button
class="list-item"
pref="[[prefs.side_panel.is_right_aligned]]"
label="$i18n{sidePanelAlignLeft}"
name="false"
no-extension-indicator>
</controlled-radio-button>
<controlled-radio-button
class="list-item"
pref="[[prefs.side_panel.is_right_aligned]]"
label="$i18n{sidePanelAlignRight}"
name="true"
no-extension-indicator>
</controlled-radio-button>
</settings-radio-group>
</div>
18 changes: 8 additions & 10 deletions browser/resources/settings/brave_appearance_page/toolbar.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@
pref="{{prefs.brave.show_bookmarks_button}}"
label="$i18n{appearanceSettingsShowBookmarksButton}">
</settings-toggle-button>
<settings-toggle-button
class="cr-row"
pref="{{prefs.brave.show_side_panel_button}}"
label="$i18n{appearanceSettingsShowSidePanelButton}">
</settings-toggle-button>
<settings-toggle-button
class="cr-row"
pref="{{prefs.brave.today.should_show_toolbar_button}}"
Expand All @@ -20,6 +15,11 @@
pref="{{prefs.brave.location_bar_is_wide}}"
label="$i18n{appearanceSettingsLocationBarIsWide}">
</settings-toggle-button>
<settings-toggle-button
pref="{{prefs.omnibox.prevent_url_elisions}}"
class="cr-row"
label="$i18n{showFullUrls}">
</settings-toggle-button>
<settings-toggle-button
class="cr-row"
pref="{{prefs.brave.autocomplete_enabled}}"
Expand All @@ -46,6 +46,9 @@
</settings-checkbox>
</div>
</template>
<if expr="enable_sidebar">
<settings-brave-appearance-sidebar prefs="{{prefs}}"></settings-brave-appearance-sidebar>
</if>
<if expr="enable_brave_vpn">
<template is="dom-if" if="[[showBraveVPNOption_()]]">
<settings-toggle-button
Expand All @@ -61,11 +64,6 @@
class="cr-row"
label="$i18n{appearanceSettingsAlwaysShowBookmarkBarOnNTP}">
</settings-toggle-button>
<settings-toggle-button
pref="{{prefs.omnibox.prevent_url_elisions}}"
class="cr-row"
label="$i18n{showFullUrls}">
</settings-toggle-button>
<settings-toggle-button
pref="{{prefs.brave.tabs_search_show}}"
class="cr-row"
Expand Down
3 changes: 0 additions & 3 deletions browser/resources/settings/brave_overrides/appearance_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ RegisterPolymerTemplateModifications({
if (!bookmarkBarToggle) {
console.error(`[Brave Settings Overrides] Couldn't find bookmark bar toggle`)
} else {
bookmarkBarToggle.insertAdjacentHTML('beforebegin', `
<settings-brave-appearance-sidebar prefs="{{prefs}}"></settings-brave-appearance-sidebar>
`)
bookmarkBarToggle.insertAdjacentHTML('afterend', `
<settings-brave-appearance-toolbar prefs="{{prefs}}"></settings-brave-appearance-toolbar>
`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_THEMES},
{"appearanceSettingsShowBookmarksButton",
IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_BOOKMARKS_BUTTON},
{"appearanceSettingsShowSidePanelButton",
IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_SIDE_PANEL_BUTTON},
{"appearanceSettingsLocationBarIsWide",
IDS_SETTINGS_APPEARANCE_SETTINGS_LOCATION_BAR_IS_WIDE},
{"appearanceSettingsShowBraveRewardsButtonLabel",
Expand Down Expand Up @@ -127,8 +125,11 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
{"appearanceSettingsTabHoverModeTooltip",
IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_TAB_HOVER_MODE_TOOLTIP},
#if BUILDFLAG(ENABLE_SIDEBAR)
{"sideBar", IDS_SETTINGS_APPEARNCE_SETTINGS_SIDEBAR_PART_TITLE},
{"appearanceSettingsShowOptionTitle",
IDS_SETTINGS_SIDEBAR_SHOW_OPTION_TITLE},
{"appearanceSettingsShowSidebarButton",
IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_SIDEBAR_BUTTON},
{"appearanceSettingsShowOptionAlways", IDS_SIDEBAR_SHOW_OPTION_ALWAYS},
{"appearanceSettingsShowOptionMouseOver",
IDS_SIDEBAR_SHOW_OPTION_MOUSEOVER},
Expand Down Expand Up @@ -699,6 +700,10 @@ void BraveAddLocalizedStrings(content::WebUIDataSource* html_source,
};
html_source->AddLocalizedStrings(kSessionOnlyToEphemeralStrings);
}

// Always disable upstream's side panel align option.
// We add our customized option at preferred position.
html_source->AddBoolean("showSidePanelOptions", false);
}

} // namespace settings

0 comments on commit 1ac1209

Please sign in to comment.