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 #9729 from brave/follow-up--9480
Browse files Browse the repository at this point in the history
Follow up of Welcome page #9480
  • Loading branch information
bsclifton committed Jun 28, 2017
1 parent d6daa08 commit 0668738
Show file tree
Hide file tree
Showing 14 changed files with 137 additions and 132 deletions.
20 changes: 0 additions & 20 deletions app/browser/reducers/aboutWelcomeReducer.js

This file was deleted.

21 changes: 21 additions & 0 deletions app/browser/reducers/tabsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,27 @@ const tabsReducer = (state, action, immutableAction) => {
}
}
break
case appConstants.APP_WINDOW_READY: {
if (!action.getIn(['createProperties', 'windowId'])) {
const senderWindowId = action.getIn(['senderWindowId'])
if (senderWindowId) {
action = action.setIn(['createProperties', 'windowId'], senderWindowId)
}
}

const welcomeScreenProperties = {
'url': 'about:welcome',
'windowId': action.getIn(['createProperties', 'windowId'])
}

const shouldShowWelcomeScreen = state.getIn(['about', 'welcome', 'showOnLoad'])
if (shouldShowWelcomeScreen) {
setImmediate(() => tabs.create(welcomeScreenProperties))
// We only need to run welcome screen once
state = state.setIn(['about', 'welcome', 'showOnLoad'], false)
}
break
}
case appConstants.APP_DRAG_ENDED: {
const dragData = state.get('dragData')
if (dragData && dragData.get('type') === dragTypes.TAB) {
Expand Down
20 changes: 1 addition & 19 deletions app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ 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 @@ -180,7 +179,6 @@ 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 @@ -658,10 +656,7 @@ const api = {
return true
},

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

create: (createProperties, cb = null, isRestore = false) => {
setImmediate(() => {
const {safeBrowsingInstance} = require('../adBlock')
createProperties = makeImmutable(createProperties).toJS()
Expand All @@ -678,19 +673,6 @@ 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({
url: 'about:welcome',
// avoid chrome-extension title
// while page's title is not fetch
title: l10n.translation('aboutWelcome')
})
appActions.activateWelcomeScreen(false)
}
}
createProperties.url = normalizeUrl(createProperties.url)
// TODO(bridiver) - this should be in filtering
Expand Down
2 changes: 1 addition & 1 deletion app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ module.exports.defaultAppState = () => {
pinnedTopSites: pinnedTopSites
},
welcome: {
showOnLoad: true
showOnLoad: !['test', 'development'].includes(process.env.NODE_ENV)
}
},
trackingProtection: {
Expand Down
11 changes: 6 additions & 5 deletions docs/appActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1138,14 +1138,15 @@ Dispatches a message to indicate that the update log is being opened



### activateWelcomeScreen(shouldShowWelcomeScreen)
### onToggleBrowsingData()

Dispatches a message to the store to show the welcome screen.
Save temp setting for clear browsing data

**Parameters**

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

### onCancelBrowsingData()

Clear temp setting for clear browsing data



Expand Down
23 changes: 15 additions & 8 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1444,15 +1444,22 @@ const appActions = {
},

/**
* 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) {
* Save temp setting for clear browsing data
*/
onToggleBrowsingData: function (property, newValue) {
dispatch({
actionType: appConstants.APP_ON_TOGGLE_BROWSING_DATA,
property,
newValue
})
},

/**
* Clear temp setting for clear browsing data
*/
onCancelBrowsingData: function () {
dispatch({
actionType: appConstants.APP_ACTIVATE_WELCOME_SCREEN,
activateWelcomeScreen
actionType: appConstants.APP_ON_CANCEL_BROWSING_DATA
})
}
}
Expand Down
1 change: 0 additions & 1 deletion js/constants/appConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ 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
2 changes: 1 addition & 1 deletion js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,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
appState = action.appState
Expand Down
38 changes: 0 additions & 38 deletions test/unit/app/browser/reducers/aboutWelcomeReducerTest.js

This file was deleted.

77 changes: 76 additions & 1 deletion test/unit/app/browser/reducers/tabsReducerTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ describe('tabsReducer unit tests', function () {
closeTab: sinon.mock(),
setActive: sinon.spy(),
moveTo: sinon.mock(),
reload: sinon.mock()
reload: sinon.mock(),
create: sinon.mock()
}

this.windowsAPI = require('../../../../../app/browser/windows')
Expand Down Expand Up @@ -608,6 +609,80 @@ describe('tabsReducer unit tests', function () {
})
})

describe('APP_WINDOW_READY', function () {
before(function () {
this.clock = sinon.useFakeTimers()
this.action = Immutable.fromJS({
actionType: appConstants.APP_WINDOW_READY,
senderWindowId: 1337,
createProperties: {
windowId: null
}
})
this.create = sinon.spy()
this.tabsAPI.create = this.create
tabsReducer = require('../../../../../app/browser/reducers/tabsReducer')
})

beforeEach(function () {
this.tabsAPI.create.reset()
})

after(function () {
this.clock.restore()
})

describe('when showOnLoad is true', function () {
before(function () {
this.newState = this.state.set('about', Immutable.fromJS({
welcome: {
showOnLoad: true
}
}))
})

it('calls tabs.create', function () {
tabsReducer(this.newState, this.action)
this.clock.tick(1510)
assert(this.tabsAPI.create.calledOnce)
})

it('sets tabs.create url argument to welcome screen', function () {
tabsReducer(this.newState, this.action)
this.clock.tick(1510)
assert.equal(
this.tabsAPI.create.args[0][0].url,
'about:welcome'
)
})

it('sets senderWindowId as the windowId when none is found', function () {
tabsReducer(this.newState, this.action)
this.clock.tick(1510)
assert.equal(
this.tabsAPI.create.args[0][0].windowId,
this.action.get('senderWindowId')
)
})
})

describe('when showOnLoad is false', function () {
before(function () {
this.newState = this.state.set('about', Immutable.fromJS({
welcome: {
showOnLoad: false
}
}))
})

it('does not call tabs.create', function () {
tabsReducer(this.newState, this.action)
this.clock.tick(1510)
assert.equal(this.tabsAPI.create.notCalled, true)
})
})
})

describe('APP_DRAG_ENDED', function () {
const action = {
actionType: appConstants.APP_DRAG_ENDED
Expand Down
34 changes: 1 addition & 33 deletions test/unit/app/browser/tabsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,6 @@ describe('tabs API unit tests', function () {
}]
})

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

this.appStore = {
getState: () => Immutable.fromJS(this.appState)
}
Expand All @@ -75,9 +67,6 @@ describe('tabs API unit tests', function () {
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
})
Expand Down Expand Up @@ -343,43 +332,22 @@ describe('tabs API unit tests', 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)
tabs.create(createProperties, cb, isRestore)
this.clock.tick(1510)
assert.equal(this.extensionsCreateTabSpy.calledOnce, true)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const {getHostPattern} = require('../../../../../../js/lib/urlutil')
let PublisherToggle
require('../../../../braveUnit')

describe('PublisherToggle component', function () {
describe.skip('PublisherToggle component', function () {
const getUrlOrigin = url => url.split('/').slice(0, 3).join('/')
const getHostName = url => getUrlOrigin(url).split('/').slice(2).join('/')
const getPattern = pattern => getHostPattern(getHostName(pattern))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const assert = require('assert')
let UrlBarIcon, windowActions
require('../../../../braveUnit')

describe('UrlBarIcon component unit tests', function () {
describe.skip('UrlBarIcon component unit tests', function () {
before(function () {
mockery.enable({
warnOnReplace: false,
Expand Down
Loading

0 comments on commit 0668738

Please sign in to comment.