Resolving lost stack widget issue

components/brave_new_tab_ui/containers/newTab/index.tsx

@@ -460,22 +460,30 @@ class NewTabPage extends React.Component<Props, State> {
   }
 
   getCryptoContent () {
-    const { widgetStackOrder, binanceState, togetherSupported, showRewards } = this.props.newTabData
+    const {
+      widgetStackOrder,
+      binanceState,
+      togetherSupported,
+      showRewards,
+      showBinance,
+      showTogether
+    } = this.props.newTabData
     const lookup = {
       'rewards': {
-        supported: showRewards,
+        display: showRewards,
         render: this.renderRewardsWidget.bind(this)
       },
       'binance': {
-        supported: binanceState.binanceSupported,
+        display: binanceState.binanceSupported && showBinance,
         render: this.renderBinanceWidget.bind(this)
       },
       'together': {
-        supported: togetherSupported,
+        display: togetherSupported && showTogether,
         render: this.renderTogetherWidget.bind(this)
       }
     }
-    const widgetList = widgetStackOrder.filter((widget: NewTab.StackWidget) => lookup[widget].supported)
+
+    const widgetList = widgetStackOrder.filter((widget: NewTab.StackWidget) => lookup[widget].display)
 
     return (
       <>

components/brave_new_tab_ui/reducers/new_tab_reducer.ts

@@ -70,6 +70,7 @@ export const newTabReducer: Reducer<NewTab.State | undefined> = (state: NewTab.S
         state = storage.migrateStackWidgetSettings(state)
       }
       state = storage.addNewStackWidget(state)
+      state = storage.replaceStackWidgets(state)
 
       break
 
@@ -140,18 +141,13 @@ export const newTabReducer: Reducer<NewTab.State | undefined> = (state: NewTab.S
         break
       }
 
-      widgetStackOrder = widgetStackOrder.filter((curWidget: NewTab.StackWidget) => {
-        return curWidget !== widget
-      })
-
       if (!removedStackWidgets.includes(widget)) {
         removedStackWidgets.push(widget)
       }
 
       state = {
         ...state,
-        removedStackWidgets,
-        widgetStackOrder
+        removedStackWidgets
       }
       break
 

components/brave_new_tab_ui/storage/new_tab_storage.ts

@@ -160,6 +160,37 @@ export const addNewStackWidget = (state: NewTab.State) => {
   return state
 }
 
+// Replaces any stack widgets that were improperly removed
+// as a result of https://github.com/brave/brave-browser/issues/10067
+export const replaceStackWidgets = (state: NewTab.State) => {
+  const {
+    binanceState,
+    showBinance,
+    showRewards,
+    showTogether,
+    togetherSupported
+  } = state
+  const displayLookup = {
+    'rewards': {
+      display: showRewards
+    },
+    'binance': {
+      display: binanceState.binanceSupported && showBinance
+    },
+    'together': {
+      display: togetherSupported && showTogether
+    }
+  }
+  for (const key in displayLookup) {
+    const widget = key as NewTab.StackWidget
+    if (!state.widgetStackOrder.includes(widget) &&
+        displayLookup[widget].display) {
+      state.widgetStackOrder.unshift(widget)
+    }
+  }
+  return state
+}
+
 const cleanData = (state: NewTab.State) => {
   // We need to disable linter as we defined in d.ts that this values are number,
   // 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

@@ -106,29 +106,10 @@ describe('newTabReducer', () => {
         expect(assertion).toEqual(expectedState)
       })
 
-      it('removes widget and updates removed widgets', () => {
+      it('adds removed widget to removed widgets list', () => {
         const assertion = newTabReducer({
           ...storage.defaultState,
-          widgetStackOrder: ['binance', 'rewards']
-        }, {
-          type: types.REMOVE_STACK_WIDGET,
-          payload: {
-            widget: 'rewards'
-          }
-        })
-        const expectedState = {
-          ...storage.defaultState,
-          widgetStackOrder: ['binance'],
-          removedStackWidgets: ['rewards']
-        }
-        expect(assertion).toEqual(expectedState)
-      })
-
-      it('removes widget and does not re-update removed widgets', () => {
-        const assertion = newTabReducer({
-          ...storage.defaultState,
-          widgetStackOrder: ['rewards', 'binance'],
-          removedStackWidgets: ['rewards']
+          removedStackWidgets: []
         }, {
           type: types.REMOVE_STACK_WIDGET,
           payload: {
@@ -137,7 +118,6 @@ describe('newTabReducer', () => {
         })
         const expectedState = {
           ...storage.defaultState,
-          widgetStackOrder: ['binance'],
           removedStackWidgets: ['rewards']
         }
         expect(assertion).toEqual(expectedState)

components/test/brave_new_tab_ui/storage/new_tab_storage_test.ts

@@ -2,10 +2,10 @@
  * 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/. */
 
-import { defaultState, migrateStackWidgetSettings } from '../../../brave_new_tab_ui/storage/new_tab_storage'
+import { defaultState, migrateStackWidgetSettings, replaceStackWidgets } from '../../../brave_new_tab_ui/storage/new_tab_storage'
 
 describe('new tab storage', () => {
-  describe('cleanData', () => {
+  describe('migrateStackWidgetSettings', () => {
     it('migrates users who had all stack widgets hidden', () => {
       // showRewards and showBinance are false in default state
       // currentStackWidget will still be set in the event both
@@ -22,73 +22,89 @@ describe('new tab storage', () => {
       }
       expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
     })
-  })
 
-  it('migrates users who were showing rewards and hiding binance', () => {
-    const initialState = {
-      ...defaultState,
-      currentStackWidget: 'rewards',
-      showRewards: true
-    }
-    const expectedState = {
-      ...defaultState,
-      showRewards: true,
-      currentStackWidget: '',
-      widgetStackOrder: ['rewards'],
-      removedStackWidgets: ['binance']
-    }
-    expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
-  })
+    it('migrates users who were showing rewards and hiding binance', () => {
+      const initialState = {
+        ...defaultState,
+        currentStackWidget: 'rewards',
+        showRewards: true
+      }
+      const expectedState = {
+        ...defaultState,
+        showRewards: true,
+        currentStackWidget: '',
+        widgetStackOrder: ['rewards'],
+        removedStackWidgets: ['binance']
+      }
+      expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
+    })
 
-  it('migrates users who were showing binance and hiding rewards', () => {
-    const initialState = {
-      ...defaultState,
-      currentStackWidget: 'binance',
-      showBinance: true
-    }
-    const expectedState = {
-      ...defaultState,
-      showBinance: true,
-      currentStackWidget: '',
-      widgetStackOrder: ['binance'],
-      removedStackWidgets: ['rewards']
-    }
-    expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
-  })
+    it('migrates users who were showing binance and hiding rewards', () => {
+      const initialState = {
+        ...defaultState,
+        currentStackWidget: 'binance',
+        showBinance: true
+      }
+      const expectedState = {
+        ...defaultState,
+        showBinance: true,
+        currentStackWidget: '',
+        widgetStackOrder: ['binance'],
+        removedStackWidgets: ['rewards']
+      }
+      expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
+    })
+
+    it('preserves order and migrates users who were showing both widgets (Binance foreground)', () => {
+      const initialState = {
+        ...defaultState,
+        currentStackWidget: 'binance',
+        showBinance: true,
+        showRewards: true
+      }
+      const expectedState = {
+        ...defaultState,
+        showBinance: true,
+        showRewards: true,
+        currentStackWidget: '',
+        widgetStackOrder: ['rewards', 'binance'],
+        removedStackWidgets: []
+      }
+      expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
+    })
 
-  it('preserves order and migrates users who were showing both widgets (Binance foreground)', () => {
-    const initialState = {
-      ...defaultState,
-      currentStackWidget: 'binance',
-      showBinance: true,
-      showRewards: true
-    }
-    const expectedState = {
-      ...defaultState,
-      showBinance: true,
-      showRewards: true,
-      currentStackWidget: '',
-      widgetStackOrder: ['rewards', 'binance'],
-      removedStackWidgets: []
-    }
-    expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
+    it('preserves order and migrates users who were showing both widgets (Rewards foreground)', () => {
+      const initialState = {
+        ...defaultState,
+        currentStackWidget: 'rewards',
+        showBinance: true,
+        showRewards: true
+      }
+      const expectedState = {
+        ...defaultState,
+        showBinance: true,
+        showRewards: true,
+        currentStackWidget: '',
+        widgetStackOrder: ['binance', 'rewards'],
+        removedStackWidgets: []
+      }
+      expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
+    })
   })
 
-  it('preserves order and migrates users who were showing both widgets (Rewards foreground)', () => {
-    const initialState = {
-      ...defaultState,
-      currentStackWidget: 'rewards',
-      showBinance: true,
-      showRewards: true
-    }
-    const expectedState = {
-      ...defaultState,
-      showBinance: true,
-      showRewards: true,
-      currentStackWidget: '',
-      widgetStackOrder: ['binance', 'rewards'],
-      removedStackWidgets: []
-    }
-    expect(migrateStackWidgetSettings(initialState)).toEqual(expectedState)
+  describe('replaceStackWidgets', () => {
+    it('adds back widgets that should be showing in the stack', () => {
+      const initialState = {
+        ...defaultState,
+        showBinance: true,
+        showRewards: true,
+        widgetStackOrder: ['binance']
+      }
+      const expectedState = {
+        ...initialState,
+        widgetStackOrder: ['rewards', 'binance']
+      }
+      expect(replaceStackWidgets(initialState)).toEqual(expectedState)
+    })
   })
 })