Skip to content

Commit

Permalink
Removes non-primitive types from UrlBar
Browse files Browse the repository at this point in the history 
Resolves brave#9756

Auditors: @bsclifton @bbondy

Test Plan:
  • Loading branch information
@NejcZdovc
NejcZdovc committed Jul 5, 2017
1 parent 4cc7d68 commit c33d345
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 103 deletions.
14 changes: 13 additions & 1 deletion app/common/lib/suggestion.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,17 @@ const filterSuggestionListByType = (suggestionList) => {
}
}

const getNormalizedSuggestion = (suggestionList, activeIndex) => {
let suggestion = ''
let normalizedSuggestion = ''
if (suggestionList && suggestionList.size > 0) {
suggestion = suggestionList.getIn([activeIndex || 0, 'location'], '')
normalizedSuggestion = normalizeLocation(suggestion)
}

return normalizedSuggestion
}

module.exports = {
sortingPriority,
sortByAccessCountWithAgeDecay,
Expand All @@ -710,5 +721,6 @@ module.exports = {
getAlexaSuggestions,
generateNewSuggestionsList,
generateNewSearchXHRResults,
filterSuggestionListByType
filterSuggestionListByType,
getNormalizedSuggestion
}
20 changes: 4 additions & 16 deletions app/renderer/components/navigation/navigationBar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,7 @@ class NavigationBar extends React.Component {

onStop () {
ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_STOP)
if (this.props.navbar.getIn(['urlbar', 'focused'])) {
windowActions.setUrlBarActive(false)
const shouldRenderSuggestions = this.props.navbar.getIn(['urlbar', 'suggestions', 'shouldRender']) === true
const suggestionList = this.props.navbar.getIn(['urlbar', 'suggestions', 'suggestionList'])
if (!shouldRenderSuggestions ||
// TODO: Once we take out suggestion generation from within URLBarSuggestions we can remove this check
// and put it in shouldRenderUrlBarSuggestions where it belongs. See https://github.com/brave/browser-laptop/issues/3151
!suggestionList || suggestionList.size === 0) {
windowActions.setUrlBarSelected(true)
}
}
windowActions.onStop(this.props.isFocused, this.props.shouldRenderSuggestions)
}

componentDidMount () {
Expand Down Expand Up @@ -144,7 +134,8 @@ class NavigationBar extends React.Component {
props.showHomeButton = !props.titleMode && getSetting(settings.SHOW_HOME_BUTTON)

// used in other functions
props.navbar = navbar // TODO(nejc) remove, primitives only
props.isFocused = navbar.getIn(['urlbar', 'focused'], false)
props.shouldRenderSuggestions = navbar.getIn(['urlbar', 'suggestions', 'shouldRender']) === true
props.sites = state.get('sites') // TODO(nejc) remove, primitives only
props.activeTabId = activeTabId
props.showHomeButton = !props.titleMode && getSetting(settings.SHOW_HOME_BUTTON)
Expand Down Expand Up @@ -208,10 +199,7 @@ class NavigationBar extends React.Component {
: null
}
</div>
<UrlBar
titleMode={this.props.titleMode}
onStop={this.onStop}
/>
<UrlBar titleMode={this.props.titleMode} />
{
this.props.showPublisherToggle
? <div className='endButtons'>
Expand Down
124 changes: 49 additions & 75 deletions app/renderer/components/navigation/urlBar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const frameStateUtil = require('../../../../js/state/frameStateUtil')
const siteSettings = require('../../../../js/state/siteSettings')
const tabState = require('../../../common/state/tabState')
const siteSettingsState = require('../../../common/state/siteSettingsState')
const menuBarState = require('../../../common/state/menuBarState')

// Utils
const cx = require('../../../../js/lib/classSet')
Expand All @@ -36,7 +35,8 @@ const contextMenus = require('../../../../js/contextMenus')
const {eventElHasAncestorWithClasses, isForSecondaryAction} = require('../../../../js/lib/eventUtil')
const {getBaseUrl, isUrl} = require('../../../../js/lib/appUrlUtil')
const {getCurrentWindowId} = require('../../currentWindow')
const {normalizeLocation} = require('../../../common/lib/suggestion')
const {normalizeLocation, getNormalizedSuggestion} = require('../../../common/lib/suggestion')
const {isDarwin} = require('../../../common/lib/platformUtil')
const publisherUtil = require('../../../common/lib/publisherUtil')

// Icons
Expand Down Expand Up @@ -99,13 +99,6 @@ class UrlBar extends React.Component {
windowActions.setRenderUrlBarSuggestions(false)
}

get activeIndex () {
if (this.props.suggestionList === null) {
return -1
}
return this.props.selectedIndex
}

onKeyDown (e) {
if (!this.props.isActive) {
windowActions.setUrlBarActive(true)
Expand All @@ -125,13 +118,12 @@ class UrlBar extends React.Component {
const isLocationUrl = isUrl(location)
if (!isLocationUrl && e.ctrlKey) {
appActions.loadURLRequested(this.props.activeTabId, `www.${location}.com`)
} else if (this.shouldRenderUrlBarSuggestions &&
((typeof this.activeIndex === 'number' && this.activeIndex >= 0) ||
} else if (this.props.showUrlBarSuggestions &&
((typeof this.props.activeIndex === 'number' && this.props.activeIndex >= 0) ||
(this.props.urlbarLocationSuffix && this.props.autocompleteEnabled))) {
// Hack to make alt enter open a new tab for url bar suggestions when hitting enter on them.
const isDarwin = process.platform === 'darwin'
if (e.altKey) {
if (isDarwin) {
if (isDarwin()) {
e.metaKey = true
} else {
e.ctrlKey = true
Expand Down Expand Up @@ -161,29 +153,28 @@ class UrlBar extends React.Component {
windowActions.setRenderUrlBarSuggestions(false)
break
case KeyCodes.UP:
if (this.shouldRenderUrlBarSuggestions) {
if (this.props.showUrlBarSuggestions) {
windowActions.previousUrlBarSuggestionSelected()
e.preventDefault()
}
break
case KeyCodes.DOWN:
if (this.shouldRenderUrlBarSuggestions) {
if (this.props.showUrlBarSuggestions) {
windowActions.nextUrlBarSuggestionSelected()
e.preventDefault()
}
break
case KeyCodes.ESC:
e.preventDefault()
this.props.onStop()
this.onStop()
this.restore()
this.select()
break
case KeyCodes.DELETE:
if (e.shiftKey) {
const selectedIndex = this.props.urlbarLocationSuffix.length > 0 ? 1 : this.props.selectedIndex
if (selectedIndex !== undefined) {
const suggestionLocation = this.props.suggestion.location
appActions.removeSite({ location: suggestionLocation })
appActions.removeSite({ location: this.props.suggestionLocation })
}
} else {
this.hideAutoComplete()
Expand All @@ -193,7 +184,7 @@ class UrlBar extends React.Component {
this.hideAutoComplete()
break
case KeyCodes.TAB:
if (this.shouldRenderUrlBarSuggestions) {
if (this.props.showUrlBarSuggestions) {
if (e.shiftKey) {
windowActions.previousUrlBarSuggestionSelected()
} else {
Expand All @@ -215,7 +206,7 @@ class UrlBar extends React.Component {
}
}

onClick (e) {
onClick () {
if (this.props.isSelected) {
windowActions.setUrlBarActive(true)
}
Expand All @@ -225,26 +216,10 @@ class UrlBar extends React.Component {
windowActions.urlBarOnBlur(getCurrentWindowId(), e.target.value, this.props.urlbarLocation, eventElHasAncestorWithClasses(e, ['urlBarSuggestions', 'urlbarForm']))
}

get suggestionLocation () {
const selectedIndex = this.props.selectedIndex
if (typeof selectedIndex === 'number') {
const suggestion = this.props.suggestion
if (suggestion) {
return suggestion.location
}
}
}

updateAutocomplete (newValue) {
let suggestion = ''
let suggestionNormalized = ''
if (this.props.suggestionList && this.props.suggestionList.size > 0) {
suggestion = this.props.suggestionList.getIn([this.activeIndex || 0, 'location']) || ''
suggestionNormalized = normalizeLocation(suggestion)
}
const newValueNormalized = normalizeLocation(newValue)
if (suggestionNormalized.startsWith(newValueNormalized) && suggestionNormalized.length > 0) {
const newSuffix = suggestionNormalized.substring(newValueNormalized.length)
if (this.props.normalizedSuggestion.startsWith(newValueNormalized) && this.props.normalizedSuggestion.length > 0) {
const newSuffix = this.props.normalizedSuggestion.substring(newValueNormalized.length)
this.setValue(newValue, newSuffix)
this.urlInput.setSelectionRange(newValue.length, newValue.length + newSuffix.length + 1)
return true
Expand Down Expand Up @@ -344,7 +319,7 @@ class UrlBar extends React.Component {
})
}

onFocus (e) {
onFocus () {
this.select()
windowActions.urlBarOnFocus(getCurrentWindowId())
}
Expand Down Expand Up @@ -415,11 +390,6 @@ class UrlBar extends React.Component {
return ''
}

get shouldRenderUrlBarSuggestions () {
return this.props.shouldRender === true &&
this.props.suggestionList && this.props.suggestionList.size > 0
}

onNoScript () {
windowActions.setNoScriptVisible()
}
Expand All @@ -428,6 +398,11 @@ class UrlBar extends React.Component {
contextMenus.onUrlBarContextMenu(e)
}

onStop () {
ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_STOP)
windowActions.onStop(this.props.isFocused, this.props.shouldRenderSuggestion)
}

mergeProps (state, ownProps) {
const currentWindow = state.get('currentWindow')
const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map()
Expand Down Expand Up @@ -459,44 +434,46 @@ class UrlBar extends React.Component {
searchShortcut = new RegExp('^' + provider.get('shortcut') + ' ', 'g')
searchURL = provider.get('search')
}
const suggestionList = urlbar.getIn(['suggestions', 'suggestionList'])
const scriptsBlocked = activeFrame.getIn(['noScript', 'blocked'])
const enableNoScript = siteSettingsState.isNoScriptEnabled(state, braverySettings)
const activeIndex = suggestionList === null ? -1 : selectedIndex

const props = {}

props.activeTabId = activeTabId
props.activeFrameKey = activeFrame.get('key')
props.frameLocation = frameLocation
props.displayURL = displayURL
// used in renderer
props.isWideURLbarEnabled = getSetting(settings.WIDE_URL_BAR)
props.publisherButtonVisible = publisherUtil.shouldShowAddPublisherButton(state, location, publisherId)
props.titleMode = ownProps.titleMode
props.hostValue = hostValue
props.urlbarLocation = urlbarLocation
props.title = activeFrame.get('title', '')
props.scriptsBlocked = activeFrame.getIn(['noScript', 'blocked'])
props.enableNoScript = siteSettingsState.isNoScriptEnabled(state, braverySettings)
props.showNoScriptInfo = props.enableNoScript && props.scriptsBlocked && props.scriptsBlocked.size
props.hasSuggestionMatch = urlbar.getIn(['suggestions', 'hasSuggestionMatch'])
props.displayURL = displayURL
props.startLoadTime = activeFrame.get('startLoadTime')
props.endLoadTime = activeFrame.get('endLoadTime')
props.loading = activeFrame.get('loading')
props.noScriptIsVisible = currentWindow.getIn(['ui', 'noScriptInfo', 'isVisible']) || false
props.menubarVisible = ownProps.menubarVisible
props.publisherButtonVisible = publisherUtil.shouldShowAddPublisherButton(state, location, publisherId)
props.onStop = ownProps.onStop
props.titleMode = ownProps.titleMode
props.urlbarLocation = urlbarLocation
props.urlbarLocationSuffix = urlbar.getIn(['suggestions', 'urlSuffix'])
props.selectedIndex = selectedIndex
props.suggestionList = urlbar.getIn(['suggestions', 'suggestionList'])
props.suggestion = urlbar.getIn(['suggestions', 'suggestionList', selectedIndex - 1])
props.shouldRender = urlbar.getIn(['suggestions', 'shouldRender'])
props.urlbarLocation = urlbarLocation
props.showDisplayTime = !props.titleMode && props.displayURL === location
props.showNoScriptInfo = enableNoScript && scriptsBlocked && scriptsBlocked.size
props.isActive = urlbar.get('active')
props.isSelected = urlbar.get('selected')
props.showUrlBarSuggestions = urlbar.getIn(['suggestions', 'shouldRender']) === true &&
suggestionList && suggestionList.size > 0

// used in other functions
props.activeFrameKey = activeFrame.get('key')
props.urlbarLocation = urlbarLocation
props.isFocused = urlbar.get('focused')
props.isWideURLbarEnabled = getSetting(settings.WIDE_URL_BAR)
props.searchSelectEntry = urlbarSearchDetail
props.frameLocation = frameLocation
props.isSelected = urlbar.get('selected')
props.noScriptIsVisible = currentWindow.getIn(['ui', 'noScriptInfo', 'isVisible'], false)
props.selectedIndex = selectedIndex
props.suggestionLocation = urlbar.getIn(['suggestions', 'suggestionList', selectedIndex - 1, 'location'])
props.normalizedSuggestion = getNormalizedSuggestion(suggestionList, activeIndex)
props.activeTabId = activeTabId
props.urlbarLocationSuffix = urlbar.getIn(['suggestions', 'urlSuffix'])
props.autocompleteEnabled = urlbar.getIn(['suggestions', 'autocompleteEnabled'])
props.searchURL = searchURL
props.searchShortcut = searchShortcut
props.showDisplayTime = !props.titleMode && props.displayURL === location
props.menubarVisible = menuBarState.isMenuBarVisible(currentWindow)
props.shouldRenderSuggestion = urlbar.getIn(['suggestions', 'shouldRender']) === true
props.activeIndex = activeIndex

return props
}
Expand Down Expand Up @@ -535,7 +512,6 @@ class UrlBar extends React.Component {
onContextMenu={this.onContextMenu}
data-l10n-id='urlbar'
className={cx({
private: this.private,
testHookLoadDone: !this.props.loading
})}
id='urlInput'
Expand Down Expand Up @@ -563,10 +539,8 @@ class UrlBar extends React.Component {
</span>
}
{
this.shouldRenderUrlBarSuggestions
? <UrlBarSuggestions
menubarVisible={this.props.menubarVisible}
/>
this.props.showUrlBarSuggestions
? <UrlBarSuggestions />
: null
}
</form>
Expand Down
5 changes: 3 additions & 2 deletions app/renderer/components/navigation/urlBarSuggestions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const locale = require('../../../../js/l10n')
const suggestions = require('../../../common/lib/suggestion')
const frameStateUtil = require('../../../../js/state/frameStateUtil')
const domUtil = require('../../lib/domUtil')
const menuBarState = require('../../../common/state/menuBarState')
const {getCurrentWindowId} = require('../../currentWindow')

class UrlBarSuggestions extends React.Component {
Expand Down Expand Up @@ -75,14 +76,14 @@ class UrlBarSuggestions extends React.Component {
const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map()
const urlBar = activeFrame.getIn(['navbar', 'urlbar'], Immutable.Map())
const documentHeight = domUtil.getStyleConstants('navbar-height')
const menuHeight = ownProps.menubarVisible ? 30 : 0
const menubarVisible = menuBarState.isMenuBarVisible(currentWindow)
const menuHeight = menubarVisible ? 30 : 0

const props = {}
// used in renderer
props.maxHeight = document.documentElement.offsetHeight - documentHeight - 2 - menuHeight

// used in functions
props.menubarVisible = ownProps.menubarVisible
props.suggestionList = urlBar.getIn(['suggestions', 'suggestionList']) // TODO (nejc) improve, use primitives
props.hasSuggestionMatch = urlBar.getIn(['suggestions', 'hasSuggestionMatch'])
props.activeIndex = props.suggestionList === null
Expand Down
30 changes: 22 additions & 8 deletions app/renderer/reducers/urlBarReducer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ const setActive = (state, isActive) => {
return state
}

const setUrlBarSelected = (state, selected) => {
const urlBarPath = activeFrameStatePath(state).concat(['navbar', 'urlbar'])
state = state.mergeIn(urlBarPath, {
selected: selected
})
// selection implies focus
if (selected) {
state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'focused']), true)
}

return state
}

const urlBarReducer = (state, action) => {
// TODO(bridiver) - this is a workaround until we can migrate frames to tabs
if (action.actionType === tabActions.didFinishNavigation.name) {
Expand Down Expand Up @@ -189,14 +202,7 @@ const urlBarReducer = (state, action) => {
state = setActive(state, false)
break
case windowConstants.WINDOW_SET_URL_BAR_SELECTED:
const urlBarPath = activeFrameStatePath(state).concat(['navbar', 'urlbar'])
state = state.mergeIn(urlBarPath, {
selected: action.selected
})
// selection implies focus
if (action.selected) {
state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'focused']), true)
}
state = setUrlBarSelected(state, action.selected)
break
case windowConstants.WINDOW_SET_FINDBAR_SHOWN:
if (action.shown) {
Expand Down Expand Up @@ -253,6 +259,14 @@ const urlBarReducer = (state, action) => {
})
}
break
case windowConstants.WINDOW_ON_STOP:
if (action.isFocused) {
state = setActive(state, false)
if (!action.shouldRender) {
state = setUrlBarSelected(state, true)
}
}
break
}
return state
}
Expand Down
Loading

0 comments on commit c33d345

Please sign in to comment.