Address review feedback and lint/gn check fixes

Can't include //chrome/browser:browser (where profile is) because this
causes a circular dep. Instead, I include profile_utils which has the
include for profile.
This commit is contained in:
Brian Clifton
2020-10-06 01:34:35 -07:00
parent dc89fdadb6
commit 4c366ad3f5
20 changed files with 98 additions and 80 deletions
+3 -2
View File
@@ -10,10 +10,11 @@ source_set("search") {
]
deps += [
"//brave/common",
"//brave/browser/profiles:profiles",
"//brave/common:pref_names",
"//chrome/common",
"//components/prefs",
"//components/pref_registry",
"//components/prefs",
]
}
}
+4 -3
View File
@@ -5,8 +5,8 @@
#include "brave/browser/search/ntp_utils.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/common/pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_registry_simple.h"
@@ -25,9 +25,10 @@ namespace new_tab_page {
void MigrateNewTabPagePrefs(Profile* profile) {
// Migrate over to the Chromium setting for shortcuts visible
// Only sets the value if user has changed it
const PrefService::Preference* pref =
profile->GetPrefs()->FindPreference(prefs::kNtpShortcutsVisible);
if (!pref->HasUserSetting()) {
profile->GetPrefs()->FindPreference(kNewTabPageShowTopSites);
if (pref->HasUserSetting()) {
profile->GetPrefs()->SetBoolean(prefs::kNtpShortcutsVisible,
profile->GetPrefs()->GetBoolean(kNewTabPageShowTopSites));
}
+1 -1
View File
@@ -21,4 +21,4 @@ void RegisterNewTabPagePrefsForMigration(
} // namespace new_tab_page
#endif // BRAVE_BROWSER_SEARCH_NTP_UTILS_H_
#endif // BRAVE_BROWSER_SEARCH_NTP_UTILS_H_
@@ -1,4 +1,4 @@
// Copyright (c) 2019 The Brave Authors. All rights reserved.
// Copyright (c) 2020 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/.
@@ -1,4 +1,4 @@
// Copyright (c) 2019 The Brave Authors. All rights reserved.
// Copyright (c) 2020 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/.
@@ -233,7 +233,8 @@ void BraveAppearanceHandler::OnPreferenceChanged(const std::string& pref_name) {
}
}
void BraveAppearanceHandler::ToggleTopSitesVisible(const base::ListValue* args) {
void BraveAppearanceHandler::ToggleTopSitesVisible(
const base::ListValue* args) {
AllowJavascript();
InstantService* instant_service =
InstantServiceFactory::GetForProfile(profile_);
+6 -4
View File
@@ -28,9 +28,6 @@ const char kUseAlternativeSearchEngineProvider[] =
"brave.use_alternate_private_search_engine";
const char kAlternativeSearchEngineProviderInTor[] =
"brave.alternate_private_search_engine_in_tor";
const char kBraveThemeType[] = "brave.theme.type"; // deprecated
const char kUseOverriddenBraveThemeType[] =
"brave.theme.use_overridden_brave_theme_type"; // deprecated
const char kLocationBarIsWide[] = "brave.location_bar_is_wide";
const char kReferralDownloadID[] = "brave.referral.download_id";
const char kReferralTimestamp[] = "brave.referral.timestamp";
@@ -56,7 +53,6 @@ const char kHangoutsEnabled[] = "brave.hangouts_enabled";
const char kIPFSCompanionEnabled[] = "brave.ipfs_companion_enabled";
const char kNewTabPageShowClock[] = "brave.new_tab_page.show_clock";
const char kNewTabPageClockFormat[] = "brave.new_tab_page.clock_format";
const char kNewTabPageShowTopSites[] = "brave.new_tab_page.show_top_sites"; // deprecated
const char kNewTabPageShowStats[] = "brave.new_tab_page.show_stats";
const char kNewTabPageShowRewards[] = "brave.new_tab_page.show_rewards";
const char kNewTabPageShowBinance[] = "brave.new_tab_page.show_binance";
@@ -90,3 +86,9 @@ const char kBackgroundVideoPlaybackEnabled[] =
const char kSafetynetCheckFailed[] = "safetynetcheck.failed";
const char kSafetynetStatus[] = "safetynet.status";
#endif
// deprecated
const char kBraveThemeType[] = "brave.theme.type";
const char kUseOverriddenBraveThemeType[] =
"brave.theme.use_overridden_brave_theme_type";
const char kNewTabPageShowTopSites[] = "brave.new_tab_page.show_top_sites";
@@ -7,29 +7,27 @@
import { types } from '../constants/grid_sites_types'
import { action } from 'typesafe-actions'
export const tilesUpdated = (gridSites: NewTab.Site[],
customLinksEnabled: boolean, visible: boolean) => {
return action(types.GRID_SITES_DATA_UPDATED, { gridSites,
customLinksEnabled, visible })
export const tilesUpdated = (gridSites: NewTab.Site[]) => {
return action(types.TILES_UPDATED, { gridSites })
}
export const tileRemoved = (url: string) => {
return action(types.GRID_SITES_REMOVE, { url })
return action(types.TILE_REMOVED, { url })
}
export const tilesReordered = (gridSites: NewTab.Site[],
oldPos: number, newPos: number) => {
return action(types.GRID_SITES_REORDER, { gridSites, oldPos, newPos })
return action(types.TILES_REORDERED, { gridSites, oldPos, newPos })
}
export const restoreDefaultTiles = () => {
return action(types.GRID_SITES_RESTORE_DEFAULTS, {})
return action(types.RESTORE_DEFAULT_TILES, {})
}
export const showTilesRemovedNotice = (shouldShow: boolean) => {
return action(types.GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION, { shouldShow })
return action(types.SHOW_TILES_REMOVED_NOTICE, { shouldShow })
}
export const undoRemoveTile = () => {
return action(types.GRID_SITES_UNDO_ACTION, {})
return action(types.UNDO_REMOVE_TILE, {})
}
@@ -46,3 +46,9 @@ export const removeStackWidget = (widget: NewTab.StackWidget) => action(types.RE
export const setForegroundStackWidget = (widget: NewTab.StackWidget) => action(types.SET_FOREGROUND_STACK_WIDGET, {
widget
})
export const setMostVisitedSettings = (showTopSites: boolean, customLinksEnabled: boolean) =>
action(types.SET_MOST_VISITED_SITES, { showTopSites, customLinksEnabled })
export const topSitesStateUpdated = (newShowTopSites: boolean, newCustomLinksEnabled: boolean) =>
action(types.TOP_SITES_STATE_UPDATED, { newShowTopSites, newCustomLinksEnabled })
+10 -17
View File
@@ -3,14 +3,19 @@
// 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/.
let areCustomLinksEnabled: boolean = true
let areTopSitesVisible: boolean = true
export type MostVisitedInfoChanged = {
tiles: NewTab.Site[]
custom_links_enabled: boolean
visible: boolean
}
export type MostVisitedInfoChangedHandler = (data: MostVisitedInfoChanged) => any
export function updateMostVisitedInfo () {
chrome.send('updateMostVisitedInfo')
}
export function addMostVistedInfoChangedListener (listener: any): void {
export function addMostVistedInfoChangedListener (listener: MostVisitedInfoChangedHandler): void {
window.cr.addWebUIListener('most-visited-info-changed', listener)
}
@@ -31,18 +36,6 @@ export function undoMostVisitedTileAction (): void {
}
export function setMostVisitedSettings (customLinksEnabled: boolean,
visible: boolean, updateInstantService: boolean): void {
areCustomLinksEnabled = customLinksEnabled
areTopSitesVisible = visible
if (updateInstantService) {
chrome.send('setMostVisitedSettings', [customLinksEnabled, visible])
}
}
export function customLinksEnabled (): boolean {
return areCustomLinksEnabled
}
export function isVisible (): boolean {
return areTopSitesVisible
visible: boolean): void {
chrome.send('setMostVisitedSettings', [customLinksEnabled, visible])
}
@@ -33,9 +33,9 @@ function onRewardsToggled (prefData: preferencesAPI.Preferences): void {
}
}
async function onMostVisitedInfoChanged (topSites: any) {
getActions().tilesUpdated(topSites.tiles, topSites.custom_links_enabled,
topSites.visible)
async function onMostVisitedInfoChanged (topSites: topSitesAPI.MostVisitedInfoChanged) {
getActions().tilesUpdated(topSites.tiles)
getActions().topSitesStateUpdated(topSites.visible, topSites.custom_links_enabled)
}
// Not marked as async so we don't return a promise
@@ -50,8 +50,8 @@ export function wireApiEventsToStore () {
binanceInitData()
getActions().setInitialData(initialData)
// Listen for API changes and dispatch to store
topSitesAPI.updateMostVisitedInfo()
topSitesAPI.addMostVistedInfoChangedListener(onMostVisitedInfoChanged)
topSitesAPI.updateMostVisitedInfo()
statsAPI.addChangeListener(updateStats)
preferencesAPI.addChangeListener(updatePreferences)
preferencesAPI.addChangeListener(onRewardsToggled)
@@ -4,11 +4,11 @@
// you can obtain one at http://mozilla.org/MPL/2.0/.
export const enum types {
GRID_SITES_DATA_UPDATED = '@@topSites/GRID_SITES_DATA_UPDATED',
GRID_SITES_REMOVE = '@@topSites/GRID_SITES_REMOVE',
GRID_SITES_REORDER = '@@topSites/GRID_SITES_REORDER',
GRID_SITES_RESTORE_DEFAULTS = '@@topSites/GRID_SITES_RESTORE_DEFAULTS',
GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION =
'@@topSites/GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION',
GRID_SITES_UNDO_ACTION = '@@topSites/GRID_SITES_UNDO_ACTION'
TILES_UPDATED = '@@topSites/TILES_UPDATED',
TILE_REMOVED = '@@topSites/TILE_REMOVED',
TILES_REORDERED = '@@topSites/TILES_REORDERED',
RESTORE_DEFAULT_TILES = '@@topSites/RESTORE_DEFAULT_TILES',
SHOW_TILES_REMOVED_NOTICE =
'@@topSites/SHOW_TILES_REMOVED_NOTICE',
UNDO_REMOVE_TILE = '@@topSites/UNDO_REMOVE_TILE'
}
@@ -18,7 +18,9 @@ export const enum types {
NEW_TAB_SET_INITIAL_DATA = '@@newtab/NEW_TAB_SET_INITIAL_DATA',
REMOVE_STACK_WIDGET = '@@newtab/REMOVE_STACK_WIDGET',
SET_FOREGROUND_STACK_WIDGET = '@@newtab/SET_FOREGROUND_STACK_WIDGET',
UPDATE_CLOCK_WIDGET = '@@newtab/UPDATE_CLOCK_WIDGET'
UPDATE_CLOCK_WIDGET = '@@newtab/UPDATE_CLOCK_WIDGET',
SET_MOST_VISITED_SITES = '@@newtab/SET_MOST_VISITED_SITES',
TOP_SITES_STATE_UPDATED = '@@newtab/TOP_SITES_STATE_UPDATED'
}
export type DismissBrandedWallpaperNotificationPayload = {
@@ -26,13 +26,6 @@ import BrandedWallpaperLogo from '../../components/default/brandedWallpaper/logo
import VisibilityTimer from '../../helpers/visibilityTimer'
import { generateQRData } from '../../binance-utils'
// API
import {
customLinksEnabled,
isVisible,
setMostVisitedSettings
} from '../../api/topSites'
// Types
import { getLocale } from '../../../common/locale'
import currencyData from '../../components/default/binance/data'
@@ -227,11 +220,13 @@ class NewTabPage extends React.Component<Props, State> {
}
toggleShowTopSites = () => {
setMostVisitedSettings(customLinksEnabled(), !isVisible(), true)
const { showTopSites, customLinksEnabled } = this.props.newTabData
this.props.actions.setMostVisitedSettings(!showTopSites, customLinksEnabled)
}
toggleCustomLinksEnabled = () => {
setMostVisitedSettings(!customLinksEnabled(), isVisible(), true)
const { showTopSites, customLinksEnabled } = this.props.newTabData
this.props.actions.setMostVisitedSettings(showTopSites, !customLinksEnabled)
}
toggleShowRewards = () => {
@@ -955,7 +950,7 @@ class NewTabPage extends React.Component<Props, State> {
const hasImage = this.imageSource !== undefined
const isShowingBrandedWallpaper = newTabData.brandedWallpaperData ? true : false
const showTopSites = !!this.props.gridSitesData.gridSites.length && isVisible()
const showTopSites = !!this.props.gridSitesData.gridSites.length && newTabData.showTopSites
const cryptoContent = this.renderCryptoContent()
return (
@@ -1015,7 +1010,7 @@ class NewTabPage extends React.Component<Props, State> {
<TopSitesGrid
actions={actions}
paddingType={'right'}
customLinksEnabled={customLinksEnabled()}
customLinksEnabled={newTabData.customLinksEnabled}
widgetTitle={getLocale('topSitesTitle')}
gridSites={gridSitesData.gridSites}
menuPosition={'right'}
@@ -1070,8 +1065,8 @@ class NewTabPage extends React.Component<Props, State> {
showClock={newTabData.showClock}
clockFormat={newTabData.clockFormat}
showStats={newTabData.showStats}
showTopSites={isVisible()}
customLinksEnabled={customLinksEnabled()}
showTopSites={newTabData.showTopSites}
customLinksEnabled={newTabData.customLinksEnabled}
showRewards={newTabData.showRewards}
showBinance={newTabData.showBinance}
brandedWallpaperOptIn={newTabData.brandedWallpaperOptIn}
@@ -16,7 +16,6 @@ import {
deleteMostVisitedTile,
reorderMostVisitedTile,
restoreMostVisitedDefaults,
setMostVisitedSettings,
undoMostVisitedTileAction
} from '../api/topSites'
@@ -37,21 +36,20 @@ export const gridSitesReducer: Reducer<NewTab.GridSitesState | undefined> = (
const startingState = state
switch (action.type) {
case types.GRID_SITES_DATA_UPDATED: {
const { gridSites, customLinksEnabled, visible } = payload
setMostVisitedSettings(customLinksEnabled, visible, false)
case types.TILES_UPDATED: {
const { gridSites } = payload
state = gridSitesState.tilesUpdated(state, gridSites)
break
}
case types.GRID_SITES_REMOVE: {
case types.TILE_REMOVED: {
const { url } = payload
deleteMostVisitedTile(url)
state = gridSitesState.showTilesRemovedNotice(state, true)
break
}
case types.GRID_SITES_REORDER: {
case types.TILES_REORDERED: {
const { gridSites, oldPos, newPos } = payload
// "Super referral" entries (if present) are always at the beginning
// Skip these indices when determining the new position for Chromium
@@ -74,18 +72,18 @@ export const gridSitesReducer: Reducer<NewTab.GridSitesState | undefined> = (
break
}
case types.GRID_SITES_RESTORE_DEFAULTS: {
case types.RESTORE_DEFAULT_TILES: {
restoreMostVisitedDefaults()
state = gridSitesState.showTilesRemovedNotice(state, false)
break
}
case types.GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION: {
case types.SHOW_TILES_REMOVED_NOTICE: {
state = gridSitesState.showTilesRemovedNotice(state, payload.shouldShow)
break
}
case types.GRID_SITES_UNDO_ACTION: {
case types.UNDO_REMOVE_TILE: {
undoMostVisitedTileAction()
state = gridSitesState.showTilesRemovedNotice(state, false)
}
@@ -17,6 +17,7 @@ import { InitialData } from '../api/initialData'
import { registerViewCount } from '../api/brandedWallpaper'
import * as preferencesAPI from '../api/preferences'
import * as storage from '../storage/new_tab_storage'
import { setMostVisitedSettings } from '../api/topSites'
let sideEffectState: NewTab.State = storage.load()
@@ -189,6 +190,24 @@ export const newTabReducer: Reducer<NewTab.State | undefined> = (state: NewTab.S
break
}
case types.SET_MOST_VISITED_SITES: {
const { showTopSites, customLinksEnabled } = payload
performSideEffect(async function (state) {
setMostVisitedSettings(customLinksEnabled, showTopSites)
})
break
}
case types.TOP_SITES_STATE_UPDATED: {
const { newShowTopSites, newCustomLinksEnabled } = payload
state = {
...state,
showTopSites: newShowTopSites,
customLinksEnabled: newCustomLinksEnabled
}
break
}
default:
break
}
@@ -17,6 +17,7 @@ export const defaultState: NewTab.State = {
showClock: false,
clockFormat: '',
showTopSites: false,
customLinksEnabled: false,
showRewards: false,
showTogether: false,
showBinance: false,
+1
View File
@@ -104,6 +104,7 @@ declare namespace NewTab {
showClock: boolean
clockFormat: string
showTopSites: boolean
customLinksEnabled: boolean
showRewards: boolean
showTogether: boolean
showBinance: boolean
@@ -21,7 +21,7 @@ describe('new tab data api tests', () => {
it('can call an action from getActions', () => {
expect(getActions().showTilesRemovedNotice(true)).toEqual({
payload: { shouldShow: true },
type: topSitesTypes.GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION
type: topSitesTypes.SHOW_TILES_REMOVED_NOTICE
})
})
})
@@ -41,7 +41,7 @@ describe('gridSitesReducer', () => {
})
})
describe('GRID_SITES_DATA_UPDATED', () => {
describe('TILES_UPDATED', () => {
let tilesUpdatedStub: jest.SpyInstance
beforeEach(() => {
@@ -54,7 +54,7 @@ describe('gridSitesReducer', () => {
it('calls tilesUpdated with the correct args', () => {
gridSitesReducer(undefined, {
type: types.GRID_SITES_DATA_UPDATED,
type: types.TILES_UPDATED,
payload: { gridSites }
})
@@ -64,14 +64,14 @@ describe('gridSitesReducer', () => {
})
it('update state.gridSites list', () => {
const assertion = gridSitesReducer(storage.initialGridSitesState, {
type: types.GRID_SITES_DATA_UPDATED,
type: types.TILES_UPDATED,
payload: { gridSites }
})
expect(assertion.gridSites).toHaveLength(2)
})
})
describe('GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION', () => {
describe('SHOW_TILES_REMOVED_NOTICE', () => {
let showTilesRemovedNoticeStub: jest.SpyInstance
beforeEach(() => {
@@ -85,7 +85,7 @@ describe('gridSitesReducer', () => {
it('calls showTilesRemovedNotice with the correct args', () => {
const shouldShow: boolean = true
gridSitesReducer(undefined, {
type: types.GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION,
type: types.SHOW_TILES_REMOVED_NOTICE,
payload: { shouldShow }
})
@@ -95,7 +95,7 @@ describe('gridSitesReducer', () => {
})
it('update state with the specified payload value', () => {
const assertion = gridSitesReducer(storage.initialGridSitesState, {
type: types.GRID_SITES_SHOW_SITE_REMOVED_NOTIFICATION,
type: types.SHOW_TILES_REMOVED_NOTICE,
payload: {
shouldShow: true
}