From 91dde786b2864f3462d623b40a691314b3076647 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Fri, 14 Jun 2019 13:57:49 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=8C=20Address=20comments=20and=20clean?= =?UTF-8?q?=20up=20code:=20Closes=20brave/brave-browser#1548?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../actions/welcome_actions.ts | 23 +++----------- .../components/screens/searchBox.tsx | 31 ++++++++++++------- .../reducers/welcome_reducer.ts | 4 --- components/brave_welcome_ui/welcomeUtils.ts | 18 +++++++++++ components/definitions/global.d.ts | 1 + .../resources/brave_components_strings.grd | 2 +- .../actions/welcome_actions_test.ts | 15 --------- .../components/searchBox_test.tsx | 11 +++---- .../reducers/welcome_reducer_test.ts | 29 +++++++++-------- components/test/testData.ts | 2 +- 10 files changed, 64 insertions(+), 72 deletions(-) create mode 100644 components/brave_welcome_ui/welcomeUtils.ts diff --git a/components/brave_welcome_ui/actions/welcome_actions.ts b/components/brave_welcome_ui/actions/welcome_actions.ts index bdf5013cd58..7ca4bb21876 100644 --- a/components/brave_welcome_ui/actions/welcome_actions.ts +++ b/components/brave_welcome_ui/actions/welcome_actions.ts @@ -3,11 +3,13 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ import { action } from 'typesafe-actions' -import { Dispatch } from 'redux' // Constants import { types } from '../constants/welcome_types' +// APIs +import * as welcomeUtils from '../welcomeUtils' + export const importNowRequested = () => action(types.IMPORT_NOW_REQUESTED) export const goToTabRequested = (url: string, target: string) => action(types.GO_TO_TAB_REQUESTED, { @@ -19,23 +21,6 @@ export const closeTabRequested = () => action(types.CLOSE_TAB_REQUESTED) export const changeDefaultSearchProvider = (searchProvider: string) => action(types.CHANGE_DEFAULT_SEARCH_PROVIDER, searchProvider) -export const getSearchEngineProvidersStarted = () => action(types.IMPORT_DEFAULT_SEARCH_PROVIDERS_STARTED) - export const getSearchEngineProvidersSuccess = (searchProviders: Array) => action(types.IMPORT_DEFAULT_SEARCH_PROVIDERS_SUCCESS, searchProviders) -export const getSearchEngineProvidersFailure = () => action(types.IMPORT_DEFAULT_SEARCH_PROVIDERS_FAILURE) - -export const getSearchEngineProviders = () => { - return (dispatch: Dispatch) => { - dispatch(getSearchEngineProvidersStarted()) - - // @ts-ignore - window.cr.sendWithPromise('getSearchEnginesList') - .then((response: Welcome.SearchEngineListResponse) => { - dispatch(getSearchEngineProvidersSuccess(response.defaults)) - }) - .catch(() => { - dispatch(getSearchEngineProvidersFailure()) - }) - } -} +export const getSearchEngineProviders = () => welcomeUtils.getSearchEngineProviders() diff --git a/components/brave_welcome_ui/components/screens/searchBox.tsx b/components/brave_welcome_ui/components/screens/searchBox.tsx index d37db43456b..e06c85cad25 100644 --- a/components/brave_welcome_ui/components/screens/searchBox.tsx +++ b/components/brave_welcome_ui/components/screens/searchBox.tsx @@ -46,8 +46,10 @@ export default class SearchEngineBox extends React.PureComponent { this.setState({ searchEngineSelected: true }) } - getDefaultSearchProvider = (searchEngineEntries: Array): Welcome.SearchEngineEntry | undefined => { - return searchEngineEntries.find(entry => entry.default) + getDefaultSearchProvider = (searchEngineEntries: Array): Welcome.SearchEngineEntry => { + const defaultSearchProvider = searchEngineEntries + .filter((searchEngine: Welcome.SearchEngineEntry) => searchEngine.default) + return defaultSearchProvider[0] } getProviderDisplayName = (searchProvider: Welcome.SearchEngineEntry, defaultSearchProvider: Welcome.SearchEngineEntry): string => @@ -64,7 +66,10 @@ export default class SearchEngineBox extends React.PureComponent { const { index, currentScreen, onClick, searchProviders } = this.props const { searchEngineSelected } = this.state const defaultProvider = this.getDefaultSearchProvider(searchProviders) - const bodyText = this.getBodyText(defaultProvider!.name) + const bodyText = defaultProvider + ? this.getBodyText(defaultProvider.name) || '' + : '' + return ( { onChange={this.onChangeDefaultSearchEngine} > - {searchProviders.map((provider, index) => - - )} + { + (searchProviders && Array.isArray(searchProviders) && searchProviders.length > 0) + ? searchProviders.map((provider, index) => + + ) + : null + } = (state: Welcome.State const modelIndex = parseInt(payload, 10) chrome.send('setDefaultSearchEngine', [modelIndex]) break - case types.IMPORT_DEFAULT_SEARCH_PROVIDERS_STARTED: - break case types.IMPORT_DEFAULT_SEARCH_PROVIDERS_SUCCESS: state = { ...state, searchProviders: payload } break - case types.IMPORT_DEFAULT_SEARCH_PROVIDERS_FAILURE: - break } if (state !== startingState) { diff --git a/components/brave_welcome_ui/welcomeUtils.ts b/components/brave_welcome_ui/welcomeUtils.ts new file mode 100644 index 00000000000..9d2dc956a8d --- /dev/null +++ b/components/brave_welcome_ui/welcomeUtils.ts @@ -0,0 +1,18 @@ +/* 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/. */ + +import { Dispatch } from 'redux' +import { getSearchEngineProvidersSuccess } from './actions/welcome_actions' + +export const getSearchEngineProviders = () => { + return (dispatch: Dispatch) => { + window.cr.sendWithPromise('getSearchEnginesList') + .then((response: Welcome.SearchEngineListResponse) => { + dispatch(getSearchEngineProvidersSuccess(response.defaults)) + }) + .catch((error: any) => { + console.error('Could not load search providers', error) + }) + } +} diff --git a/components/definitions/global.d.ts b/components/definitions/global.d.ts index 3e556f5bc68..273946ec8db 100644 --- a/components/definitions/global.d.ts +++ b/components/definitions/global.d.ts @@ -13,6 +13,7 @@ declare global { loadTimeData: loadTimeData cr: { define: (name: string, init: () => void) => void + sendWithPromise: (method: string, ...args: any[]) => any } i18nTemplate: { process: (document: Document, translations: loadTimeData) => void diff --git a/components/resources/brave_components_strings.grd b/components/resources/brave_components_strings.grd index 5b738cf0d00..42339c34328 100644 --- a/components/resources/brave_components_strings.grd +++ b/components/resources/brave_components_strings.grd @@ -182,7 +182,7 @@ Manage your shields Protect against privacy-invading ads and trackers while browsing with Brave Shields. Set Shields to "down" if a site doesn’t seem to be working properly. Shield Settings - Default + default Set default Select a search engine Choose the search engine you would like to use by default when searching the web from the address bar. diff --git a/components/test/brave_welcome_ui/actions/welcome_actions_test.ts b/components/test/brave_welcome_ui/actions/welcome_actions_test.ts index 7f507170bd4..aece26e44f1 100644 --- a/components/test/brave_welcome_ui/actions/welcome_actions_test.ts +++ b/components/test/brave_welcome_ui/actions/welcome_actions_test.ts @@ -37,12 +37,6 @@ describe('welcome_actions', () => { }) }) - it('getSearchEngineProvidersStarted', () => { - expect(actions.getSearchEngineProvidersStarted()).toEqual({ - type: types.IMPORT_DEFAULT_SEARCH_PROVIDERS_STARTED - }) - }) - it('getSearchEngineProvidersSuccess', () => { const mockPayload = [] expect(actions.getSearchEngineProvidersSuccess(mockPayload)).toEqual({ @@ -50,13 +44,4 @@ describe('welcome_actions', () => { payload: mockPayload }) }) - - it('getSearchEngineProvidersFailure', () => { - expect(actions.getSearchEngineProvidersFailure()).toEqual({ - type: types.IMPORT_DEFAULT_SEARCH_PROVIDERS_FAILURE - }) - }) - - // TODO: getSearchEngineProviders can be tested via mocking the entire - // store. Would require redux-mock-store as dependency }) diff --git a/components/test/brave_welcome_ui/components/searchBox_test.tsx b/components/test/brave_welcome_ui/components/searchBox_test.tsx index 30bc76ccd08..ccf84799444 100644 --- a/components/test/brave_welcome_ui/components/searchBox_test.tsx +++ b/components/test/brave_welcome_ui/components/searchBox_test.tsx @@ -2,7 +2,6 @@ * 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 * as sinon from 'sinon' import * as React from 'react' import { shallow } from 'enzyme' import { Content } from 'brave-ui/features/welcome' @@ -19,7 +18,7 @@ describe('searchBox component tests', () => { changeDefaultSearchProvider: () => null, searchProviders: mockSearchProviders } - describe('searchBox method tests', () => { + describe('searchBox render tests', () => { it('renders the component DOM without crashing', () => { const wrapper = shallow( { describe('searchBox method tests', () => { describe('onChangeDefaultSearchEngine', () => { it('should not call API action if no provider selected', () => { - const mockAction = sinon.spy() + const mockAction = jest.fn() const mockEvent = { target: { value: '' @@ -53,11 +52,11 @@ describe('searchBox component tests', () => { searchProviders={mockProps.searchProviders} />) wrapper.instance().onChangeDefaultSearchEngine(mockEvent) - sinon.assert.notCalled(mockAction) + expect(mockAction.mock.calls.length).toBe(0) }) it('should call API action if search provider selected', () => { - const mockAction = sinon.spy() + const mockAction = jest.fn() const mockEvent = { target: { value: '1' @@ -72,7 +71,7 @@ describe('searchBox component tests', () => { searchProviders={mockProps.searchProviders} />) wrapper.instance().onChangeDefaultSearchEngine(mockEvent) - sinon.assert.calledOnce(mockAction) + expect(mockAction.mock.calls.length).toBe(1) }) it('should have searchEngineSelected as true if search provider selected', () => { diff --git a/components/test/brave_welcome_ui/reducers/welcome_reducer_test.ts b/components/test/brave_welcome_ui/reducers/welcome_reducer_test.ts index 07e4bdf43f2..a53df85029f 100644 --- a/components/test/brave_welcome_ui/reducers/welcome_reducer_test.ts +++ b/components/test/brave_welcome_ui/reducers/welcome_reducer_test.ts @@ -2,7 +2,6 @@ * 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 * as sinon from 'sinon' import welcomeReducer from '../../../brave_welcome_ui/reducers/welcome_reducer' import * as actions from '../../../brave_welcome_ui/actions/welcome_actions' import { types } from '../../../brave_welcome_ui/constants/welcome_types' @@ -19,14 +18,14 @@ describe('welcomeReducer', () => { }) describe('IMPORT_NOW_REQUESTED', () => { - let importNowRequestStub + let importNowRequestStub: jest.SpyInstance beforeEach(() => { - importNowRequestStub = sinon.stub(chrome, 'send').resolves() + importNowRequestStub = jest.spyOn(chrome, 'send') }) afterEach(() => { - importNowRequestStub.restore() + importNowRequestStub.mockRestore() }) it('should call chrome.send with the correct arguments', () => { @@ -34,8 +33,8 @@ describe('welcomeReducer', () => { type: types.IMPORT_NOW_REQUESTED }) - sinon.assert.calledOnce(importNowRequestStub) - sinon.assert.calledWith(importNowRequestStub, 'importNowRequested') + expect(importNowRequestStub).toBeCalledTimes(1) + expect(importNowRequestStub).toBeCalledWith('importNowRequested', []) }) }) @@ -50,14 +49,14 @@ describe('welcomeReducer', () => { }) describe('CLOSE_TAB_REQUESTED', () => { - let closeTabRequestStub + let closeTabRequestStub: jest.SpyInstance beforeEach(() => { - closeTabRequestStub = sinon.stub(window, 'close').resolves() + closeTabRequestStub = jest.spyOn(window, 'close') }) afterEach(() => { - closeTabRequestStub.restore() + closeTabRequestStub.mockRestore() }) it('calls window.close', () => { @@ -65,19 +64,19 @@ describe('welcomeReducer', () => { type: types.CLOSE_TAB_REQUESTED, payload: undefined }) - sinon.assert.calledOnce(closeTabRequestStub) + expect(closeTabRequestStub).toBeCalledTimes(1) }) }) describe('CHANGE_DEFAULT_SEARCH_PROVIDER', () => { - let changeSearchProviderStub + let changeSearchProviderStub: jest.SpyInstance beforeEach(() => { - changeSearchProviderStub = sinon.stub(chrome, 'send').resolves() + changeSearchProviderStub = jest.spyOn(chrome, 'send') }) afterEach(() => { - changeSearchProviderStub.restore() + changeSearchProviderStub.mockRestore() }) it('should call chrome.send with the correct argument', () => { @@ -85,8 +84,8 @@ describe('welcomeReducer', () => { type: types.CHANGE_DEFAULT_SEARCH_PROVIDER, payload: '12345' }) - sinon.assert.calledOnce(changeSearchProviderStub) - sinon.assert.calledWith(changeSearchProviderStub, 'setDefaultSearchEngine', [12345]) + expect(changeSearchProviderStub).toBeCalledTimes(1) + expect(changeSearchProviderStub).toBeCalledWith('setDefaultSearchEngine', [12345]) }) }) }) diff --git a/components/test/testData.ts b/components/test/testData.ts index c51e4aa17ad..be2d3625f72 100644 --- a/components/test/testData.ts +++ b/components/test/testData.ts @@ -111,7 +111,7 @@ interface OnMessageEvent extends chrome.events.Event<(message: object, options: export const getMockChrome = () => { let mock = { - send: () => undefined, + send: (methodName: string, ...args: Array) => undefined, getVariableValue: () => undefined, braveRewards: { getPublisherData: (id: number, url: string, favicon: string) => undefined