Merge pull request #13304 from brave/show-bookmarks-button-pref
Add setting to show or hide the Bookmarks button in the Toolbar
This commit is contained in:
@@ -62,6 +62,9 @@
|
||||
<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_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_LOCATION_BAR_IS_WIDE" desc="The label for whether the location bar should display wide or not">
|
||||
Use wide address bar
|
||||
</message>
|
||||
|
||||
@@ -178,6 +178,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
|
||||
registry);
|
||||
|
||||
// appearance
|
||||
registry->RegisterBooleanPref(kShowBookmarksButton, true);
|
||||
registry->RegisterBooleanPref(kLocationBarIsWide, false);
|
||||
registry->RegisterBooleanPref(brave_rewards::prefs::kShowButton, true);
|
||||
registry->RegisterBooleanPref(kMRUCyclingEnabled, false);
|
||||
|
||||
@@ -152,6 +152,8 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() {
|
||||
settings_api::PrefType::PREF_TYPE_BOOLEAN;
|
||||
|
||||
// appearance prefs
|
||||
(*s_brave_allowlist)[kShowBookmarksButton] =
|
||||
settings_api::PrefType::PREF_TYPE_BOOLEAN;
|
||||
(*s_brave_allowlist)[kLocationBarIsWide] =
|
||||
settings_api::PrefType::PREF_TYPE_BOOLEAN;
|
||||
(*s_brave_allowlist)[kAutocompleteEnabled] =
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
<style include="settings-shared">
|
||||
</style>
|
||||
<settings-toggle-button
|
||||
class="cr-row"
|
||||
pref="{{prefs.brave.show_bookmarks_button}}"
|
||||
label="$i18n{appearanceSettingsShowBookmarksButton}">
|
||||
</settings-toggle-button>
|
||||
<settings-toggle-button
|
||||
class="cr-row"
|
||||
pref="{{prefs.brave.location_bar_is_wide}}"
|
||||
|
||||
@@ -11,9 +11,12 @@
|
||||
|
||||
#include "base/bind.h"
|
||||
#include "brave/app/brave_command_ids.h"
|
||||
#include "brave/browser/brave_wallet/brave_wallet_context_utils.h"
|
||||
#include "brave/browser/ui/views/toolbar/bookmark_button.h"
|
||||
#include "brave/browser/ui/views/toolbar/wallet_button.h"
|
||||
#include "brave/common/pref_names.h"
|
||||
#include "brave/components/brave_vpn/buildflags/buildflags.h"
|
||||
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
|
||||
#include "chrome/app/chrome_command_ids.h"
|
||||
#include "chrome/browser/browser_process.h"
|
||||
#include "chrome/browser/defaults.h"
|
||||
@@ -27,9 +30,6 @@
|
||||
#include "components/prefs/pref_service.h"
|
||||
#include "ui/base/window_open_disposition.h"
|
||||
#include "ui/events/event.h"
|
||||
#include "brave/browser/brave_wallet/brave_wallet_context_utils.h"
|
||||
#include "brave/browser/ui/views/toolbar/wallet_button.h"
|
||||
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
|
||||
|
||||
#if BUILDFLAG(ENABLE_BRAVE_VPN)
|
||||
#include "brave/browser/brave_vpn/vpn_utils.h"
|
||||
@@ -133,6 +133,10 @@ void BraveToolbarView::Init() {
|
||||
bookmarks::prefs::kEditBookmarksEnabled, profile->GetPrefs(),
|
||||
base::BindRepeating(&BraveToolbarView::OnEditBookmarksEnabledChanged,
|
||||
base::Unretained(this)));
|
||||
show_bookmarks_button_.Init(
|
||||
kShowBookmarksButton, browser_->profile()->GetPrefs(),
|
||||
base::BindRepeating(&BraveToolbarView::OnShowBookmarksButtonChanged,
|
||||
base::Unretained(this)));
|
||||
// track changes in wide locationbar setting
|
||||
location_bar_is_wide_.Init(
|
||||
kLocationBarIsWide, profile->GetPrefs(),
|
||||
@@ -191,6 +195,13 @@ void BraveToolbarView::OnEditBookmarksEnabledChanged() {
|
||||
Update(nullptr);
|
||||
}
|
||||
|
||||
void BraveToolbarView::OnShowBookmarksButtonChanged() {
|
||||
if (!bookmark_)
|
||||
return;
|
||||
|
||||
UpdateBookmarkVisibility();
|
||||
}
|
||||
|
||||
void BraveToolbarView::OnLocationBarIsWideChanged() {
|
||||
DCHECK_EQ(DisplayMode::NORMAL, display_mode_);
|
||||
|
||||
@@ -229,11 +240,9 @@ void BraveToolbarView::LoadImages() {
|
||||
|
||||
void BraveToolbarView::Update(content::WebContents* tab) {
|
||||
ToolbarView::Update(tab);
|
||||
|
||||
// Decide whether to show the bookmark button
|
||||
if (bookmark_) {
|
||||
bookmark_->SetVisible(browser_defaults::bookmarks_enabled &&
|
||||
edit_bookmarks_enabled_.GetValue());
|
||||
}
|
||||
UpdateBookmarkVisibility();
|
||||
|
||||
// Remove avatar menu if only a single user profile exists.
|
||||
// Always show if private / tor / guest window, as an indicator.
|
||||
@@ -246,6 +255,16 @@ void BraveToolbarView::Update(content::WebContents* tab) {
|
||||
}
|
||||
}
|
||||
|
||||
void BraveToolbarView::UpdateBookmarkVisibility() {
|
||||
if (!bookmark_)
|
||||
return;
|
||||
|
||||
DCHECK_EQ(DisplayMode::NORMAL, display_mode_);
|
||||
bookmark_->SetVisible(browser_defaults::bookmarks_enabled &&
|
||||
edit_bookmarks_enabled_.GetValue() &&
|
||||
show_bookmarks_button_.GetValue());
|
||||
}
|
||||
|
||||
void BraveToolbarView::ShowBookmarkBubble(
|
||||
const GURL& url,
|
||||
bool already_bookmarked,
|
||||
|
||||
@@ -40,6 +40,7 @@ class BraveToolbarView : public ToolbarView,
|
||||
void OnThemeChanged() override;
|
||||
void OnEditBookmarksEnabledChanged();
|
||||
void OnLocationBarIsWideChanged();
|
||||
void OnShowBookmarksButtonChanged();
|
||||
void ShowBookmarkBubble(const GURL& url,
|
||||
bool already_bookmarked,
|
||||
bookmarks::BookmarkBubbleObserver* observer) override;
|
||||
@@ -48,6 +49,7 @@ class BraveToolbarView : public ToolbarView,
|
||||
void LoadImages() override;
|
||||
void ResetLocationBarBounds();
|
||||
void ResetButtonBounds();
|
||||
void UpdateBookmarkVisibility();
|
||||
|
||||
// ProfileAttributesStorage::Observer:
|
||||
void OnProfileAdded(const base::FilePath& profile_path) override;
|
||||
@@ -65,6 +67,8 @@ class BraveToolbarView : public ToolbarView,
|
||||
BooleanPrefMember show_brave_vpn_button_;
|
||||
#endif
|
||||
|
||||
BooleanPrefMember show_bookmarks_button_;
|
||||
|
||||
BooleanPrefMember location_bar_is_wide_;
|
||||
// Whether this toolbar has been initialized.
|
||||
bool brave_initialized_ = false;
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
#include "brave/browser/ui/views/frame/brave_browser_view.h"
|
||||
#include "brave/browser/ui/views/toolbar/bookmark_button.h"
|
||||
#include "brave/browser/ui/views/toolbar/brave_toolbar_view.h"
|
||||
#include "brave/common/pref_names.h"
|
||||
#include "brave/components/skus/common/features.h"
|
||||
@@ -66,7 +67,10 @@ class BraveToolbarViewTest : public InProcessBrowserTest {
|
||||
void Init(Browser* browser) {
|
||||
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
|
||||
ASSERT_NE(browser_view, nullptr);
|
||||
ASSERT_NE(browser_view->toolbar(), nullptr);
|
||||
|
||||
toolbar_view_ = static_cast<BraveToolbarView*>(browser_view->toolbar());
|
||||
ASSERT_NE(toolbar_view_, nullptr);
|
||||
|
||||
toolbar_button_provider_ = browser_view->toolbar_button_provider();
|
||||
ASSERT_NE(toolbar_button_provider_, nullptr);
|
||||
}
|
||||
@@ -78,8 +82,15 @@ class BraveToolbarViewTest : public InProcessBrowserTest {
|
||||
return button->GetVisible();
|
||||
}
|
||||
|
||||
bool is_bookmark_button_shown() {
|
||||
BookmarkButton* bookmark_button = toolbar_view_->bookmark_button();
|
||||
DCHECK(bookmark_button);
|
||||
return bookmark_button->GetVisible();
|
||||
}
|
||||
|
||||
private:
|
||||
raw_ptr<ToolbarButtonProvider> toolbar_button_provider_ = nullptr;
|
||||
raw_ptr<BraveToolbarView> toolbar_view_ = nullptr;
|
||||
|
||||
#if BUILDFLAG(ENABLE_BRAVE_VPN)
|
||||
base::test::ScopedFeatureList scoped_feature_list_;
|
||||
@@ -170,3 +181,20 @@ IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest,
|
||||
Init(browser);
|
||||
EXPECT_EQ(true, is_avatar_button_shown());
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest,
|
||||
BookmarkButtonCanBeToggledWithPref) {
|
||||
auto* prefs = browser()->profile()->GetPrefs();
|
||||
|
||||
// By default, the button should be shown.
|
||||
EXPECT_TRUE(prefs->GetBoolean(kShowBookmarksButton));
|
||||
EXPECT_TRUE(is_bookmark_button_shown());
|
||||
|
||||
// Hide button.
|
||||
prefs->SetBoolean(kShowBookmarksButton, false);
|
||||
EXPECT_FALSE(is_bookmark_button_shown());
|
||||
|
||||
// Reshowing the button should also work.
|
||||
prefs->SetBoolean(kShowBookmarksButton, true);
|
||||
EXPECT_TRUE(is_bookmark_button_shown());
|
||||
}
|
||||
|
||||
@@ -98,6 +98,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
|
||||
{"braveAdditionalSettingsTitle", IDS_SETTINGS_BRAVE_ADDITIONAL_SETTINGS},
|
||||
{"appearanceSettingsBraveTheme",
|
||||
IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_THEMES},
|
||||
{"appearanceSettingsShowBookmarksButton",
|
||||
IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_BOOKMARKS_BUTTON},
|
||||
{"appearanceSettingsLocationBarIsWide",
|
||||
IDS_SETTINGS_APPEARANCE_SETTINGS_LOCATION_BAR_IS_WIDE},
|
||||
{"appearanceSettingsShowBraveRewardsButtonLabel",
|
||||
|
||||
@@ -37,6 +37,7 @@ const char kShowAlternativeSearchEngineProviderToggle[] =
|
||||
"brave.show_alternate_private_search_engine_toggle";
|
||||
const char kAlternativeSearchEngineProviderInTor[] =
|
||||
"brave.alternate_private_search_engine_in_tor";
|
||||
const char kShowBookmarksButton[] = "brave.show_bookmarks_button";
|
||||
const char kLocationBarIsWide[] = "brave.location_bar_is_wide";
|
||||
const char kReferralDownloadID[] = "brave.referral.download_id";
|
||||
const char kReferralTimestamp[] = "brave.referral.timestamp";
|
||||
|
||||
@@ -30,6 +30,7 @@ extern const char kShowAlternativeSearchEngineProviderToggle[];
|
||||
extern const char kAlternativeSearchEngineProviderInTor[];
|
||||
extern const char kBraveThemeType[];
|
||||
extern const char kUseOverriddenBraveThemeType[];
|
||||
extern const char kShowBookmarksButton[];
|
||||
extern const char kLocationBarIsWide[];
|
||||
extern const char kReferralDownloadID[];
|
||||
extern const char kReferralTimestamp[];
|
||||
|
||||
Reference in New Issue
Block a user