👌 Address comments and clean up code: Closes brave/brave-browser#1548

This commit is contained in:
Peter Xu
2019-06-14 13:57:49 -07:00
parent 076d764430
commit 91dde786b2
10 changed files with 64 additions and 72 deletions
@@ -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<Welcome.SearchEngineEntry>) => 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()
@@ -46,8 +46,10 @@ export default class SearchEngineBox extends React.PureComponent<Props, State> {
this.setState({ searchEngineSelected: true })
}
getDefaultSearchProvider = (searchEngineEntries: Array<Welcome.SearchEngineEntry>): Welcome.SearchEngineEntry | undefined => {
return searchEngineEntries.find(entry => entry.default)
getDefaultSearchProvider = (searchEngineEntries: Array<Welcome.SearchEngineEntry>): 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<Props, State> {
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 (
<Content
zIndex={index}
@@ -80,14 +85,18 @@ export default class SearchEngineBox extends React.PureComponent<Props, State> {
onChange={this.onChangeDefaultSearchEngine}
>
<option key={0} value=''>{getLocale('selectSearchEngine')}</option>
{searchProviders.map((provider, index) =>
<option
key={index + 1}
value={provider.modelIndex.toString()}
>
{this.getProviderDisplayName(provider, defaultProvider!)}
</option>
)}
{
(searchProviders && Array.isArray(searchProviders) && searchProviders.length > 0)
? searchProviders.map((provider, index) =>
<option
key={index + 1}
value={provider.modelIndex.toString()}
>
{this.getProviderDisplayName(provider, defaultProvider)}
</option>
)
: null
}
</SelectBox>
<PrimaryButton
level='primary'
@@ -32,13 +32,9 @@ const welcomeReducer: Reducer<Welcome.State | undefined> = (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) {
@@ -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)
})
}
}
+1
View File
@@ -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
@@ -182,7 +182,7 @@
<message name="IDS_BRAVE_WELCOME_PAGE_SHIELDS_TITLE" desc="Welcome message title for shields">Manage your shields</message>
<message name="IDS_BRAVE_WELCOME_PAGE_SHIELDS_DESC" desc="Explainer text about shields">Protect against privacy-invading ads and trackers while browsing with Brave Shields. Set Shields to "down" if a site doesnt seem to be working properly.</message>
<message name="IDS_BRAVE_WELCOME_PAGE_SHIELDS_BUTTON" desc="Button to open shield settings">Shield Settings</message>
<message name="IDS_BRAVE_WELCOME_PAGE_DEFAULT_TEXT" desc="Indicator text to show current default">Default</message>
<message name="IDS_BRAVE_WELCOME_PAGE_DEFAULT_TEXT" desc="Indicator text to show current default">default</message>
<message name="IDS_BRAVE_WELCOME_PAGE_SET_DEFAULT_SEARCH_BUTTON" desc="Button to confirm default search engine">Set default</message>
<message name="IDS_BRAVE_WELCOME_PAGE_SEARCH_SELECT" desc="Default option message for selecting search engines">Select a search engine</message>
<message name="IDS_BRAVE_WELCOME_PAGE_SEARCH_DESC" desc="Explainer text about search engines in Brave">Choose the search engine you would like to use by default when searching the web from the address bar.</message>
@@ -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
})
@@ -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(
<SearchBox
@@ -38,7 +37,7 @@ describe('searchBox component tests', () => {
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', () => {
@@ -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])
})
})
})
+1 -1
View File
@@ -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<any>) => undefined,
getVariableValue: () => undefined,
braveRewards: {
getPublisherData: (id: number, url: string, favicon: string) => undefined