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

Update show bookmarks setting #15577

Merged
merged 1 commit into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/brave_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@
#define IDC_CONTENT_CONTEXT_IMPORT_IPNS_KEYS_START 56100
#define IDC_CONTENT_CONTEXT_IMPORT_IPNS_KEYS_END 56199

#define IDC_BRAVE_BOOKMARK_BAR_SUBMENU 56200
#define IDC_BRAVE_BOOKMARK_BAR_ALWAYS 56201
#define IDC_BRAVE_BOOKMARK_BAR_NTP 56202
#define IDC_BRAVE_BOOKMARK_BAR_NEVER 56203

#define IDC_BRAVE_COMMANDS_LAST 57000

#endif // BRAVE_APP_BRAVE_COMMAND_IDS_H_
12 changes: 12 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,22 @@
<message name="IDS_SHOW_BRAVE_REWARDS" desc="The show Brave Rewards menu in the app menu">
Brave Rewards
</message>
<message name="IDS_BOOKMAR_BAR_MENU_SHOW_ALWAYS" desc="The name of the context menu item to always show bookmark bar">
Always
</message>
<message name="IDS_BOOKMAR_BAR_MENU_SHOW_NEVER" desc="The name of the context menu item to never show bookmark bar">
Never
</message>
<if expr="use_titlecase">
<message name="IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER" desc="The menu item to report a broken site in the app menu">
Report a Broken Site
</message>
<message name="IDS_COPY_CLEAN_LINK" desc="The name of the context menu item to copy clean link">
Copy Clean Link
</message>
<message name="IDS_BOOKMAR_BAR_MENU_SHOW_NTP" desc="The name of the context menu item to show bookmark bar on ntp only">
Only On the New Tab Page
</message>
</if>
<if expr="not use_titlecase">
<message name="IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER" desc="The menu item to report a broken site in the app menu">
Expand All @@ -183,6 +192,9 @@
<message name="IDS_COPY_CLEAN_LINK" desc="The name of the context menu item to copy clean link">
Copy clean link
</message>
<message name="IDS_BOOKMAR_BAR_MENU_SHOW_NTP" desc="The name of the context menu item to show bookmark bar on ntp only">
Only on the new tab page
</message>
</if>
<message name="IDS_SHOW_BRAVE_SYNC" desc="The show brave sync menu in the app menu">
Sync
Expand Down
20 changes: 19 additions & 1 deletion app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,26 @@
<message name="IDS_SETTINGS_SHOW_BRAVE_NEWS_BUTTON_LABEL" desc="The label for the settings switch controlling whether the Brave News button is shown in the toolbar">
Show Brave News button in address bar
</message>
<message name="IDS_SETTINGS_SHOW_BOOKMARK_BAR" desc="The label for settings switch controlling the permanent visibility of bookmarks bar">
Show bookmarks
</message>
<message name="IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ALWAYS" desc="The label for settings switch controlling the permanent visibility of bookmarks bar">
Always
</message>
<message name="IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ALWAYS_DESC" desc="The label for settings switch controlling the permanent visibility of bookmarks bar">
Bookmark bar is visible on all pages
</message>
<message name="IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ON_NTP" desc="The label for settings switch controlling the visibility of bookmarks bar on NTP">
Always show bookmarks on new tab page
Only on the new tab page
</message>
<message name="IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ON_NTP_DESC" desc="The label for settings switch controlling the visibility of bookmarks bar on NTP description">
Bookmark bar is hidden on all pages except the new tab page
</message>
<message name="IDS_SETTINGS_NEVER_SHOW_BOOKMARK_BAR" desc="The label for settings switch controlling the visibility of bookmarks bar">
Never
</message>
<message name="IDS_SETTINGS_NEVER_SHOW_BOOKMARK_BAR_DESC" desc="The label for settings switch controlling the visibility of bookmarks bar description">
Bookmarks bar is hidden on all pages
</message>
<message name="IDS_SETTINGS_ALWAYS_SHOW_FULL_URLS" desc="The label for a setting to show full URLs on omnibox">
Always show full URLs
Expand Down
21 changes: 21 additions & 0 deletions browser/resources/settings/brave_appearance_page/bookmark_bar.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<style include="settings-shared iron-flex">
.settings-box.bottom-border {
border-bottom: var(--cr-separator-line);
border-top: none;
}
</style>
<div class="settings-box bottom-border">
<div class="flex" id="labelWrapper" aria-hidden="true">
<div class="label">$i18n{appearanceSettingsBookmarBar}</div>
<div class="secondary label" id="sub-label">
<span id="sub-label-text" aria-hidden="true">
[[bookmarkBarShowEnabledLabel_]]
</span>
</div>
</div>
<settings-dropdown-menu
pref="[[bookmarkBarStatePref_]]"
on-settings-control-change="onShowOptionChanged_"
menu-options="[[bookmarkBarShowOptions_]]">
</settings-dropdown-menu>
</div>
144 changes: 144 additions & 0 deletions browser/resources/settings/brave_appearance_page/bookmark_bar.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// 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 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 './bookmark_bar.html.js'

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

enum BookmarkBarState {
ALWAYS = 0,
NONE = 1,
NTP = 2,
}

const kAlwaysShowBookmarBarPrefName = 'brave.always_show_bookmark_bar_on_ntp'
const kShowOnAllTabsPrefName = 'bookmark_bar.show_on_all_tabs'

/**
* 'settings-brave-appearance-bookmark-bar' is the settings page area containing
* brave's bookmark bar visibility settings in appearance settings.
*/
export class SettingsBraveAppearanceBookmarkBarElement
extends SettingsBraveAppearanceBookmarkBarElementBase {
static get is() {
return 'settings-brave-appearance-bookmark-bar'
}

static get template() {
return getTemplate()
}

static get properties() {
return {
/** @private {chrome.settingsPrivate.PrefType} */
bookmarkBarStatePref_: {
key: '',
type: Object,
value() {
return {
key: '',
type: chrome.settingsPrivate.PrefType.NUMBER,
value: BookmarkBarState.NTP
}
}
}
}
}

bookmarkBarStatePref_: chrome.settingsPrivate.PrefObject

private bookmarkBarShowOptions_ = [
{
value: BookmarkBarState.ALWAYS,
name: this.i18n('appearanceSettingsBookmarBarAlways')
},
{
value: BookmarkBarState.NONE,
name: this.i18n('appearanceSettingsBookmarBarNever')
},
{
value: BookmarkBarState.NTP,
name: this.i18n('appearanceSettingsBookmarBarNTP')
}
]
private bookmarkBarShowEnabledLabel_: string

static get observers() {
return [
'onPrefsChanged_(prefs.bookmark_bar.show_on_all_tabs.value,' +
' prefs.brave.always_show_bookmark_bar_on_ntp.value)'
]
}

override ready() {
super.ready()
window.addEventListener('load', this.onLoad_.bind(this));
}

private onLoad_() {
this.setControlValueFromPrefs()
}

private getBookmarkBarStateFromPrefs(): BookmarkBarState {
if (this.getPref(kShowOnAllTabsPrefName).value)
return BookmarkBarState.ALWAYS

if (this.getPref(kAlwaysShowBookmarBarPrefName).value)
return BookmarkBarState.NTP
return BookmarkBarState.NONE
}

private saveBookmarkBarStateToPrefs(state: BookmarkBarState) {
if (state === BookmarkBarState.ALWAYS) {
this.setPrefValue(kShowOnAllTabsPrefName, true)
} else if (state === BookmarkBarState.NTP) {
this.setPrefValue(kShowOnAllTabsPrefName, false)
this.setPrefValue(kAlwaysShowBookmarBarPrefName, true)
} else {
this.setPrefValue(kShowOnAllTabsPrefName, false)
this.setPrefValue(kAlwaysShowBookmarBarPrefName, false)
}
}
private setControlValueFromPrefs() {
const state = this.getBookmarkBarStateFromPrefs()
if (this.bookmarkBarStatePref_.value === state)
return
this.bookmarkBarStatePref_ = {
key: '',
type: chrome.settingsPrivate.PrefType.NUMBER,
value: state
};
}
private onPrefsChanged_() {
this.setControlValueFromPrefs()
}
private onShowOptionChanged_() {
const state = this.bookmarkBarStatePref_.value
if (state === BookmarkBarState.ALWAYS) {
this.bookmarkBarShowEnabledLabel_ =
this.i18n('appearanceSettingsBookmarBarAlwaysDesc')
} else if (state === BookmarkBarState.NTP) {
this.bookmarkBarShowEnabledLabel_ =
this.i18n('appearanceSettingsBookmarBarNTPDesc')
} else {
this.bookmarkBarShowEnabledLabel_ =
this.i18n('appearanceSettingsBookmarBarNeverDesc')
}

this.saveBookmarkBarStateToPrefs(this.bookmarkBarStatePref_.value)
}

}

customElements.define(SettingsBraveAppearanceBookmarkBarElement.is,
SettingsBraveAppearanceBookmarkBarElement)
4 changes: 2 additions & 2 deletions browser/resources/settings/brave_appearance_page/toolbar.html
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@
</template>
</if>
<settings-toggle-button
pref="{{prefs.brave.always_show_bookmark_bar_on_ntp}}"
pref="{{prefs.omnibox.prevent_url_elisions}}"
class="cr-row"
label="$i18n{appearanceSettingsAlwaysShowBookmarkBarOnNTP}">
label="$i18n{showFullUrls}">
</settings-toggle-button>
<settings-toggle-button
pref="{{prefs.brave.tabs_search_show}}"
Expand Down
12 changes: 11 additions & 1 deletion browser/resources/settings/brave_overrides/appearance_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {loadTimeData} from '../i18n_setup.js'
import '../brave_appearance_page/super_referral.js'
import '../brave_appearance_page/brave_theme.js'
import '../brave_appearance_page/toolbar.js'
import '../brave_appearance_page/bookmark_bar.js'

const superReferralStringId = 'superReferralThemeName'

Expand Down Expand Up @@ -59,9 +60,18 @@ RegisterPolymerTemplateModifications({
if (!bookmarkBarToggle) {
console.error(`[Brave Settings Overrides] Couldn't find bookmark bar toggle`)
} else {
bookmarkBarToggle.insertAdjacentHTML('beforebegin', `
<settings-brave-appearance-bookmark-bar prefs="{{prefs}}">
</settings-brave-appearance-bookmark-bar>
`)

bookmarkBarToggle.insertAdjacentHTML('afterend', `
<settings-brave-appearance-toolbar prefs="{{prefs}}"></settings-brave-appearance-toolbar>
<settings-brave-appearance-toolbar prefs="{{prefs}}">
</settings-brave-appearance-toolbar>
`)
// Remove Chromium bookmark toggle becasue it is replaced by
// settings-brave-appearance-bookmark-bar
bookmarkBarToggle.remove()
}
const zoomLevel = templateContent.getElementById('zoomLevel')
if (!zoomLevel || !zoomLevel.parentNode) {
Expand Down
1 change: 1 addition & 0 deletions browser/resources/settings/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ brave_settings_preprocess_deps =
brave_settings_web_component_files = [
"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/toolbar.ts",
"brave_clear_browsing_data_dialog/brave_clear_browsing_data_on_exit_page.ts",
Expand Down
8 changes: 8 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,14 @@ source_set("ui") {
"omnibox/brave_omnibox_client_impl.cc",
"omnibox/brave_omnibox_client_impl.h",
"session_crashed_bubble_brave.cc",
"toolbar/bookmark_bar_sub_menu_model.cc",
"toolbar/bookmark_bar_sub_menu_model.h",
"toolbar/brave_app_menu_model.cc",
"toolbar/brave_app_menu_model.h",
"toolbar/brave_bookmark_context_menu_controller.cc",
"toolbar/brave_bookmark_context_menu_controller.h",
"toolbar/brave_bookmark_sub_menu_model.cc",
"toolbar/brave_bookmark_sub_menu_model.h",
"toolbar/brave_recent_tabs_sub_menu_model.h",
"webui/brave_rewards/rewards_panel_handler.cc",
"webui/brave_rewards/rewards_panel_handler.h",
Expand Down Expand Up @@ -211,6 +217,8 @@ source_set("ui") {
sources += [
"views/bookmarks/bookmark_bar_instructions_view.cc",
"views/bookmarks/bookmark_bar_instructions_view.h",
"views/bookmarks/brave_bookmark_context_menu.cc",
"views/bookmarks/brave_bookmark_context_menu.h",
"views/brave_first_run_dialog.cc",
"views/brave_first_run_dialog.h",
"views/brave_layout_provider.cc",
Expand Down
21 changes: 21 additions & 0 deletions browser/ui/bookmark/BUILD.gn
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
# Copyright (c) 2021 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/.

source_set("bookmark") {
# Remove when https://github.com/brave/brave-browser/issues/10654 is resolved.
check_includes = false
assert(!is_android)
sources = [
"bookmark_helper.cc",
"bookmark_helper.h",
"bookmark_prefs_service.cc",
"bookmark_prefs_service.h",
"bookmark_prefs_service_factory.cc",
Expand Down Expand Up @@ -32,3 +40,16 @@ source_set("bookmark") {
"//ui/webui/resources/js/browser_command:mojo_bindings",
]
}

source_set("unittest") {
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm a bit outdated, so don't really know what is the current convention - put tests into smaller BUILD.gn`s or continue stockpiling them in brave/test/BUILD.gn ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

something wrong here?

testonly = true
assert(!is_android)
sources = [ "bookmark_helper_unittest.cc" ]

deps = [
"//brave/browser/ui/bookmark",
"//components/bookmarks/browser",
"//components/sync_preferences:test_support",
"//testing/gtest",
]
}
39 changes: 39 additions & 0 deletions browser/ui/bookmark/bookmark_helper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* 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/. */

#include "brave/browser/ui/bookmark/bookmark_helper.h"

#include "brave/components/constants/pref_names.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/prefs/pref_service.h"

namespace brave {

BookmarkBarState GetBookmarkBarState(PrefService* prefs) {
// kShowBookmarkBar has higher priority and the bookmark bar is shown always.
if (prefs->GetBoolean(bookmarks::prefs::kShowBookmarkBar))
return BookmarkBarState::kAlways;
// kShowBookmarkBar is false, kAlwaysShowBookmarkBarOnNTP is true
// -> the bookmark bar is shown only for NTP.
if (prefs->GetBoolean(kAlwaysShowBookmarkBarOnNTP))
return BookmarkBarState::kNtp;
// NEVER show the bookmark bar.
return BookmarkBarState::kNever;
}

void SetBookmarkState(BookmarkBarState state, PrefService* prefs) {
if (state == BookmarkBarState::kAlways) {
prefs->SetBoolean(bookmarks::prefs::kShowBookmarkBar, true);
prefs->SetBoolean(kAlwaysShowBookmarkBarOnNTP, false);
} else if (state == BookmarkBarState::kNtp) {
prefs->SetBoolean(bookmarks::prefs::kShowBookmarkBar, false);
prefs->SetBoolean(kAlwaysShowBookmarkBarOnNTP, true);
} else {
prefs->SetBoolean(bookmarks::prefs::kShowBookmarkBar, false);
prefs->SetBoolean(kAlwaysShowBookmarkBarOnNTP, false);
}
}

} // namespace brave
Loading