From e5d2de407814bca1f877766e99dcc0623bfe1bd4 Mon Sep 17 00:00:00 2001 From: bridiver Date: Mon, 17 Apr 2017 18:46:26 -0700 Subject: [PATCH] first pass at action consolidation for urlbar --- app/common/state/frameState.js | 24 +- app/common/state/navigationBarState.js | 109 +++-- app/common/state/tabState.js | 37 +- .../components/navigation/navigationBar.js | 71 +-- app/renderer/components/navigation/urlBar.js | 37 +- app/renderer/reducers/urlBarReducer.js | 424 +++++++++++++++++- .../reducers/urlBarSuggestionsReducer.js | 402 ----------------- docs/windowActions.md | 10 - js/actions/windowActions.js | 29 +- js/components/frame.js | 6 +- js/constants/windowConstants.js | 4 +- js/stores/windowStore.js | 14 - test/unit/app/common/state/frameStateTest.js | 2 +- .../renderer/reducers/urlBarReducerTest.js | 180 ++++++++ .../reducers/urlBarSuggestionsReducerTest.js | 186 -------- 15 files changed, 786 insertions(+), 749 deletions(-) delete mode 100644 app/renderer/reducers/urlBarSuggestionsReducer.js delete mode 100644 test/unit/app/renderer/reducers/urlBarSuggestionsReducerTest.js diff --git a/app/common/state/frameState.js b/app/common/state/frameState.js index 3b780a082d6..dbbe8b3275e 100644 --- a/app/common/state/frameState.js +++ b/app/common/state/frameState.js @@ -34,14 +34,14 @@ const api = { // in WindowState const index = state.get('frames').findIndex((frame) => frame.get('key') === frameKey) if (index === -1) { - return path.concat('nosuchframe') + return null } return path.concat(['frames', index]) } else { // in AppState const index = state.get('tabs').findIndex((tab) => tab.getIn(['frame', 'key']) === frameKey) if (index === -1) { - return path.concat('nosuchtab') + return null } return makeImmutable(['tabs', index, 'frame']) } @@ -61,21 +61,33 @@ const api = { // in WindowState const index = state.get('frames').findIndex((frame) => frame.get('tabId') === tabId) if (index === -1) { - return makeImmutable(['nosuchframe']) + return null } return path.concat(['frames', index]) }, getByFrameKey: (state, frameKey) => { - return state.getIn(api.getPathByFrameKey(state, frameKey)) + const path = api.getPathByFrameKey(state, frameKey) + if (path == null) { + return null + } + return state.getIn(path) }, getByTabId: (state, tabId) => { - return state.getIn(api.getPathByTabId(state, tabId)) + const path = api.getPathByTabId(state, tabId) + if (path == null) { + return null + } + return state.getIn(path) }, getTabIdByFrameKey: (state, frameKey) => { - return state.getIn(api.getPathByFrameKey(state, frameKey).concat('tabId')) || -1 + const path = api.getPathByFrameKey(state, frameKey) + if (path == null) { + return null + } + return state.getIn(path.concat('tabId')) || -1 } } diff --git a/app/common/state/navigationBarState.js b/app/common/state/navigationBarState.js index bb31ea6d517..248500c7a7a 100644 --- a/app/common/state/navigationBarState.js +++ b/app/common/state/navigationBarState.js @@ -40,7 +40,11 @@ const api = { state = validateState(state) tabState.validateTabId(tabId) - return tabState.getFramePathByTabId(state, tabId).concat('navbar') + const path = tabState.getFramePathByTabId(state, tabId) + if (path == null) { + return null + } + return path.push('navbar') }, getNavigationBar: (state, tabId) => { @@ -51,15 +55,25 @@ const api = { return defaultState } - return state.getIn(api.getNavigationBarPath(state, tabId)) || defaultState + const path = api.getNavigationBarPath(state, tabId) + if (path == null) { + return defaultState + } + + return state.getIn(path) || defaultState }, getUrlBarPath: (state, tabId) => { - return api.getNavigationBarPath(state, tabId).concat('urlbar') + const path = api.getNavigationBarPath(state, tabId) + return path == null ? null : path.push('urlbar') }, getUrlBar: (state, tabId) => { - return state.getIn(api.getUrlBarPath(state, tabId)) || defaultState.get('urlbar') + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return defaultState.get('urlbar') + } + return state.getIn(path) || defaultState.get('urlbar') }, locationValueSuffix: (state, tabId) => { @@ -103,7 +117,11 @@ const api = { setSelectedIndex: (state, tabId, index = -1) => { state = validateState(state) - return state.setIn(api.getUrlBarPath(state, tabId).concat(['suggestions', 'selectedIndex']), index) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + return state.setIn(path.concat(['suggestions', 'selectedIndex']), index) }, getSelectedIndex: (state, tabId) => { @@ -113,8 +131,12 @@ const api = { setSuggestionList: (state, tabId, suggestionList = []) => { state = validateState(state) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } suggestionList = makeImmutable(suggestionList) - return state.setIn(api.getUrlBarPath(state, tabId).concat(['suggestions', 'suggestionList']), suggestionList) + return state.setIn(path.concat(['suggestions', 'suggestionList']), suggestionList) }, getSuggestionList: (state, tabId) => { @@ -133,7 +155,11 @@ const api = { setAutocompleteEnabled: (state, tabId, enabled = false) => { state = validateState(state) - return state.setIn(api.getUrlBarPath(state, tabId).concat(['suggestions', 'autocompleteEnabled']), enabled) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + return state.setIn(path.concat(['suggestions', 'autocompleteEnabled']), enabled) }, isAutocompleteEnabled: (state, tabId) => { @@ -142,7 +168,11 @@ const api = { setLocation: (state, tabId, location, counter) => { state = validateState(state) - return state.setIn(api.getUrlBarPath(state, tabId).push('location'), location) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + return state.setIn(path.push('location'), location) }, getLocation: (state, tabId) => { @@ -150,8 +180,13 @@ const api = { }, setKeyCounter: (state, tabId, counter) => { + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + if (counter) { - state = state.setIn(api.getUrlBarPath(state, tabId).push('keyCounter'), counter) + state = state.setIn(path.push('keyCounter'), counter) } return state }, @@ -162,9 +197,13 @@ const api = { setShouldRenderUrlBarSuggestions: (state, tabId, enabled = false) => { state = validateState(state) - state = state.setIn(api.getUrlBarPath(state, tabId).concat(['suggestions', 'shouldRender']), enabled) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + state = state.setIn(path.concat(['suggestions', 'shouldRender']), enabled) if (!enabled) { - state = state.mergeIn(api.getUrlBarPath(state, tabId).push('suggestions'), defaultState.getIn(['urlbar', 'suggestions'])) + state = state.mergeIn(path.push('suggestions'), defaultState.getIn(['urlbar', 'suggestions'])) } return state }, @@ -175,39 +214,13 @@ const api = { suggestionList && suggestionList.size > 0 }, - setPreviousUrlBarSuggestionSelected: (state, tabId) => { - if (api.shouldRenderUrlBarSuggestions(state, tabId)) { - const selectedIndex = api.getSelectedIndex(state, tabId) - const lastSuffix = api.locationValueSuffix(state, tabId) - - if (selectedIndex !== 0 && !lastSuffix) { - state = api.setSelectedIndex(state, tabId, 0) - } else if (selectedIndex > 0) { - state = api.setSelectedIndex(state, tabId, selectedIndex - 1) - } - // state = updateUrlSuffix(state, state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), suggestionList)) - } - return state - }, - - setNextUrlBarSuggestionSelected: (state, tabId) => { - if (api.shouldRenderUrlBarSuggestions(state, tabId)) { - const selectedIndex = api.getSelectedIndex(state, tabId) - const lastSuffix = api.locationValueSuffix(state, tabId) - - if (selectedIndex !== 0 && !lastSuffix) { - state = api.setSelectedIndex(state, tabId, 0) - } else if (selectedIndex > 0) { - state = api.setSelectedIndex(state, tabId, selectedIndex - 1) - } - // state = updateUrlSuffix(state, state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), suggestionList)) - } - return state - }, - setActive: (state, tabId, active = false) => { state = validateState(state) - return state.setIn(api.getUrlBarPath(state, tabId).push('active'), active) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + return state.setIn(path.push('active'), active) }, isActive: (state, tabId) => { @@ -216,7 +229,11 @@ const api = { setFocused: (state, tabId, focused = false) => { state = validateState(state) - return state.setIn(api.getUrlBarPath(state, tabId).push('focused'), focused) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } + return state.setIn(path.push('focused'), focused) }, isFocused: (state, tabId) => { @@ -225,11 +242,15 @@ const api = { setSelected: (state, tabId, selected = false) => { state = validateState(state) + const path = api.getUrlBarPath(state, tabId) + if (path == null) { + return state + } if (selected) { // selection implies focus state = state = api.setFocused(state, tabId, true) } - return state.setIn(api.getUrlBarPath(state, tabId).push('selected'), selected) + return state.setIn(path.push('selected'), selected) }, isSelected: (state, tabId) => { diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js index 52d0683b333..93ea8185b84 100644 --- a/app/common/state/tabState.js +++ b/app/common/state/tabState.js @@ -171,8 +171,8 @@ const tabState = { state = validateState(state) const index = state.get('tabs').findIndex((tab) => tab.get('tabId') === tabId) - if (index === -1) { - return makeImmutable(['nosuchpath']) + if (index === tabState.TAB_ID_NONE) { + return null } return makeImmutable(['tabs', index]) }, @@ -185,7 +185,11 @@ const tabState = { return null } - return state.getIn(tabState.getPathByTabId(state, tabId)) + const path = tabState.getPathByTabId(state, tabId) + if (path == null) { + return null + } + return state.getIn(path) }, getTab: (state, tabValue) => { @@ -305,8 +309,17 @@ const tabState = { getFramePathByTabId: (state, tabId) => { state = makeImmutable(state) tabId = validateId('tabId', tabId) - if (state.get('tabs')) { - return tabState.getPathByTabId(state, tabId).push('frame') + + if (tabId === tabState.TAB_ID_NONE) { + return null + } + + if (state.get('tabs') && !state.get('frames')) { + const path = tabState.getPathByTabId(state, tabId) + if (path == null) { + return null + } + return path.push('frame') } else { return frameState.getPathByTabId(state, tabId) } @@ -324,32 +337,36 @@ const tabState = { }, getFrameByTabId: (state, tabId) => { + const path = tabState.getFramePathByTabId(state, tabId) + if (path == null) { + return null + } return state.getIn(tabState.getFramePathByTabId(state, tabId)) }, isSecure: (state, tabId) => { const frame = tabState.getFrameByTabId(state, tabId) - return frameStateUtil.isFrameSecure(frame) + return frame ? frameStateUtil.isFrameSecure(frame) : null }, isLoading: (state, tabId) => { const frame = tabState.getFrameByTabId(state, tabId) - return frameStateUtil.isFrameLoading(frame) + return frame ? frameStateUtil.isFrameLoading(frame) : null }, startLoadTime: (state, tabId) => { const frame = tabState.getFrameByTabId(state, tabId) - return frameStateUtil.startLoadTime(frame) + return frame ? frameStateUtil.startLoadTime(frame) : null }, endLoadTime: (state, tabId) => { const frame = tabState.getFrameByTabId(state, tabId) - return frameStateUtil.endLoadTime(frame) + return frame ? frameStateUtil.endLoadTime(frame) : null }, getHistory: (state, tabId) => { const frame = tabState.getFrameByTabId(state, tabId) - return frameStateUtil.getHistory(frame) + return frame ? frameStateUtil.getHistory(frame) : null }, getLocation: (state, tabId) => { diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js index bda38294ed9..ff86580d514 100644 --- a/app/renderer/components/navigation/navigationBar.js +++ b/app/renderer/components/navigation/navigationBar.js @@ -44,10 +44,6 @@ class NavigationBar extends React.Component { return windowStore.getFrame(this.props.activeFrameKey) } - get loading () { - return this.props.activeFrameKey !== undefined && this.props.loading - } - onToggleBookmark () { const editing = this.bookmarked // show the AddEditBookmarkHanger control; saving/deleting takes place there @@ -111,21 +107,6 @@ class NavigationBar extends React.Component { })) } - get titleMode () { - const hasTitle = this.props.title && this.props.location && this.props.title !== this.props.location.replace(/^https?:\/\//, '') - return this.props.activeTabShowingMessageBox || - ( - this.props.mouseInTitlebar === false && - !this.props.bookmarkDetail && - hasTitle && - !['about:blank', 'about:newtab'].includes(this.props.location) && - !this.loading && - !this.props.navbar.getIn(['urlbar', 'focused']) && - !this.props.navbar.getIn(['urlbar', 'active']) && - getSetting(settings.DISABLE_TITLE_MODE) === false - ) - } - componentDidMount () { ipc.on(messages.SHORTCUT_ACTIVE_FRAME_BOOKMARK, () => this.onToggleBookmark()) ipc.on(messages.SHORTCUT_ACTIVE_FRAME_REMOVE_BOOKMARK, () => this.onToggleBookmark()) @@ -134,26 +115,50 @@ class NavigationBar extends React.Component { mergeProps (state, dispatchProps, ownProps) { const windowState = state.get('currentWindow') const activeFrame = frameStateUtil.getActiveFrame(windowState) || Immutable.Map() + const activeFrameKey = activeFrame.get('key') const activeTabId = tabState.getActiveTabId(state, getCurrentWindowId()) + + const activeTabShowingMessageBox = tabState.isShowingMessageBox(state, activeTabId) + const bookmarkDetail = windowState.get('bookmarkDetail') + const mouseInTitlebar = windowState.getIn(['ui', 'mouseInTitlebar']) + const title = activeFrame.get('title') || '' + const loading = activeFrame.get('loading') + const location = activeFrame.get('location') || '' + const navbar = activeFrame.get('navbar') || Immutable.Map() + + const hasTitle = title && location && title !== location.replace(/^https?:\/\//, '') + const titleMode = activeTabShowingMessageBox || + ( + mouseInTitlebar === false && + !bookmarkDetail && + hasTitle && + !['about:blank', 'about:newtab'].includes(location) && + !loading && + !navbar.getIn(['urlbar', 'focused']) && + !navbar.getIn(['urlbar', 'active']) && + getSetting(settings.DISABLE_TITLE_MODE) === false + ) + const props = {} - props.navbar = activeFrame.get('navbar') + props.navbar = navbar props.sites = state.get('sites') - props.activeFrameKey = activeFrame.get('key') - props.location = activeFrame.get('location') || '' - props.title = activeFrame.get('title') || '' + props.activeFrameKey = activeFrameKey + props.location = location + props.title = title props.partitionNumber = activeFrame.get('partitionNumber') || 0 props.isSecure = activeFrame.getIn(['security', 'isSecure']) - props.loading = activeFrame.get('loading') - props.bookmarkDetail = windowState.get('bookmarkDetail') - props.mouseInTitlebar = windowState.getIn(['ui', 'mouseInTitlebar']) + props.loading = loading + props.bookmarkDetail = bookmarkDetail + props.mouseInTitlebar = mouseInTitlebar props.enableNoScript = ownProps.enableNoScript props.settings = state.get('settings') props.menubarVisible = ownProps.menubarVisible props.siteSettings = state.get('siteSettings') props.synopsis = state.getIn(['publisherInfo', 'synopsis']) || new Immutable.Map() - props.activeTabShowingMessageBox = tabState.isShowingMessageBox(state, activeTabId) + props.activeTabShowingMessageBox = activeTabShowingMessageBox props.locationInfo = state.get('locationInfo') + props.titleMode = titleMode return props } @@ -168,7 +173,7 @@ class NavigationBar extends React.Component { ref='navigator' data-frame-key={this.props.activeFrameKey} className={cx({ - titleMode: this.titleMode + titleMode: this.props.titleMode })}> { this.props.bookmarkDetail && this.props.bookmarkDetail.get('isBookmarkHanger') @@ -182,9 +187,9 @@ class NavigationBar extends React.Component { : null } { - this.titleMode + this.props.titleMode ? null - : this.loading + : this.props.loading ?