Skip to content

Commit d3ae61b

Browse files
ryanmlryanml
committed
Fixes brave/brave-browser#10067 - Resolving lost stack widget issue
1 parent 04befa8 commit d3ae61b

File tree

5 files changed

+129
-98
lines changed

5 files changed

+129
-98
lines changed

‎components/brave_new_tab_ui/containers/newTab/index.tsx

+13-5
Original file line numberDiff line numberDiff line change
@@ -460,22 +460,30 @@ class NewTabPage extends React.Component<Props, State> {
460460
}
461461

462462
getCryptoContent () {
463-
const { widgetStackOrder, binanceState, togetherSupported, showRewards } = this.props.newTabData
463+
const {
464+
widgetStackOrder,
465+
binanceState,
466+
togetherSupported,
467+
showRewards,
468+
showBinance,
469+
showTogether
470+
} = this.props.newTabData
464471
const lookup = {
465472
'rewards': {
466-
supported: showRewards,
473+
display: showRewards,
467474
render: this.renderRewardsWidget.bind(this)
468475
},
469476
'binance': {
470-
supported: binanceState.binanceSupported,
477+
display: binanceState.binanceSupported && showBinance,
471478
render: this.renderBinanceWidget.bind(this)
472479
},
473480
'together': {
474-
supported: togetherSupported,
481+
display: togetherSupported && showTogether,
475482
render: this.renderTogetherWidget.bind(this)
476483
}
477484
}
478-
const widgetList = widgetStackOrder.filter((widget: NewTab.StackWidget) => lookup[widget].supported)
485+
486+
const widgetList = widgetStackOrder.filter((widget: NewTab.StackWidget) => lookup[widget].display)
479487

480488
return (
481489
<>

‎components/brave_new_tab_ui/reducers/new_tab_reducer.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export const newTabReducer: Reducer<NewTab.State | undefined> = (state: NewTab.S
7070
state = storage.migrateStackWidgetSettings(state)
7171
}
7272
state = storage.addNewStackWidget(state)
73+
state = storage.replaceStackWidgets(state)
7374

7475
break
7576

@@ -140,18 +141,13 @@ export const newTabReducer: Reducer<NewTab.State | undefined> = (state: NewTab.S
140141
break
141142
}
142143

143-
widgetStackOrder = widgetStackOrder.filter((curWidget: NewTab.StackWidget) => {
144-
return curWidget !== widget
145-
})
146-
147144
if (!removedStackWidgets.includes(widget)) {
148145
removedStackWidgets.push(widget)
149146
}
150147

151148
state = {
152149
...state,
153-
removedStackWidgets,
154-
widgetStackOrder
150+
removedStackWidgets
155151
}
156152
break
157153

‎components/brave_new_tab_ui/storage/new_tab_storage.ts

+31
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,37 @@ export const addNewStackWidget = (state: NewTab.State) => {
160160
return state
161161
}
162162

163+
// Replaces any stack widgets that were improperly removed
164+
// as a result of https://github.com/brave/brave-browser/issues/10067
165+
export const replaceStackWidgets = (state: NewTab.State) => {
166+
const {
167+
binanceState,
168+
showBinance,
169+
showRewards,
170+
showTogether,
171+
togetherSupported
172+
} = state
173+
const displayLookup = {
174+
'rewards': {
175+
display: showRewards
176+
},
177+
'binance': {
178+
display: binanceState.binanceSupported && showBinance
179+
},
180+
'together': {
181+
display: togetherSupported && showTogether
182+
}
183+
}
184+
for (const key in displayLookup) {
185+
const widget = key as NewTab.StackWidget
186+
if (!state.widgetStackOrder.includes(widget) &&
187+
displayLookup[widget].display) {
188+
state.widgetStackOrder.unshift(widget)
189+
}
190+
}
191+
return state
192+
}
193+
163194
const cleanData = (state: NewTab.State) => {
164195
// We need to disable linter as we defined in d.ts that this values are number,
165196
// but we need this check to covert from old version to a new one

‎components/test/brave_new_tab_ui/reducers/new_tab_reducer_test.ts

+2-22
Original file line numberDiff line numberDiff line change
@@ -106,29 +106,10 @@ describe('newTabReducer', () => {
106106
expect(assertion).toEqual(expectedState)
107107
})
108108

109-
it('removes widget and updates removed widgets', () => {
109+
it('adds removed widget to removed widgets list', () => {
110110
const assertion = newTabReducer({
111111
...storage.defaultState,
112-
widgetStackOrder: ['binance', 'rewards']
113-
}, {
114-
type: types.REMOVE_STACK_WIDGET,
115-
payload: {
116-
widget: 'rewards'
117-
}
118-
})
119-
const expectedState = {
120-
...storage.defaultState,
121-
widgetStackOrder: ['binance'],
122-
removedStackWidgets: ['rewards']
123-
}
124-
expect(assertion).toEqual(expectedState)
125-
})
126-
127-
it('removes widget and does not re-update removed widgets', () => {
128-
const assertion = newTabReducer({
129-
...storage.defaultState,
130-
widgetStackOrder: ['rewards', 'binance'],
131-
removedStackWidgets: ['rewards']
112+
removedStackWidgets: []
132113
}, {
133114
type: types.REMOVE_STACK_WIDGET,
134115
payload: {
@@ -137,7 +118,6 @@ describe('newTabReducer', () => {
137118
})
138119
const expectedState = {
139120
...storage.defaultState,
140-
widgetStackOrder: ['binance'],
141121
removedStackWidgets: ['rewards']
142122
}
143123
expect(assertion).toEqual(expectedState)

‎components/test/brave_new_tab_ui/storage/new_tab_storage_test.ts

+81-65
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
33
* You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
import { defaultState, migrateStackWidgetSettings } from '../../../brave_new_tab_ui/storage/new_tab_storage'
5+
import { defaultState, migrateStackWidgetSettings, replaceStackWidgets } from '../../../brave_new_tab_ui/storage/new_tab_storage'
66

77
describe('new tab storage', () => {
8-
describe('cleanData', () => {
8+
describe('migrateStackWidgetSettings', () => {
99
it('migrates users who had all stack widgets hidden', () => {
1010
// showRewards and showBinance are false in default state
1111
// currentStackWidget will still be set in the event both
@@ -22,73 +22,89 @@ describe('new tab storage', () => {
2222
}
2323
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
2424
})
25-
})
2625

27-
it('migrates users who were showing rewards and hiding binance', () => {
28-
const initialState = {
29-
...defaultState,
30-
currentStackWidget: 'rewards',
31-
showRewards: true
32-
}
33-
const expectedState = {
34-
...defaultState,
35-
showRewards: true,
36-
currentStackWidget: '',
37-
widgetStackOrder: ['rewards'],
38-
removedStackWidgets: ['binance']
39-
}
40-
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
41-
})
26+
it('migrates users who were showing rewards and hiding binance', () => {
27+
const initialState = {
28+
...defaultState,
29+
currentStackWidget: 'rewards',
30+
showRewards: true
31+
}
32+
const expectedState = {
33+
...defaultState,
34+
showRewards: true,
35+
currentStackWidget: '',
36+
widgetStackOrder: ['rewards'],
37+
removedStackWidgets: ['binance']
38+
}
39+
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
40+
})
4241

43-
it('migrates users who were showing binance and hiding rewards', () => {
44-
const initialState = {
45-
...defaultState,
46-
currentStackWidget: 'binance',
47-
showBinance: true
48-
}
49-
const expectedState = {
50-
...defaultState,
51-
showBinance: true,
52-
currentStackWidget: '',
53-
widgetStackOrder: ['binance'],
54-
removedStackWidgets: ['rewards']
55-
}
56-
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
57-
})
42+
it('migrates users who were showing binance and hiding rewards', () => {
43+
const initialState = {
44+
...defaultState,
45+
currentStackWidget: 'binance',
46+
showBinance: true
47+
}
48+
const expectedState = {
49+
...defaultState,
50+
showBinance: true,
51+
currentStackWidget: '',
52+
widgetStackOrder: ['binance'],
53+
removedStackWidgets: ['rewards']
54+
}
55+
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
56+
})
57+
58+
it('preserves order and migrates users who were showing both widgets (Binance foreground)', () => {
59+
const initialState = {
60+
...defaultState,
61+
currentStackWidget: 'binance',
62+
showBinance: true,
63+
showRewards: true
64+
}
65+
const expectedState = {
66+
...defaultState,
67+
showBinance: true,
68+
showRewards: true,
69+
currentStackWidget: '',
70+
widgetStackOrder: ['rewards', 'binance'],
71+
removedStackWidgets: []
72+
}
73+
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
74+
})
5875

59-
it('preserves order and migrates users who were showing both widgets (Binance foreground)', () => {
60-
const initialState = {
61-
...defaultState,
62-
currentStackWidget: 'binance',
63-
showBinance: true,
64-
showRewards: true
65-
}
66-
const expectedState = {
67-
...defaultState,
68-
showBinance: true,
69-
showRewards: true,
70-
currentStackWidget: '',
71-
widgetStackOrder: ['rewards', 'binance'],
72-
removedStackWidgets: []
73-
}
74-
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
76+
it('preserves order and migrates users who were showing both widgets (Rewards foreground)', () => {
77+
const initialState = {
78+
...defaultState,
79+
currentStackWidget: 'rewards',
80+
showBinance: true,
81+
showRewards: true
82+
}
83+
const expectedState = {
84+
...defaultState,
85+
showBinance: true,
86+
showRewards: true,
87+
currentStackWidget: '',
88+
widgetStackOrder: ['binance', 'rewards'],
89+
removedStackWidgets: []
90+
}
91+
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
92+
})
7593
})
7694

77-
it('preserves order and migrates users who were showing both widgets (Rewards foreground)', () => {
78-
const initialState = {
79-
...defaultState,
80-
currentStackWidget: 'rewards',
81-
showBinance: true,
82-
showRewards: true
83-
}
84-
const expectedState = {
85-
...defaultState,
86-
showBinance: true,
87-
showRewards: true,
88-
currentStackWidget: '',
89-
widgetStackOrder: ['binance', 'rewards'],
90-
removedStackWidgets: []
91-
}
92-
expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
95+
describe('replaceStackWidgets', () => {
96+
it('adds back widgets that should be showing in the stack', () => {
97+
const initialState = {
98+
...defaultState,
99+
showBinance: true,
100+
showRewards: true,
101+
widgetStackOrder: ['binance']
102+
}
103+
const expectedState = {
104+
...initialState,
105+
widgetStackOrder: ['rewards', 'binance']
106+
}
107+
expect(replaceStackWidgets(initialState)).toEqual(expectedState)
108+
})
93109
})
94110
})

0 commit comments

Comments
 (0)