Skip to content

Commit

Permalink
Fixing a few remaining items:
Browse files Browse the repository at this point in the history
- Move gridSiteState to sessionStorage (not localStorage) and default value for "use custom links".
  Includes cleanup of previous localStorage value (if user has one).
  sessionStorage seems to be persisted between tabs and should be cleaned
  up when browser is closed.
- X to remove tiles is now right aligned
- Update top sites mode/visible setting when NTP gets updates
- Fixed disabled icon showing when tiles are movable (condition was reversed)
- Title case the new preference (to match existing preference)

And of course, closing keywords for this PR:
Fixes brave/brave-browser#11500
Fixes brave/brave-browser#9788
Fixes brave/brave-browser#9457
Fixes brave/brave-browser#11551
  • Loading branch information
bsclifton committed Sep 24, 2020
1 parent eac8f96 commit 8ed0745
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 22 deletions.
6 changes: 4 additions & 2 deletions components/brave_new_tab_ui/actions/grid_sites_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import { types } from '../constants/grid_sites_types'
import { action } from 'typesafe-actions'

export const tilesUpdated = (gridSites: NewTab.Site[]) => {
return action(types.GRID_SITES_DATA_UPDATED, { gridSites })
export const tilesUpdated = (gridSites: NewTab.Site[],
custom_links_enabled: boolean, visible: boolean) => {
return action(types.GRID_SITES_DATA_UPDATED, { gridSites,
custom_links_enabled, visible })
}

export const showTilesRemovedNotice = (shouldShow: boolean) => {
Expand Down
10 changes: 6 additions & 4 deletions components/brave_new_tab_ui/api/topSites.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// 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 are_custom_links_enabled: boolean
let is_visible: boolean
let are_custom_links_enabled: boolean = true
let is_visible: boolean = true

export function updateMostVisitedInfo () {
chrome.send('updateMostVisitedInfo')
Expand All @@ -31,10 +31,12 @@ export function undoMostVisitedTileAction (): void {
}

export function setMostVisitedSettings (custom_links_enabled: boolean,
visible: boolean): void {
visible: boolean, update_instant_service: boolean): void {
are_custom_links_enabled = custom_links_enabled
is_visible = visible
chrome.send('setMostVisitedSettings', [custom_links_enabled, visible])
if (update_instant_service) {
chrome.send('setMostVisitedSettings', [custom_links_enabled, visible])
}
}

export function customLinksEnabled (): boolean {
Expand Down
3 changes: 2 additions & 1 deletion components/brave_new_tab_ui/apiEventsToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ function onRewardsToggled (prefData: preferencesAPI.Preferences): void {
}

async function onMostVisitedInfoChanged (topSites: any) {
getActions().tilesUpdated(topSites.tiles)
getActions().tilesUpdated(topSites.tiles, topSites.custom_links_enabled,
topSites.visible)
}

// Not marked as async so we don't return a promise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const TileActionsContainer = styled<{}, 'nav'>('nav')`
position: absolute;
z-index: 2;
top: 0;
left: 0;
right: 0;
text-align: center;
display: flex;
Expand Down
7 changes: 1 addition & 6 deletions components/brave_new_tab_ui/containers/newTab/gridTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import {

import {
deleteMostVisitedTile,
generateGridSiteFavicon,
customLinksEnabled
generateGridSiteFavicon
} from '../../api/topSites'

// Types
Expand All @@ -48,10 +47,6 @@ class TopSite extends React.PureComponent<Props, {}> {
<Tile
title={siteData.title}
tabIndex={0}
style={{
// Visually inform users that dragging a site is not allowed.
cursor: customLinksEnabled() ? 'grab' : 'not-allowed'
}}
>
<TileActionsContainer>
<TileAction onClick={this.onIgnoredTopSite.bind(this, siteData)}>
Expand Down
5 changes: 3 additions & 2 deletions components/brave_new_tab_ui/containers/newTab/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import BrandedWallpaperLogo from '../../components/default/brandedWallpaper/logo
import VisibilityTimer from '../../helpers/visibilityTimer'
import { generateQRData } from '../../binance-utils'

// API
import {
customLinksEnabled,
setMostVisitedSettings
Expand Down Expand Up @@ -228,12 +229,12 @@ class NewTabPage extends React.Component<Props, State> {
// Settings page needs to be updated to read that value too.
const new_value = !this.props.newTabData.showTopSites
this.props.saveShowTopSites(new_value)
setMostVisitedSettings(customLinksEnabled(), new_value)
setMostVisitedSettings(customLinksEnabled(), new_value, true)
}

toggleCustomLinksEnabled = () => {
setMostVisitedSettings(!customLinksEnabled(),
this.props.newTabData.showTopSites)
this.props.newTabData.showTopSites, true)
}

toggleShowRewards = () => {
Expand Down
4 changes: 3 additions & 1 deletion components/brave_new_tab_ui/reducers/grid_sites_reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ export const gridSitesReducer: Reducer<NewTab.GridSitesState | undefined> = (

switch (action.type) {
case types.GRID_SITES_DATA_UPDATED: {
state = gridSitesState.tilesUpdated(state, payload.gridSites)
const { gridSites, custom_links_enabled, visible } = payload
setMostVisitedSettings(custom_links_enabled, visible, false)
state = gridSitesState.tilesUpdated(state, gridSites)
break
}

Expand Down
15 changes: 11 additions & 4 deletions components/brave_new_tab_ui/storage/grid_sites_storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

// Utils
import { debounce } from '../../common/debounce'
const keyName = 'grid-sites-data-v1'
const oldkeyName = 'grid-sites-data-v1'
const newkeyName = 'grid-sites-data-v2'

export const initialGridSitesState: NewTab.GridSitesState = {
gridSites: [],
Expand All @@ -14,7 +15,11 @@ export const initialGridSitesState: NewTab.GridSitesState = {
}

export const load = (): NewTab.GridSitesState => {
const data: string | null = window.localStorage.getItem(keyName)
// Cleanup legacy localStorage (not needed anymore)
if (window.localStorage.getItem(oldkeyName)) {
window.localStorage.removeItem(oldkeyName)
}
const data: string | null = window.sessionStorage.getItem(newkeyName)
let state = initialGridSitesState
let storedState: NewTab.GridSitesState

Expand All @@ -33,9 +38,11 @@ export const load = (): NewTab.GridSitesState => {
return state
}

// Saving the state is useful so that something will show when opening
// a new tab. There is a delay before MostVisitedInfoChanged() is called.
// Using `sessionStorage` won't persist to disk.
export const debouncedSave = debounce<NewTab.GridSitesState>((data: NewTab.GridSitesState) => {
if (data) {
window.localStorage.setItem(keyName, JSON.stringify(data))
window.sessionStorage.setItem(newkeyName, JSON.stringify(data))
}
}, 50)

2 changes: 1 addition & 1 deletion components/resources/brave_components_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@
<message name="IDS_BRAVE_NEW_TAB_CLOCK_FORMAT_12" desc="Text for choice '12-hour clock' Clock>Format setting">12-hour clock</message>
<message name="IDS_BRAVE_NEW_TAB_CLOCK_FORMAT_24" desc="Text for choice '24-hour clock' Clock>Format setting">24-hour clock</message>
<message name="IDS_BRAVE_NEW_TAB_SHOW_TOP_SITES" desc="Text for settings show top sites option">Show Top Sites</message>
<message name="IDS_BRAVE_NEW_TAB_CUSTOM_LINKS_ENABLED" desc="Text for allowing customizing of top sites">Customize top sites</message>
<message name="IDS_BRAVE_NEW_TAB_CUSTOM_LINKS_ENABLED" desc="Text for allowing customizing of top sites">Customize Top Sites</message>
<message name="IDS_BRAVE_NEW_TAB_SHOW_REWARDS" desc="Text for settings show rewards option">Show Brave Rewards</message>
<message name="IDS_BRAVE_NEW_TAB_SHOW_BINANCE" desc="Text for show binance option">Show Binance</message>
<message name="IDS_BRAVE_NEW_TAB_SHOW_TOGETHER" desc="Text for show together option">Brave Together</message>
Expand Down

0 comments on commit 8ed0745

Please sign in to comment.