Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Merge pull request #9480 from cezaraugusto/about/firstrun
Browse files Browse the repository at this point in the history
Make welcome screen show up on first time run
  • Loading branch information
cezaraugusto authored Jun 25, 2017
2 parents 9c10fa7 + 16d24b1 commit cf6e22f
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 18 deletions.
20 changes: 20 additions & 0 deletions app/browser/reducers/aboutWelcomeReducer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* 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/. */

'use strict'

const appConstants = require('../../../js/constants/appConstants')
const {makeImmutable} = require('../../common/state/immutableUtil')

const aboutWelcomeReducer = (state, action, immutableAction) => {
action = immutableAction || makeImmutable(action)
switch (action.get('actionType')) {
case appConstants.APP_ACTIVATE_WELCOME_SCREEN:
state = state.setIn(['about', 'welcome', 'showOnLoad'], action.get('activateWelcomeScreen'))
break
}
return state
}

module.exports = aboutWelcomeReducer
20 changes: 19 additions & 1 deletion app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {cleanupWebContents, currentWebContents, getWebContents, updateWebContents
const {FilterOptions} = require('ad-block')
const {isResourceEnabled} = require('../filtering')
const autofill = require('../autofill')
const l10n = require('../../js/l10n')

let currentPartitionNumber = 0
const incrementPartitionNumber = () => ++currentPartitionNumber
Expand Down Expand Up @@ -179,6 +180,7 @@ const updateAboutDetails = (tab, tabValue) => {
const autofillAddresses = appState.getIn(['autofill', 'addresses'])
const versionInformation = appState.getIn(['about', 'brave', 'versionInformation'])
const aboutDetails = tabValue.get('aboutDetails')

// TODO(bridiver) - convert this to an action
if (url === 'about:preferences#payments') {
tab.on('destroyed', () => {
Expand Down Expand Up @@ -656,7 +658,10 @@ const api = {
return true
},

create: (createProperties, cb = null, isRestore = false) => {
create: (createProperties, cb = null, isRestore = false, skipIfTest = true) => {
const appState = appStore.getState()
let shouldShowWelcomeScreen = appState.getIn(['about', 'welcome', 'showOnLoad'])

This comment has been minimized.

Copy link
@bridiver

bridiver Jun 26, 2017

Collaborator

this is not really the right place for this. appStore.getState should only rarely be needed and in this case we'd want to move this logic into a reducer. DM me if you need more info.


setImmediate(() => {
const {safeBrowsingInstance} = require('../adBlock')
createProperties = makeImmutable(createProperties).toJS()
Expand All @@ -673,6 +678,19 @@ const api = {
}
if (!createProperties.url) {
createProperties.url = newFrameUrl()
// don't open welcome screen for general tests
if (skipIfTest && process.env.NODE_ENV === 'test') {
shouldShowWelcomeScreen = false
}
if (shouldShowWelcomeScreen !== false) {
appActions.createTabRequested({

This comment has been minimized.

Copy link
@bridiver

bridiver Jun 26, 2017

Collaborator

also we don't want to call actions from inside api methods, particularly if those methods are called from actions. In this case if we kept this logic here you would just create a second tab directly

url: 'about:welcome',
// avoid chrome-extension title
// while page's title is not fetch
title: l10n.translation('aboutWelcome')
})
appActions.activateWelcomeScreen(false)

This comment has been minimized.

Copy link
@bridiver

bridiver Jun 26, 2017

Collaborator

action names should be declarative. In this case what I would do is handle 'about', 'welcome', 'showOnLoad' in appStore APP_NEW_WINDOW and conditionally add the welcome from to the new window

}
}
createProperties.url = normalizeUrl(createProperties.url)
// TODO(bridiver) - this should be in filtering
Expand Down
3 changes: 3 additions & 0 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,9 @@ module.exports.defaultAppState = () => {
sites: topSites,
ignoredTopSites: [],
pinnedTopSites: pinnedTopSites
},
welcome: {
showOnLoad: true
}
},
trackingProtection: {
Expand Down
11 changes: 11 additions & 0 deletions docs/appActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,17 @@ Clear temp setting for clear browsing data



### activateWelcomeScreen(shouldShowWelcomeScreen)

Dispatches a message to the store to show the welcome screen.

**Parameters**

**shouldShowWelcomeScreen**: `Boolean`, true if a new tab
with the welcome screen should be show




* * *

Expand Down
13 changes: 13 additions & 0 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,19 @@ const appActions = {
dispatch({
actionType: appConstants.APP_ON_CANCEL_BROWSING_DATA
})
},

/**
* Dispatches a message to the store to show the welcome screen.
*
* @param {Boolean} shouldShowWelcomeScreen - true if a new tab
* with the welcome screen should be show
*/
activateWelcomeScreen: function (activateWelcomeScreen) {
dispatch({
actionType: appConstants.APP_ACTIVATE_WELCOME_SCREEN,
activateWelcomeScreen
})
}
}

Expand Down
1 change: 1 addition & 0 deletions js/constants/appConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ const appConstants = {
APP_ON_GO_TO_INDEX: _,
APP_ON_GO_BACK_LONG: _,
APP_ON_GO_FORWARD_LONG: _,
APP_ACTIVATE_WELCOME_SCREEN: _,
APP_AUTOPLAY_BLOCKED: _,
APP_SAVE_PASSWORD: _,
APP_UPDATE_PASSWORD: _,
Expand Down
1 change: 1 addition & 0 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ const handleAppAction = (action) => {
require('../../app/browser/reducers/extensionsReducer'),
require('../../app/browser/reducers/shareReducer'),
require('../../app/browser/reducers/updatesReducer'),
require('../../app/browser/reducers/aboutWelcomeReducer'),
require('../../app/ledger').doAction
]
initialized = true
Expand Down
38 changes: 38 additions & 0 deletions test/unit/app/browser/reducers/aboutWelcomeReducerTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* global describe, it, before, after */
const mockery = require('mockery')
const Immutable = require('immutable')
const assert = require('assert')
const appConstants = require('../../../../../js/constants/appConstants')
const fakeElectron = require('../../../lib/fakeElectron')

require('../../../braveUnit')

describe('aboutWelcomeReducer', function () {
let aboutWelcomeReducer

before(function () {
mockery.enable({
warnOnReplace: false,
warnOnUnregistered: false,
useCleanCache: true
})
mockery.registerMock('electron', fakeElectron)
aboutWelcomeReducer = require('../../../../../app/browser/reducers/aboutWelcomeReducer')
})

after(function () {
mockery.disable()
})

describe('APP_ACTIVATE_WELCOME_SCREEN', function () {
before(function () {
this.newState = aboutWelcomeReducer(Immutable.Map(), {
actionType: appConstants.APP_ACTIVATE_WELCOME_SCREEN,
activateWelcomeScreen: false
})
})
it('can set visibility state to false', function () {
assert.equal(this.newState.getIn(['about', 'welcome', 'showOnLoad']), false)
})
})
})
96 changes: 79 additions & 17 deletions test/unit/app/browser/tabsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const fakeAdBlock = require('../../lib/fakeAdBlock')

require('../../braveUnit')

describe('tabs API', function () {
describe('tabs API unit tests', function () {
let tabs, appActions
before(function () {
mockery.enable({
Expand Down Expand Up @@ -59,9 +59,28 @@ describe('tabs API', function () {
}]
})

this.appState = {
about: {
welcome: {
showOnLoad: true
}
}
}

this.appStore = {
getState: () => Immutable.fromJS(this.appState)
}

mockery.registerMock('electron', fakeElectron)
mockery.registerMock('ad-block', fakeAdBlock)
mockery.registerMock('leveldown', {})
mockery.registerMock('../../js/stores/appStore', this.appStore)
mockery.registerMock('../../js/l10n', {
translation: (word) => word
})
mockery.registerMock('../filtering', {
isResourceEnabled: (resourceName, url, isPrivate) => false
})

mockery.registerMock('./webContentsCache', {
getWebContents: (tabId) => {
Expand Down Expand Up @@ -90,25 +109,17 @@ describe('tabs API', function () {
appActions = require('../../../../js/actions/appActions')
})

beforeEach(function () {
this.newWindowSpy = sinon.spy(appActions, 'newWindow')
this.newWebContentsAddedSpy = sinon.spy(appActions, 'newWebContentsAdded')
})

after(function () {
mockery.disable()
})

afterEach(function () {
this.tabWithDevToolsClosed.openDevTools.reset()
this.tabWithDevToolsClosed.closeDevTools.reset()
this.tabWithDevToolsOpened.openDevTools.reset()
this.tabWithDevToolsOpened.closeDevTools.reset()
this.newWindowSpy.restore()
this.newWebContentsAddedSpy.restore()
})

describe('toggleDevTools', function () {
afterEach(function () {
this.tabWithDevToolsClosed.openDevTools.reset()
this.tabWithDevToolsClosed.closeDevTools.reset()
this.tabWithDevToolsOpened.openDevTools.reset()
this.tabWithDevToolsOpened.closeDevTools.reset()
})
it('opens dev tools if closed', function () {
tabs.toggleDevTools(1)
assert(this.tabWithDevToolsClosed.openDevTools.calledOnce)
Expand All @@ -130,6 +141,7 @@ describe('tabs API', function () {
assert(this.tabWithDevToolsOpenedAndFocused.closeDevTools.notCalled)
})
})

describe('isDevToolsFocused', function () {
it('returns false if devtools are opened but not focused', function () {
assert.equal(tabs.isDevToolsFocused(1), false)
Expand All @@ -141,12 +153,21 @@ describe('tabs API', function () {
assert.equal(tabs.isDevToolsFocused(3), true)
})
})

describe('moveTo', function () {
before(function () {
this.browserOpts = {
positionByMouseCursor: true
}
})
beforeEach(function () {
this.newWindowSpy = sinon.spy(appActions, 'newWindow')
this.newWebContentsAddedSpy = sinon.spy(appActions, 'newWebContentsAdded')
})
afterEach(function () {
this.newWindowSpy.restore()
this.newWebContentsAddedSpy.restore()
})
it('moves tab to a new window', function () {
const state = this.state.set('dragData', Immutable.fromJS({
windowId: 1,
Expand Down Expand Up @@ -318,8 +339,49 @@ describe('tabs API', function () {
})
})

describe.skip('create', function () {
it('todo', function () {
describe('create', function () {
before(function () {
this.clock = sinon.useFakeTimers()
this.createTabRequestedSpy = sinon.spy(appActions, 'createTabRequested')
this.activateWelcomeScreenSpy = sinon.spy(appActions, 'activateWelcomeScreen')
this.extensionsCreateTabSpy = sinon.spy(fakeElectron.extensions, 'createTab')
})

after(function () {
this.clock.restore()
this.createTabRequestedSpy.restore()
this.activateWelcomeScreenSpy.restore()
this.extensionsCreateTabSpy.restore()
})

const createProperties = Immutable.fromJS({})
const cb = null
const isRestore = false
const skipIfTest = false

describe('when createProperties.url is not set', function () {
it('calls appActions.createTabRequested', function () {
this.createTabRequestedSpy.reset()
tabs.create(createProperties, cb, isRestore, skipIfTest)
this.clock.tick(1510)
assert.equal(this.createTabRequestedSpy.calledOnce, true)
})

describe('when welcome screen has not shown yet', function () {
it('calls appActions.activateWelcomeScreen if state is true', function () {
this.activateWelcomeScreenSpy.reset()
tabs.create(createProperties, cb, isRestore, skipIfTest)
this.clock.tick(1510)
assert.equal(this.activateWelcomeScreenSpy.calledOnce, true)
})
})
})

it('calls electron.extensions.createTab', function () {
this.extensionsCreateTabSpy.reset()
tabs.create(createProperties, cb, isRestore, skipIfTest)
this.clock.tick(1510)
assert.equal(this.extensionsCreateTabSpy.calledOnce, true)
})
})

Expand Down
4 changes: 4 additions & 0 deletions test/unit/app/sessionStoreTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,10 @@ describe('sessionStore unit tests', function () {
})

describe('defaultAppState', function () {
it('sets showOnLoad to true (which triggers about:welcome)', function () {
const defaultAppState = sessionStore.defaultAppState()
assert.equal(defaultAppState.about.welcome.showOnLoad, true)
})
})

describe('isProtocolHandled', function () {
Expand Down
3 changes: 3 additions & 0 deletions test/unit/lib/fakeElectron.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ const fakeElectron = {
defaultSession: {
partition: 'default'
}
},
extensions: {
createTab: function () {}
}
}

Expand Down

0 comments on commit cf6e22f

Please sign in to comment.