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

[Brave News] Remove Brave News Subscribe button flag #16009

Merged
merged 6 commits into from
Nov 29, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@
<message name="IDS_SETTINGS_SHOW_BRAVE_REWARDS_BUTTON_LABEL" desc="The label for settings switch controlling the visibility of Brave Rewards button">
Show Brave Rewards icon in address bar
</message>
<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_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
</message>
Expand Down
12 changes: 0 additions & 12 deletions browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -321,12 +321,6 @@ constexpr char kBraveNewsCardPeekFeatureDescription[] =
"Prompt Brave News via the top featured article peeking up from the bottom "
"of the New Tab Page, after a short delay.";

constexpr char kBraveNewsSubscribeButtonName[] =
"Enable Brave News Subscribe Button";
constexpr char kBraveNewsSubscribeButtonDescription[] =
"Show a button in the toolbar to allow you to add supported sites to Brave "
"News.";

constexpr char kCryptoWalletsForNewInstallsName[] =
"Enable Crypto Wallets option in settings";
constexpr char kCryptoWalletsForNewInstallsDescription[] =
Expand Down Expand Up @@ -509,12 +503,6 @@ constexpr char kBraveBackgroundVideoPlaybackDescription[] =
flag_descriptions::kBraveNewsCardPeekFeatureDescription, \
kOsDesktop, \
FEATURE_VALUE_TYPE(brave_today::features::kBraveNewsCardPeekFeature)}, \
{"brave-news-subscribe-button", \
flag_descriptions::kBraveNewsSubscribeButtonName, \
fallaciousreasoning marked this conversation as resolved.
Show resolved Hide resolved
flag_descriptions::kBraveNewsSubscribeButtonDescription, \
kOsLinux | kOsMac | kOsWin, \
FEATURE_VALUE_TYPE( \
brave_today::features::kBraveNewsSubscribeButtonFeature)},

#define BRAVE_FEDERATED_FEATURE_ENTRIES \
{"brave-federated", \
Expand Down
6 changes: 1 addition & 5 deletions browser/brave_news/brave_news_tab_helper_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ class WaitForFeedsChanged : public BraveNewsTabHelper::PageFeedsObserver {
class BraveNewsTabHelperTest : public InProcessBrowserTest {
public:
BraveNewsTabHelperTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
features_.InitWithFeatures(
{brave_today::features::kBraveNewsSubscribeButtonFeature}, {});
fallaciousreasoning marked this conversation as resolved.
Show resolved Hide resolved
}
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}

void SetUp() override {
brave::RegisterPathProvider();
Expand Down Expand Up @@ -118,7 +115,6 @@ class BraveNewsTabHelperTest : public InProcessBrowserTest {
net::EmbeddedTestServer* https_server() { return &https_server_; }

private:
base::test::ScopedFeatureList features_;
net::EmbeddedTestServer https_server_;
content::ContentMockCertVerifier cert_verifier_;
};
Expand Down
4 changes: 1 addition & 3 deletions browser/brave_tab_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,7 @@ void AttachTabHelpers(content::WebContents* web_contents) {
ipfs::IPFSTabHelper::MaybeCreateForWebContents(web_contents);
#endif

if (base::FeatureList::IsEnabled(
brave_today::features::kBraveNewsSubscribeButtonFeature) &&
!web_contents->GetBrowserContext()->IsOffTheRecord()) {
if (!web_contents->GetBrowserContext()->IsOffTheRecord()) {
fallaciousreasoning marked this conversation as resolved.
Show resolved Hide resolved
BraveNewsTabHelper::CreateForWebContents(web_contents);
}

Expand Down
3 changes: 3 additions & 0 deletions browser/extensions/api/settings_private/brave_prefs_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "brave/components/brave_ads/common/pref_names.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/brave_shields/common/pref_names.h"
#include "brave/components/brave_today/common/pref_names.h"
#include "brave/components/brave_vpn/buildflags/buildflags.h"
#include "brave/components/brave_wallet/browser/pref_names.h"
#include "brave/components/brave_wayback_machine/buildflags.h"
Expand Down Expand Up @@ -144,6 +145,8 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() {
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_allowlist)[kShowSidePanelButton] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_allowlist)[brave_news::prefs::kShouldShowToolbarButton] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_allowlist)[kLocationBarIsWide] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_brave_allowlist)[omnibox::kAutocompleteEnabled] =
Expand Down
5 changes: 5 additions & 0 deletions browser/resources/settings/brave_appearance_page/toolbar.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
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}}"
label="$i18n{appearanceSettingsShowBraveNewsButtonLabel}">
</settings-toggle-button>
<settings-toggle-button
class="cr-row"
pref="{{prefs.brave.location_bar_is_wide}}"
Expand Down
4 changes: 1 addition & 3 deletions browser/ui/views/location_bar/brave_location_bar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ void BraveLocationBarView::Init() {
focus_ring->SetColorId(color_id.value());
}

if (base::FeatureList::IsEnabled(
brave_today::features::kBraveNewsSubscribeButtonFeature) &&
base::FeatureList::IsEnabled(brave_today::features::kBraveNewsFeature) &&
if (base::FeatureList::IsEnabled(brave_today::features::kBraveNewsFeature) &&
!browser_->profile()->IsOffTheRecord()) {
brave_news_location_view_ =
AddChildView(std::make_unique<BraveNewsLocationView>(
Expand Down
1 change: 0 additions & 1 deletion browser/ui/webui/brave_webui_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ void CustomizeWebUIHTMLSource(content::WebUI* web_ui,
{ "searchPromotionSearchBoxPlaceholderLabel", IDS_BRAVE_NEW_TAB_SEARCH_PROMOTION_SEARCH_BOX_PLACEHOLDER}, // NOLINT

{ "braveTodayTitle", IDS_BRAVE_TODAY_TITLE },
{ "braveTodayShowToolbarButton", IDS_BRAVE_TODAY_SHOW_TOOLBAR_BUTTON },
{ "braveTodayStatusFetching", IDS_BRAVE_TODAY_STATUS_FETCHING},
{ "braveTodayActionRefresh", IDS_BRAVE_TODAY_ACTION_REFRESH},
{ "braveTodayScrollHint", IDS_BRAVE_TODAY_SCROLL_HINT},
Expand Down
12 changes: 2 additions & 10 deletions browser/ui/webui/new_tab_page/brave_new_tab_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ base::Value::Dict GetPreferencesDictionary(PrefService* prefs) {
pref_data.Set("showStats", prefs->GetBoolean(kNewTabPageShowStats));
pref_data.Set("showToday",
prefs->GetBoolean(brave_news::prefs::kNewTabPageShowToday));
pref_data.Set("showBraveNewsButton",
prefs->GetBoolean(brave_news::prefs::kShouldShowToolbarButton));
pref_data.Set("showRewards", prefs->GetBoolean(kNewTabPageShowRewards));
pref_data.Set("isBrandedWallpaperNotificationDismissed",
prefs->GetBoolean(kBrandedWallpaperNotificationDismissed));
Expand Down Expand Up @@ -307,13 +305,9 @@ void BraveNewTabMessageHandler::OnJavascriptAllowed() {
base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged,
base::Unretained(this)));
pref_change_registrar_.Add(
brave_news::prefs::kShouldShowToolbarButton,
kNewTabPageShowRewards,
base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged,
base::Unretained(this))),
pref_change_registrar_.Add(
kNewTabPageShowRewards,
base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged,
base::Unretained(this)));
base::Unretained(this)));
pref_change_registrar_.Add(
kBrandedWallpaperNotificationDismissed,
base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged,
Expand Down Expand Up @@ -425,8 +419,6 @@ void BraveNewTabMessageHandler::HandleSaveNewTabPagePref(
settingsKey = kNewTabPageShowStats;
} else if (settingsKeyInput == "showToday") {
settingsKey = brave_news::prefs::kNewTabPageShowToday;
} else if (settingsKeyInput == "showBraveNewsButton") {
settingsKey = brave_news::prefs::kShouldShowToolbarButton;
} else if (settingsKeyInput == "isBraveTodayOptedIn") {
settingsKey = brave_news::prefs::kBraveTodayOptedIn;
} else if (settingsKeyInput == "showRewards") {
Expand Down
4 changes: 0 additions & 4 deletions browser/ui/webui/new_tab_page/brave_new_tab_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ BraveNewTabUI::BraveNewTabUI(content::WebUI* web_ui, const std::string& name)
source->AddBoolean(
"featureFlagBraveNewsV2Enabled",
base::FeatureList::IsEnabled(brave_today::features::kBraveNewsV2Feature));
source->AddBoolean(
"featureFlagBraveNewsSubscribeButtonEnabled",
base::FeatureList::IsEnabled(
brave_today::features::kBraveNewsSubscribeButtonFeature));

web_ui->AddMessageHandler(
base::WrapUnique(BraveNewTabMessageHandler::Create(source, profile)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_APPEARANCE_SETTINGS_LOCATION_BAR_IS_WIDE},
{"appearanceSettingsShowBraveRewardsButtonLabel",
IDS_SETTINGS_SHOW_BRAVE_REWARDS_BUTTON_LABEL},
{"appearanceSettingsShowBraveNewsButtonLabel",
IDS_SETTINGS_SHOW_BRAVE_NEWS_BUTTON_LABEL},
{"appearanceSettingsAlwaysShowBookmarkBarOnNTP",
IDS_SETTINGS_ALWAYS_SHOW_BOOKMARK_BAR_ON_NTP},
{"appearanceSettingsShowAutocompleteInAddressBar",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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/.

#include "base/metrics/histogram_base.h"
#include "base/test/metrics/histogram_tester.h"

// We use the RSS Links Fetcher from our own TabHelper, which means the
// histogram counts are not what the test expects. Get get around this, we just
// disable the checks for histogram counts.
namespace base {
namespace {
class HistogramTesterStub {
fallaciousreasoning marked this conversation as resolved.
Show resolved Hide resolved
public:
HistogramTesterStub() = default;
~HistogramTesterStub() = default;

void ExpectTotalCount(StringPiece name, HistogramBase::Count count) const {}
};
} // namespace
} // namespace base

#define HistogramTester HistogramTesterStub

#include "src/chrome/browser/feed/rss_links_fetcher_browsertest.cc"

#undef HistogramTester
4 changes: 0 additions & 4 deletions components/brave_new_tab_ui/api/preferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ export function saveShowToday (value: boolean): void {
sendSavePref('showToday', value)
}

export function saveShowBraveNewsButton (value: boolean): void {
sendSavePref('showBraveNewsButton', value)
}

export function saveShowRewards (value: boolean): void {
sendSavePref('showRewards', value)
}
Expand Down
1 change: 0 additions & 1 deletion components/brave_new_tab_ui/containers/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ function DefaultPage (props: Props) {
actions={actions}
saveShowBackgroundImage={PreferencesAPI.saveShowBackgroundImage}
saveShowToday={PreferencesAPI.saveShowToday}
saveShowBraveNewsButton={PreferencesAPI.saveShowBraveNewsButton}
saveShowRewards={PreferencesAPI.saveShowRewards}
saveShowBraveTalk={PreferencesAPI.saveShowBraveTalk}
saveBrandedWallpaperOptIn={PreferencesAPI.saveBrandedWallpaperOptIn}
Expand Down
10 changes: 0 additions & 10 deletions components/brave_new_tab_ui/containers/newTab/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ interface Props {
getBraveNewsDisplayAd: GetDisplayAdContent
saveShowBackgroundImage: (value: boolean) => void
saveShowToday: (value: boolean) => any
saveShowBraveNewsButton: (value: boolean) => any
saveShowRewards: (value: boolean) => void
saveShowBraveTalk: (value: boolean) => void
saveBrandedWallpaperOptIn: (value: boolean) => void
Expand Down Expand Up @@ -273,12 +272,6 @@ class NewTabPage extends React.Component<Props, State> {
this.props.actions.setMostVisitedSettings(!showTopSites, customLinksEnabled)
}

toggleShowBraveNewsButton = () => {
this.props.saveShowBraveNewsButton(
!this.props.newTabData.showBraveNewsButton
)
}

toggleCustomLinksEnabled = () => {
const { showTopSites, customLinksEnabled } = this.props.newTabData
this.props.actions.setMostVisitedSettings(showTopSites, !customLinksEnabled)
Expand Down Expand Up @@ -714,14 +707,12 @@ class NewTabPage extends React.Component<Props, State> {
showSettingsMenu={showSettingsMenu}
featureFlagBraveNewsEnabled={newTabData.featureFlagBraveNewsEnabled}
featureCustomBackgroundEnabled={newTabData.featureCustomBackgroundEnabled}
featureFlagBraveNewsSubscribeButtonEnabled={newTabData.featureFlagBraveNewsSubscribeButtonEnabled}
onClose={this.closeSettings}
setActiveTab={this.state.activeSettingsTab || undefined}
onDisplayTodaySection={this.props.actions.today.ensureSettingsData}
onClearTodayPrefs={this.props.actions.today.resetTodayPrefsToDefault}
toggleShowBackgroundImage={this.toggleShowBackgroundImage}
toggleShowToday={this.toggleShowToday}
toggleShowBraveNewsButton={this.toggleShowBraveNewsButton}
toggleShowTopSites={this.toggleShowTopSites}
setMostVisitedSettings={this.setMostVisitedSettings}
toggleBrandedWallpaperOptIn={this.toggleShowBrandedWallpaper}
Expand All @@ -732,7 +723,6 @@ class NewTabPage extends React.Component<Props, State> {
setColorBackground={this.props.setColorBackground}
showBackgroundImage={newTabData.showBackgroundImage}
showToday={newTabData.showToday}
showBraveNewsButton={newTabData.showBraveNewsButton}
showTopSites={newTabData.showTopSites}
customLinksEnabled={newTabData.customLinksEnabled}
showRewards={newTabData.showRewards}
Expand Down
6 changes: 0 additions & 6 deletions components/brave_new_tab_ui/containers/newTab/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,11 @@ export interface Props {
showSettingsMenu: boolean
featureFlagBraveNewsEnabled: boolean
featureCustomBackgroundEnabled: boolean
featureFlagBraveNewsSubscribeButtonEnabled: boolean
onClose: () => void
onDisplayTodaySection: () => any
onClearTodayPrefs: () => any
toggleShowBackgroundImage: () => void
toggleShowToday: () => any
toggleShowBraveNewsButton: () => any
toggleShowTopSites: () => void
setMostVisitedSettings: (show: boolean, customize: boolean) => void
toggleShowRewards: () => void
Expand All @@ -72,7 +70,6 @@ export interface Props {
onEnableRewards: () => void
showBackgroundImage: boolean
showToday: boolean
showBraveNewsButton: boolean
showTopSites: boolean
customLinksEnabled: boolean
brandedWallpaperOptIn: boolean
Expand Down Expand Up @@ -375,9 +372,6 @@ export default class Settings extends React.PureComponent<Props, State> {
onClearPrefs={this.props.onClearTodayPrefs}
showToday={this.props.showToday}
toggleShowToday={this.props.toggleShowToday}
showBraveNewsButton={this.props.showBraveNewsButton}
featureFlagBraveNewsSubscribeButtonEnabled={this.props.featureFlagBraveNewsSubscribeButtonEnabled}
toggleShowBraveNewsButton={this.props.toggleShowBraveNewsButton}
/>
) : null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ export interface Props {
onClearPrefs: () => any
showToday: boolean
toggleShowToday: () => any
showBraveNewsButton: boolean
featureFlagBraveNewsSubscribeButtonEnabled: boolean
toggleShowBraveNewsButton: () => any
}

export default function BraveTodayPrefs (props: Props) {
Expand Down Expand Up @@ -61,16 +58,6 @@ export default function BraveTodayPrefs (props: Props) {
/>
</SettingsRow>
)}
{!category &&
props.showToday &&
props.featureFlagBraveNewsSubscribeButtonEnabled &&
<SettingsRow>
<SettingsText>{getLocale('braveTodayShowToolbarButton')}</SettingsText>
<Toggle
checked={props.showBraveNewsButton}
onChange={props.toggleShowBraveNewsButton}
size='large'/>
</SettingsRow>}
{shouldShowSources &&
<Sources category={category} setCategory={setCategory} {...props} />
}
Expand Down
2 changes: 0 additions & 2 deletions components/brave_new_tab_ui/storage/new_tab_storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@ export const defaultState: NewTab.State = {
featureFlagBraveNewsEnabled: loadTimeData.getBoolean('featureFlagBraveNewsEnabled'),
featureFlagBraveNewsV2Enabled: loadTimeData.getBoolean('featureFlagBraveNewsV2Enabled'),
featureFlagBraveNewsPromptEnabled: loadTimeData.getBoolean('featureFlagBraveNewsPromptEnabled'),
featureFlagBraveNewsSubscribeButtonEnabled: loadTimeData.getBoolean('featureFlagBraveNewsSubscribeButtonEnabled'),
featureCustomBackgroundEnabled: loadTimeData.getBoolean('featureCustomBackgroundEnabled'),
searchPromotionEnabled: false,
showBackgroundImage: false,
showStats: false,
showToday: false,
showBraveNewsButton: false,
showClock: false,
clockFormat: '',
showTopSites: false,
Expand Down
1 change: 0 additions & 1 deletion components/brave_new_tab_ui/stories/regular.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ export const Regular = () => {
actions={getActions()}
saveShowBackgroundImage={doNothing}
saveShowToday={doNothing}
saveShowBraveNewsButton={doNothing}
saveShowRewards={doNothing}
saveShowBraveTalk={doNothing}
saveBrandedWallpaperOptIn={onShowBrandedImageChanged}
Expand Down
3 changes: 0 additions & 3 deletions components/brave_today/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ BASE_FEATURE(kBraveNewsV2Feature,
BASE_FEATURE(kBraveNewsCardPeekFeature,
"BraveNewsCardPeek",
base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kBraveNewsSubscribeButtonFeature,
"BraveNewsSubscribeButton",
base::FEATURE_DISABLED_BY_DEFAULT);

} // namespace features
} // namespace brave_today
1 change: 0 additions & 1 deletion components/brave_today/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ namespace features {
BASE_DECLARE_FEATURE(kBraveNewsFeature);
BASE_DECLARE_FEATURE(kBraveNewsV2Feature);
BASE_DECLARE_FEATURE(kBraveNewsCardPeekFeature);
BASE_DECLARE_FEATURE(kBraveNewsSubscribeButtonFeature);

} // namespace features
} // namespace brave_today
Expand Down
2 changes: 0 additions & 2 deletions components/definitions/newTab.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ declare namespace NewTab {
brandedWallpaperOptIn: boolean
showStats: boolean
showToday: boolean
showBraveNewsButton: boolean
showClock: boolean
clockFormat: string
showTopSites: boolean
Expand All @@ -136,7 +135,6 @@ declare namespace NewTab {
featureFlagBraveNewsEnabled: boolean
featureFlagBraveNewsV2Enabled: boolean
featureFlagBraveNewsPromptEnabled: boolean
featureFlagBraveNewsSubscribeButtonEnabled: boolean
searchPromotionEnabled: boolean
featureCustomBackgroundEnabled: boolean
isIncognito: boolean
Expand Down
3 changes: 0 additions & 3 deletions components/resources/brave_components_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,6 @@
<message name="IDS_BRAVE_TODAY_TITLE" translateable="false" desc="Title for Brave News news feed section">
Brave News
</message>
<message name="IDS_BRAVE_TODAY_SHOW_TOOLBAR_BUTTON" desc="Button text for opting in to Brave News">
Show Brave News button in the toolbar
</message>
<message name="IDS_BRAVE_NEWS_BUBBLE_FEED_ITEM_LOADING" desc="The text displayed while a feed is being subscribed or unsubscribed">
Loading...
</message>
Expand Down