From 44d717bf7c4809192e122e026bc8d9b4a0fb28d6 Mon Sep 17 00:00:00 2001 From: Ian Krieger Date: Wed, 23 Oct 2024 19:17:37 -0400 Subject: [PATCH 1/2] feat(ntp): store last search engine in widget as a pref --- .../brave_new_tab_message_handler.cc | 101 +++++++++--------- .../components/search/EngineContext.tsx | 83 ++++++++++++++ .../components/search/SearchContext.tsx | 15 +-- .../components/search/SearchEngineIcon.tsx | 2 +- .../components/search/config.ts | 62 +---------- .../containers/newTab/index.tsx | 7 +- .../containers/newTab/settings/search.tsx | 22 ++-- .../storage/new_tab_storage.ts | 3 +- .../brave_search_conversion/pref_names.h | 3 + components/brave_search_conversion/utils.cc | 2 + components/definitions/newTab.d.ts | 1 + 11 files changed, 176 insertions(+), 125 deletions(-) create mode 100644 components/brave_new_tab_ui/components/search/EngineContext.tsx diff --git a/browser/ui/webui/new_tab_page/brave_new_tab_message_handler.cc b/browser/ui/webui/new_tab_page/brave_new_tab_message_handler.cc index f44497dd4b20..1e9233eab60f 100644 --- a/browser/ui/webui/new_tab_page/brave_new_tab_message_handler.cc +++ b/browser/ui/webui/new_tab_page/brave_new_tab_message_handler.cc @@ -11,18 +11,14 @@ #include #include "base/functional/bind.h" -#include "base/json/json_writer.h" #include "base/json/values_util.h" #include "base/memory/weak_ptr.h" -#include "base/metrics/histogram_macros.h" #include "base/threading/thread_restrictions.h" -#include "base/time/time.h" #include "base/values.h" #include "brave/browser/brave_ads/ads_service_factory.h" #include "brave/browser/ntp_background/view_counter_service_factory.h" #include "brave/browser/profiles/profile_util.h" #include "brave/browser/search_engines/pref_names.h" -#include "brave/browser/ui/webui/new_tab_page/brave_new_tab_ui.h" #include "brave/components/brave_ads/core/public/ads_util.h" #include "brave/components/brave_news/common/pref_names.h" #include "brave/components/brave_perf_predictor/common/pref_names.h" @@ -32,13 +28,11 @@ #include "brave/components/ntp_background_images/browser/view_counter_service.h" #include "brave/components/ntp_background_images/common/pref_names.h" #include "brave/components/p3a/utils.h" -#include "brave/components/services/bat_ads/public/interfaces/bat_ads.mojom.h" #include "brave/components/time_period_storage/weekly_storage.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/first_run/first_run.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/webui/plural_string_handler.h" -#include "chrome/common/chrome_features.h" #include "chrome/common/pref_names.h" #include "components/grit/brave_components_strings.h" #include "components/prefs/pref_change_registrar.h" @@ -95,6 +89,9 @@ base::Value::Dict GetPreferencesDictionary(PrefService* prefs) { pref_data.Set( "showSearchBox", prefs->GetBoolean(brave_search_conversion::prefs::kShowNTPSearchBox)); + pref_data.Set("lastUsedNtpSearchEngine", + prefs->GetString( + brave_search_conversion::prefs::kLastUsedNTPSearchEngine)); pref_data.Set("promptEnableSearchSuggestions", prefs->GetBoolean( brave_search_conversion::prefs::kPromptEnableSuggestions)); @@ -292,6 +289,10 @@ void BraveNewTabMessageHandler::OnJavascriptAllowed() { brave_search_conversion::prefs::kShowNTPSearchBox, base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged, base::Unretained(this))); + pref_change_registrar_.Add( + brave_search_conversion::prefs::kLastUsedNTPSearchEngine, + base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged, + base::Unretained(this))); pref_change_registrar_.Add( brave_search_conversion::prefs::kPromptEnableSuggestions, base::BindRepeating(&BraveNewTabMessageHandler::OnPreferencesChanged, @@ -393,73 +394,75 @@ void BraveNewTabMessageHandler::HandleSaveNewTabPagePref( } PrefService* prefs = profile_->GetPrefs(); // Collect args - std::string settingsKeyInput = args[0].GetString(); - auto settingsValue = args[1].Clone(); - std::string settingsKey; + std::string settings_key_input = args[0].GetString(); + auto settings_value = args[1].Clone(); + std::string settings_key; // Prevent News onboarding below NTP and sponsored NTP notification // state from triggering the "shown & changed" answer for the // customize dialog metric. - if (settingsKeyInput != "showToday" && - settingsKeyInput != "isBraveNewsOptedIn" && - settingsKeyInput != "isBrandedWallpaperNotificationDismissed") { + if (settings_key_input != "showToday" && + settings_key_input != "isBraveNewsOptedIn" && + settings_key_input != "isBrandedWallpaperNotificationDismissed") { p3a::RecordValueIfGreater( NTPCustomizeUsage::kOpenedAndEdited, kCustomizeUsageHistogramName, kNTPCustomizeUsageStatus, g_browser_process->local_state()); } // Handle string settings - if (settingsValue.is_string()) { - const auto settingsValueString = settingsValue.GetString(); - if (settingsKeyInput == "clockFormat") { - settingsKey = kNewTabPageClockFormat; + if (settings_value.is_string()) { + const auto settings_value_string = settings_value.GetString(); + if (settings_key_input == "clockFormat") { + settings_key = kNewTabPageClockFormat; + } else if (settings_key_input == "lastUsedNtpSearchEngine") { + settings_key = brave_search_conversion::prefs::kLastUsedNTPSearchEngine; } else { LOG(ERROR) << "Invalid setting key"; return; } - prefs->SetString(settingsKey, settingsValueString); + prefs->SetString(settings_key, settings_value_string); return; } // Handle bool settings - if (!settingsValue.is_bool()) { + if (!settings_value.is_bool()) { LOG(ERROR) << "Invalid value type"; return; } - const auto settingsValueBool = settingsValue.GetBool(); - if (settingsKeyInput == "showBackgroundImage") { - settingsKey = kNewTabPageShowBackgroundImage; - } else if (settingsKeyInput == "brandedWallpaperOptIn") { + const auto settings_value_bool = settings_value.GetBool(); + if (settings_key_input == "showBackgroundImage") { + settings_key = kNewTabPageShowBackgroundImage; + } else if (settings_key_input == "brandedWallpaperOptIn") { // TODO(simonhong): I think above |brandedWallpaperOptIn| should be changed // to |sponsoredImagesWallpaperOptIn|. - settingsKey = kNewTabPageShowSponsoredImagesBackgroundImage; - } else if (settingsKeyInput == "showClock") { - settingsKey = kNewTabPageShowClock; - } else if (settingsKeyInput == "showStats") { - settingsKey = kNewTabPageShowStats; - } else if (settingsKeyInput == "showToday") { - settingsKey = brave_news::prefs::kNewTabPageShowToday; - } else if (settingsKeyInput == "isBraveNewsOptedIn") { - settingsKey = brave_news::prefs::kBraveNewsOptedIn; - } else if (settingsKeyInput == "showRewards") { - settingsKey = kNewTabPageShowRewards; - } else if (settingsKeyInput == "isBrandedWallpaperNotificationDismissed") { - settingsKey = kBrandedWallpaperNotificationDismissed; - } else if (settingsKeyInput == "hideAllWidgets") { - settingsKey = kNewTabPageHideAllWidgets; - } else if (settingsKeyInput == "showBraveTalk") { - settingsKey = kNewTabPageShowBraveTalk; - } else if (settingsKeyInput == "showSearchBox") { - settingsKey = brave_search_conversion::prefs::kShowNTPSearchBox; - } else if (settingsKeyInput == "promptEnableSearchSuggestions") { - settingsKey = brave_search_conversion::prefs::kPromptEnableSuggestions; - } else if (settingsKeyInput == "searchSuggestionsEnabled") { - settingsKey = prefs::kSearchSuggestEnabled; + settings_key = kNewTabPageShowSponsoredImagesBackgroundImage; + } else if (settings_key_input == "showClock") { + settings_key = kNewTabPageShowClock; + } else if (settings_key_input == "showStats") { + settings_key = kNewTabPageShowStats; + } else if (settings_key_input == "showToday") { + settings_key = brave_news::prefs::kNewTabPageShowToday; + } else if (settings_key_input == "isBraveNewsOptedIn") { + settings_key = brave_news::prefs::kBraveNewsOptedIn; + } else if (settings_key_input == "showRewards") { + settings_key = kNewTabPageShowRewards; + } else if (settings_key_input == "isBrandedWallpaperNotificationDismissed") { + settings_key = kBrandedWallpaperNotificationDismissed; + } else if (settings_key_input == "hideAllWidgets") { + settings_key = kNewTabPageHideAllWidgets; + } else if (settings_key_input == "showBraveTalk") { + settings_key = kNewTabPageShowBraveTalk; + } else if (settings_key_input == "showSearchBox") { + settings_key = brave_search_conversion::prefs::kShowNTPSearchBox; + } else if (settings_key_input == "promptEnableSearchSuggestions") { + settings_key = brave_search_conversion::prefs::kPromptEnableSuggestions; + } else if (settings_key_input == "searchSuggestionsEnabled") { + settings_key = prefs::kSearchSuggestEnabled; } else { LOG(ERROR) << "Invalid setting key"; return; } - prefs->SetBoolean(settingsKey, settingsValueBool); + prefs->SetBoolean(settings_key, settings_value_bool); } void BraveNewTabMessageHandler::HandleRegisterNewTabPageView( @@ -541,10 +544,10 @@ void BraveNewTabMessageHandler::HandleGetWallpaperData( // Even though we show sponsored image, we should pass "Background wallpaper" // data so that NTP customization menu can know which wallpaper is selected by // users. - auto backgroundWallpaper = service->GetCurrentWallpaper(); + auto background_wallpaper = service->GetCurrentWallpaper(); wallpaper.Set(kBackgroundWallpaperKey, - backgroundWallpaper - ? base::Value(std::move(*backgroundWallpaper)) + background_wallpaper + ? base::Value(std::move(*background_wallpaper)) : base::Value()); const std::string* creative_instance_id = diff --git a/components/brave_new_tab_ui/components/search/EngineContext.tsx b/components/brave_new_tab_ui/components/search/EngineContext.tsx new file mode 100644 index 000000000000..3fb363c72cca --- /dev/null +++ b/components/brave_new_tab_ui/components/search/EngineContext.tsx @@ -0,0 +1,83 @@ +// Copyright (c) 2024 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 { SearchEngineInfo } from '../../api/background' +import { useNewTabPref } from '../../hooks/usePref' +import * as React from 'react'; +import { ENABLED_SEARCH_ENGINES_KEY, LAST_SEARCH_ENGINE_KEY, braveSearchHost } from './config'; + +interface Engine { + setLastSearchEngine: (engine: SearchEngineInfo) => void + lastSearchEngine: string, + engineConfig: Record, + setEngineConfig: (engine: string, enabled: boolean) => void, +} + +const EngineContext = React.createContext({ + setLastSearchEngine: () => { }, + lastSearchEngine: '', + engineConfig: {}, + setEngineConfig: () => { }, +}) + +export function EngineContextProvider(props: React.PropsWithChildren<{}>) { + const [lastUsed, setLastUsed] = useNewTabPref('lastUsedNtpSearchEngine') + const lastLocalStorage = localStorage.getItem(LAST_SEARCH_ENGINE_KEY) + const [config, setConfig] = React.useState>(() => { + const localStorageValue = localStorage.getItem(ENABLED_SEARCH_ENGINES_KEY); + if (localStorageValue) { + return JSON.parse(localStorageValue); + } + return { + // Default to enabling Brave Search + [braveSearchHost]: true + }; + }); + + const setLastNtpSearchEngine = React.useCallback((engine: SearchEngineInfo) => { + setLastUsed(engine.host); + }, [setLastUsed]) + + const lastNtpSearchEngine = React.useMemo(() => { + let last = lastUsed; + + // Migrates the existing local storage value to the new pref value, letting the local storage value take precedence. + if (lastLocalStorage && config[lastLocalStorage] && lastUsed !== lastLocalStorage) { + setLastUsed(lastLocalStorage); + last = lastLocalStorage; + localStorage.removeItem(LAST_SEARCH_ENGINE_KEY) + } + + // If the last search engine we used has been disabled or doesn't exist in the config, return the first enabled + // one, or Brave Search. + if (!last || !config[last]) { + return Object.keys(config).find(key => config[key]) ?? braveSearchHost + } + + return last + }, [lastUsed, lastLocalStorage, config]); + + const setEngineEnabled = React.useCallback(((engine: string, enabled: boolean) => { + setConfig(prevConfig => { + const newState = { ...prevConfig, [engine]: enabled }; + localStorage.setItem(ENABLED_SEARCH_ENGINES_KEY, JSON.stringify(newState)); + return newState; + }); + }), []) + + return ( + + {props.children} + + ) +} + +export function useEngineContext() { + return React.useContext(EngineContext) +} diff --git a/components/brave_new_tab_ui/components/search/SearchContext.tsx b/components/brave_new_tab_ui/components/search/SearchContext.tsx index 8d06ffc66d41..eea2fe963c80 100644 --- a/components/brave_new_tab_ui/components/search/SearchContext.tsx +++ b/components/brave_new_tab_ui/components/search/SearchContext.tsx @@ -8,7 +8,7 @@ import { AutocompleteResult, OmniboxPopupSelection, PageHandler, PageHandlerRemo import { stringToMojoString16 } from 'chrome://resources/js/mojo_type_util.js'; import * as React from 'react'; import getNTPBrowserAPI, { SearchEngineInfo } from '../../api/background'; -import { getDefaultSearchEngine, isSearchEngineEnabled, setDefaultSearchEngine } from './config'; +import { useEngineContext } from './EngineContext'; interface Context { open: boolean, @@ -23,7 +23,7 @@ interface Context { const Context = React.createContext({ open: false, - setOpen: () => {}, + setOpen: () => { }, query: '', setQuery: () => { }, searchEngine: undefined, @@ -73,18 +73,19 @@ class SearchPage implements PageInterface { for (const listener of this.selectionListeners) listener(selection) } - setInputText(inputText: string) {} - setThumbnail(thumbnailUrl: string) {} + setInputText(inputText: string) { } + setThumbnail(thumbnailUrl: string) { } } export const search = new SearchPage() export function SearchContext(props: React.PropsWithChildren<{}>) { + const { engineConfig, lastSearchEngine, setLastSearchEngine } = useEngineContext() const [open, setOpen] = React.useState(false) const [searchEngine, setSearchEngineInternal] = React.useState() const [query, setQuery] = React.useState('') const { result: searchEngines = [] } = usePromise(() => searchEnginesPromise, []) - const filteredSearchEngines = searchEngines.filter(isSearchEngineEnabled) + const filteredSearchEngines = searchEngines.filter(s => engineConfig[s.host]) const setSearchEngine = React.useCallback((engine: SearchEngineInfo | string) => { if (typeof engine === 'string') { @@ -93,7 +94,7 @@ export function SearchContext(props: React.PropsWithChildren<{}>) { if (!engine) return - setDefaultSearchEngine(engine) + setLastSearchEngine(engine) getNTPBrowserAPI().newTabMetrics.reportNTPSearchDefaultEngine(engine.prepopulateId) setSearchEngineInternal(engine) }, [searchEngines]); @@ -102,7 +103,7 @@ export function SearchContext(props: React.PropsWithChildren<{}>) { React.useEffect(() => { if (!searchEngines.length) return - const match = filteredSearchEngines.find(s => s.host === getDefaultSearchEngine()) + const match = filteredSearchEngines.find(s => s.host === lastSearchEngine) ?? searchEngines[0] getNTPBrowserAPI().newTabMetrics.reportNTPSearchDefaultEngine(match.prepopulateId) setSearchEngine(match) diff --git a/components/brave_new_tab_ui/components/search/SearchEngineIcon.tsx b/components/brave_new_tab_ui/components/search/SearchEngineIcon.tsx index f05d7d52ccaa..4abf0efa784b 100644 --- a/components/brave_new_tab_ui/components/search/SearchEngineIcon.tsx +++ b/components/brave_new_tab_ui/components/search/SearchEngineIcon.tsx @@ -41,7 +41,7 @@ function MaybeImage(props: React.DetailedHTMLProps diff --git a/components/brave_new_tab_ui/components/search/config.ts b/components/brave_new_tab_ui/components/search/config.ts index 3361ce478300..ea578b9879f7 100644 --- a/components/brave_new_tab_ui/components/search/config.ts +++ b/components/brave_new_tab_ui/components/search/config.ts @@ -2,62 +2,10 @@ // 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 { radius } from "@brave/leo/tokens/css/variables"; -import { SearchEngineInfo } from "../../api/background"; - -// At some point we might want to store this in prefs, but -// at this point its only used on Desktop, and only on the NTP -// so this seems fine. -const ENABLED_SEARCH_ENGINES_KEY = 'search-engines' -const LAST_SEARCH_ENGINE_KEY = 'last-search-engine' +import { radius } from "@brave/leo/tokens/css/variables" +// You can obtain one at https://mozilla.org/MPL/2.0/. +export const ENABLED_SEARCH_ENGINES_KEY = 'search-engines' +export const LAST_SEARCH_ENGINE_KEY = 'last-search-engine' export const braveSearchHost = 'search.brave.com' -export const searchBoxRadius = radius.xl; - -let cache: Record | undefined - -const getConfig = () => { - if (!cache) { - cache = JSON.parse(localStorage.getItem(ENABLED_SEARCH_ENGINES_KEY)!) ?? { - // Default to enabling Brave Search - [braveSearchHost]: true - } - } - return cache! -} - -// Determines whether any search engines are enabled -export const hasEnabledEngine = () => Object.values(getConfig()).find(c => c) - -export const maybeEnableDefaultEngine = () => { - // If no engines are enabled, enable the default one. - if (!hasEnabledEngine()) { - setEngineEnabled({ host: braveSearchHost }, true) - } -} - -export const setEngineEnabled = (engine: { host: string }, enabled: boolean) => { - const config = getConfig() - config[engine.host] = enabled - - localStorage.setItem(ENABLED_SEARCH_ENGINES_KEY, JSON.stringify(config)) -} - -export const isSearchEngineEnabled = (engine: SearchEngineInfo) => getConfig()[engine.host] - -export const getDefaultSearchEngine = () => { - const config = getConfig() - const last = localStorage.getItem(LAST_SEARCH_ENGINE_KEY) - - // If the last search engine we used has been disabled, return the first enabled - // one, or Brave Search. - if (!config[last!]) { - return Object.entries(config).find(([key, value]) => value)?.[0] - ?? braveSearchHost - } - return last -} - -export const setDefaultSearchEngine = (engine: SearchEngineInfo) => { - localStorage.setItem(LAST_SEARCH_ENGINE_KEY, engine.host) -} +export const searchBoxRadius = radius.xl diff --git a/components/brave_new_tab_ui/containers/newTab/index.tsx b/components/brave_new_tab_ui/containers/newTab/index.tsx index c6deb0a24204..f4e4f3002ae3 100644 --- a/components/brave_new_tab_ui/containers/newTab/index.tsx +++ b/components/brave_new_tab_ui/containers/newTab/index.tsx @@ -44,6 +44,7 @@ import Icon from '@brave/leo/react/icon' import * as style from './style' import { defaultState } from '../../storage/new_tab_storage' +import { EngineContextProvider } from '../../components/search/EngineContext' const BraveNewsPeek = React.lazy(() => import('../../../brave_news/browser/resources/Peek')) const SearchPlaceholder = React.lazy(() => import('../../components/search/SearchPlaceholder')) @@ -273,7 +274,7 @@ class NewTabPage extends React.Component { if (activeSettingsTabRaw) { const allSettingsTabTypes = [...Object.keys(SettingsTabType)] if (allSettingsTabTypes.includes(activeSettingsTabRaw)) { - activeSettingsTab = SettingsTabType[activeSettingsTabRaw] + activeSettingsTab = SettingsTabType[activeSettingsTabRaw as keyof typeof SettingsTabType] } } this.setState({ showSettingsMenu: true, activeSettingsTab }) @@ -403,7 +404,7 @@ class NewTabPage extends React.Component { showRewards, showBraveTalk } = this.props.newTabData - const lookup = { + const lookup: { [p: string]: { display: boolean, render: any } } = { 'rewards': { display: braveRewardsSupported && showRewards, render: this.renderRewardsWidget.bind(this) @@ -621,6 +622,7 @@ class NewTabPage extends React.Component { data-show-news-prompt={((this.state.backgroundHasLoaded || colorForBackground) && this.state.isPromptingBraveNews && !defaultState.featureFlagBraveNewsFeedV2Enabled) ? true : undefined}> + { /> : null } + ) diff --git a/components/brave_new_tab_ui/containers/newTab/settings/search.tsx b/components/brave_new_tab_ui/containers/newTab/settings/search.tsx index a52a8db1879d..eb3ae93deacf 100644 --- a/components/brave_new_tab_ui/containers/newTab/settings/search.tsx +++ b/components/brave_new_tab_ui/containers/newTab/settings/search.tsx @@ -20,9 +20,10 @@ import { } from '../../../components/default' import { searchEnginesPromise } from '../../../components/search/SearchContext' import { MediumSearchEngineIcon } from '../../../components/search/SearchEngineIcon' -import { hasEnabledEngine, isSearchEngineEnabled, maybeEnableDefaultEngine, setEngineEnabled } from '../../../components/search/config' import { useNewTabPref } from '../../../hooks/usePref' import { getLocale } from '../../../../common/locale' +import { braveSearchHost } from '../../../components/search/config' +import { useEngineContext } from '../../../components/search/EngineContext' const EnginesContainer = styled(Flex)` font: ${font.default.regular}; @@ -70,6 +71,14 @@ const CheckboxText = styled.span`flex: 1`; export default function SearchSettings() { const { result: engines = [] } = usePromise(() => searchEnginesPromise, []) const [showSearchBox, setShowSearchBox] = useNewTabPref('showSearchBox') + const { setEngineConfig, engineConfig } = useEngineContext() + const hasEnabledEngine = (config: Record) => Object.keys(config).some(key => config[key]) + + React.useEffect(() => { + if (!hasEnabledEngine(engineConfig)) { + setShowSearchBox(false) + } + }, [engineConfig]) return @@ -79,8 +88,8 @@ export default function SearchSettings() { // If we've just enabled the searchbox, make sure at least one engine // is enabled. - if (e.checked) { - maybeEnableDefaultEngine() + if (e.checked && !hasEnabledEngine(engineConfig)) { + setEngineConfig(braveSearchHost, true) } }} /> @@ -90,11 +99,8 @@ export default function SearchSettings() {
{getLocale('searchEnableSearchEnginesTitle')}
{engines.map(engine => - { - setEngineEnabled(engine, e.checked) - if (!hasEnabledEngine()) { - setShowSearchBox(false) - } + { + setEngineConfig(engine.host, e.checked) }}> {engine.name} diff --git a/components/brave_new_tab_ui/storage/new_tab_storage.ts b/components/brave_new_tab_ui/storage/new_tab_storage.ts index fe253ba06f92..86faa22c95a2 100644 --- a/components/brave_new_tab_ui/storage/new_tab_storage.ts +++ b/components/brave_new_tab_ui/storage/new_tab_storage.ts @@ -28,6 +28,7 @@ export const defaultState: NewTab.State = { showRewards: false, showBraveTalk: false, showSearchBox: true, + lastUsedNtpSearchEngine: "search.brave.com", promptEnableSearchSuggestions: true, searchSuggestionsEnabled: false, showBitcoinDotCom: false, @@ -115,7 +116,7 @@ export const replaceStackWidgets = (state: NewTab.State) => { braveRewardsSupported, braveTalkSupported } = state - const displayLookup = { + const displayLookup: { [p: string]: { display: boolean } } = { 'rewards': { display: braveRewardsSupported && showRewards }, diff --git a/components/brave_search_conversion/pref_names.h b/components/brave_search_conversion/pref_names.h index d1db191df841..64a750005943 100644 --- a/components/brave_search_conversion/pref_names.h +++ b/components/brave_search_conversion/pref_names.h @@ -27,6 +27,9 @@ inline constexpr char kLatestDDGBannerTypeFirstShownTime[] = inline constexpr char kShowNTPSearchBox[] = "brave.brave_search.show-ntp-search"; +inline constexpr char kLastUsedNTPSearchEngine[] = + "brave.brave_search.last-used-ntp-search-engine"; + // Determines whether the search box on the NTP prompts the user to enable // search suggestions. inline constexpr char kPromptEnableSuggestions[] = diff --git a/components/brave_search_conversion/utils.cc b/components/brave_search_conversion/utils.cc index 6bc42ab5fa18..59f285ba7cbc 100644 --- a/components/brave_search_conversion/utils.cc +++ b/components/brave_search_conversion/utils.cc @@ -171,6 +171,8 @@ ConversionType GetConversionType(PrefService* prefs, void RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterBooleanPref(prefs::kDismissed, false); registry->RegisterBooleanPref(prefs::kShowNTPSearchBox, true); + registry->RegisterStringPref(prefs::kLastUsedNTPSearchEngine, + "search.brave.com"); registry->RegisterBooleanPref(prefs::kPromptEnableSuggestions, true); registry->RegisterTimePref(prefs::kMaybeLaterClickedTime, base::Time()); registry->RegisterIntegerPref(prefs::kDDGBannerTypeIndex, 0); diff --git a/components/definitions/newTab.d.ts b/components/definitions/newTab.d.ts index d68a6ab62f30..85a51e26196a 100644 --- a/components/definitions/newTab.d.ts +++ b/components/definitions/newTab.d.ts @@ -122,6 +122,7 @@ declare namespace NewTab { showRewards: boolean showBraveTalk: boolean showSearchBox: boolean + lastUsedNtpSearchEngine: string promptEnableSearchSuggestions: boolean searchSuggestionsEnabled: boolean hideAllWidgets: boolean From 860cf0f44cf609fb5f0f113261a3386f11495263 Mon Sep 17 00:00:00 2001 From: Ian Krieger Date: Fri, 25 Oct 2024 09:17:19 -0400 Subject: [PATCH 2/2] fix: lift functions, remove magic string --- .../components/search/EngineContext.tsx | 22 ++++++++++--------- .../containers/newTab/settings/search.tsx | 2 +- .../storage/new_tab_storage.ts | 3 ++- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/components/brave_new_tab_ui/components/search/EngineContext.tsx b/components/brave_new_tab_ui/components/search/EngineContext.tsx index 3fb363c72cca..aa8c1a98fc60 100644 --- a/components/brave_new_tab_ui/components/search/EngineContext.tsx +++ b/components/brave_new_tab_ui/components/search/EngineContext.tsx @@ -21,19 +21,21 @@ const EngineContext = React.createContext({ setEngineConfig: () => { }, }) +const searchEngineConfig = () => { + const localStorageValue = localStorage.getItem(ENABLED_SEARCH_ENGINES_KEY); + if (localStorageValue) { + return JSON.parse(localStorageValue); + } + return { + // Default to enabling Brave Search + [braveSearchHost]: true + }; +} + export function EngineContextProvider(props: React.PropsWithChildren<{}>) { const [lastUsed, setLastUsed] = useNewTabPref('lastUsedNtpSearchEngine') const lastLocalStorage = localStorage.getItem(LAST_SEARCH_ENGINE_KEY) - const [config, setConfig] = React.useState>(() => { - const localStorageValue = localStorage.getItem(ENABLED_SEARCH_ENGINES_KEY); - if (localStorageValue) { - return JSON.parse(localStorageValue); - } - return { - // Default to enabling Brave Search - [braveSearchHost]: true - }; - }); + const [config, setConfig] = React.useState>(searchEngineConfig); const setLastNtpSearchEngine = React.useCallback((engine: SearchEngineInfo) => { setLastUsed(engine.host); diff --git a/components/brave_new_tab_ui/containers/newTab/settings/search.tsx b/components/brave_new_tab_ui/containers/newTab/settings/search.tsx index eb3ae93deacf..e5cf9e608eda 100644 --- a/components/brave_new_tab_ui/containers/newTab/settings/search.tsx +++ b/components/brave_new_tab_ui/containers/newTab/settings/search.tsx @@ -67,12 +67,12 @@ const Hr = styled.hr` ` const CheckboxText = styled.span`flex: 1`; +const hasEnabledEngine = (config: Record) => Object.keys(config).some(key => config[key]) export default function SearchSettings() { const { result: engines = [] } = usePromise(() => searchEnginesPromise, []) const [showSearchBox, setShowSearchBox] = useNewTabPref('showSearchBox') const { setEngineConfig, engineConfig } = useEngineContext() - const hasEnabledEngine = (config: Record) => Object.keys(config).some(key => config[key]) React.useEffect(() => { if (!hasEnabledEngine(engineConfig)) { diff --git a/components/brave_new_tab_ui/storage/new_tab_storage.ts b/components/brave_new_tab_ui/storage/new_tab_storage.ts index 86faa22c95a2..c16b8a8d3b70 100644 --- a/components/brave_new_tab_ui/storage/new_tab_storage.ts +++ b/components/brave_new_tab_ui/storage/new_tab_storage.ts @@ -6,6 +6,7 @@ // Utils import { debounce } from '../../common/debounce' import { loadTimeData } from '../../common/loadTimeData' +import { braveSearchHost } from '../components/search/config' export const keyName = 'new-tab-data' @@ -28,7 +29,7 @@ export const defaultState: NewTab.State = { showRewards: false, showBraveTalk: false, showSearchBox: true, - lastUsedNtpSearchEngine: "search.brave.com", + lastUsedNtpSearchEngine: braveSearchHost, promptEnableSearchSuggestions: true, searchSuggestionsEnabled: false, showBitcoinDotCom: false,