diff --git a/app/browser/menu.js b/app/browser/menu.js index b7d89cef612..9e5034d8d77 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -34,6 +34,8 @@ const frameStateUtil = require('../../js/state/frameStateUtil') const menuUtil = require('../common/lib/menuUtil') const {getSetting} = require('../../js/settings') const locale = require('../locale') +// TODO REMOVE +const appStore = require('../../js/stores/appStore') const {isLocationBookmarked} = require('../../js/state/siteUtil') const {isDarwin, isLinux} = require('../common/lib/platformUtil') @@ -266,7 +268,7 @@ const createViewSubmenu = (state) => { accelerator: isDarwin ? 'Cmd+Alt+I' : 'Ctrl+Shift+I', click: function () { const win = BrowserWindow.getActiveWindow() - const activeTab = tabState.getActiveTab(state, win.id) + const activeTab = tabState.getActiveTab(appStore.getState(), win.id) appActions.toggleDevTools(activeTab.get('tabId')) } }, @@ -676,6 +678,8 @@ const doAction = (state, action) => { } break case appConstants.APP_ADD_SITE: + case appConstants.APP_ADD_BOOKMARK: + case appConstants.APP_EDIT_BOOKMARK: { if (action.tag === siteTags.BOOKMARK || action.tag === siteTags.BOOKMARK_FOLDER) { createMenu(state) diff --git a/app/browser/reducers/sitesReducer.js b/app/browser/reducers/sitesReducer.js index 8a4e87fec73..02fb434224d 100644 --- a/app/browser/reducers/sitesReducer.js +++ b/app/browser/reducers/sitesReducer.js @@ -54,35 +54,56 @@ const sitesReducer = (state, action, immutableAction) => { break } case appConstants.APP_ADD_SITE: - const isSyncEnabled = syncEnabled() - if (Immutable.List.isList(action.siteDetail)) { - action.siteDetail.forEach((s) => { - state = siteUtil.addSite(state, s, action.tag, undefined, action.skipSync) + { + const isSyncEnabled = syncEnabled() + if (Immutable.List.isList(action.siteDetail)) { + action.siteDetail.forEach((s) => { + state = siteUtil.addSite(state, s, action.tag, action.skipSync) + if (isSyncEnabled) { + state = syncUtil.updateSiteCache(state, s) + } + }) + } else { + let sites = state.get('sites') + if (!action.siteDetail.get('folderId') && siteUtil.isFolder(action.siteDetail)) { + action.siteDetail = action.siteDetail.set('folderId', siteUtil.getNextFolderId(sites)) + } + state = siteUtil.addSite(state, action.siteDetail, action.tag, action.skipSync) if (isSyncEnabled) { - state = syncUtil.updateSiteCache(state, s) + state = syncUtil.updateSiteCache(state, action.siteDetail) } - }) - } else { + } + state = updateActiveTabBookmarked(state) + break + } + case appConstants.APP_ADD_BOOKMARK: + case appConstants.APP_EDIT_BOOKMARK: + { + const isSyncEnabled = syncEnabled() + let destination = null // TODO add it let sites = state.get('sites') - if (!action.siteDetail.get('folderId') && siteUtil.isFolder(action.siteDetail)) { + if (!action.siteDetail.get('folderId') && action.tag === siteTags.BOOKMARK_FOLDER) { action.siteDetail = action.siteDetail.set('folderId', siteUtil.getNextFolderId(sites)) } - state = siteUtil.addSite(state, action.siteDetail, action.tag, action.originalSiteDetail, action.skipSync) - if (action.destinationDetail) { + state = siteUtil.addSite(state, action.siteDetail, action.tag, action.editKey) + + // TODO do we need it? + if (destination) { const sourceKey = siteUtil.getSiteKey(action.siteDetail) - const destinationKey = siteUtil.getSiteKey(action.destinationDetail) + const destinationKey = siteUtil.getSiteKey(destination) if (sourceKey != null) { - state = siteUtil.moveSite(state, - sourceKey, destinationKey, false, false, true) + state = siteUtil.moveSite(state, sourceKey, destinationKey, false, false, true) } } + if (isSyncEnabled) { - state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail) + state = syncUtil.updateSiteCache(state, destination || action.siteDetail) } + + state = updateActiveTabBookmarked(state) + break } - state = updateActiveTabBookmarked(state) - break case appConstants.APP_REMOVE_SITE: const removeSiteSyncCallback = action.skipSync ? undefined : syncActions.removeSite state = siteUtil.removeSite(state, action.siteDetail, action.tag, true, removeSiteSyncCallback) diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js index 086f9345b0d..8873091a9ad 100644 --- a/app/browser/reducers/urlBarSuggestionsReducer.js +++ b/app/browser/reducers/urlBarSuggestionsReducer.js @@ -14,6 +14,8 @@ const tabState = require('../../common/state/tabState') const urlBarSuggestionsReducer = (state, action) => { switch (action.actionType) { case appConstants.APP_ADD_SITE: + case appConstants.APP_ADD_BOOKMARK: + case appConstants.APP_EDIT_BOOKMARK: if (Immutable.List.isList(action.siteDetail)) { action.siteDetail.forEach((s) => { add(s) diff --git a/app/common/lib/bookmarkUtil.js b/app/common/lib/bookmarkUtil.js index f1beea204f0..0a6d10fca02 100644 --- a/app/common/lib/bookmarkUtil.js +++ b/app/common/lib/bookmarkUtil.js @@ -16,17 +16,20 @@ const {calculateTextWidth} = require('../../../js/lib/textCalculator') const {iconSize} = require('../../../js/constants/config') const {getSetting} = require('../../../js/settings') -const bookmarkHangerHeading = (detail, isFolder, shouldShowLocation) => { +function bookmarkHangerHeading (editMode, isFolder, isAdded) { if (isFolder) { - return shouldShowLocation + return editMode ? 'bookmarkFolderEditing' : 'bookmarkFolderAdding' } - return shouldShowLocation - ? (!detail || !detail.has('location')) - ? 'bookmarkCreateNew' - : 'bookmarkEdit' - : 'bookmarkAdded' + + if (isAdded) { + return 'bookmarkAdded' + } + + return editMode + ? 'bookmarkEdit' + : 'bookmarkCreateNew' } const displayBookmarkName = (detail) => { @@ -37,11 +40,10 @@ const displayBookmarkName = (detail) => { return detail.get('title') || '' } -const isBookmarkNameValid = (detail, isFolder) => { - const title = detail.get('title') || detail.get('customTitle') - const location = detail.get('location') +const isBookmarkNameValid = (title, location, isFolder, customTitle) => { + const newTitle = title || customTitle return isFolder - ? (title != null && title.trim().length > 0) + ? (newTitle != null && newTitle.trim().length > 0) : (location != null && location.trim().length > 0) } diff --git a/app/renderer/components/bookmarks/addEditBookmarkForm.js b/app/renderer/components/bookmarks/addEditBookmarkForm.js new file mode 100644 index 00000000000..34368176b28 --- /dev/null +++ b/app/renderer/components/bookmarks/addEditBookmarkForm.js @@ -0,0 +1,285 @@ +/* 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/. */ + +const React = require('react') +const Immutable = require('immutable') +const {StyleSheet, css} = require('aphrodite/no-important') + +// Components +const BrowserButton = require('../common/browserButton') +const { + CommonFormSection, + CommonFormDropdown, + CommonFormTextbox, + CommonFormButtonWrapper, + commonFormStyles +} = require('../common/commonForm') + +// Actions +const appActions = require('../../../../js/actions/appActions') +const windowActions = require('../../../../js/actions/windowActions') + +// Constants +const KeyCodes = require('../../../common/constants/keyCodes') +const siteTags = require('../../../../js/constants/siteTags') +const settings = require('../../../../js/constants/settings') + +// Utils +const UrlUtil = require('../../../../js/lib/urlutil') +const {getSetting} = require('../../../../js/settings') +const {isBookmarkNameValid} = require('../../../common/lib/bookmarkUtil') + +// Styles +const globalStyles = require('../styles/global') +const commonStyles = require('../styles/commonStyles') + +class AddEditBookmarkForm extends React.Component { + constructor (props) { + super(props) + this.onNameChange = this.onNameChange.bind(this) + this.onLocationChange = this.onLocationChange.bind(this) + this.onParentFolderChange = this.onParentFolderChange.bind(this) + this.onKeyDown = this.onKeyDown.bind(this) + this.onClose = this.onClose.bind(this) + this.onSave = this.onSave.bind(this) + this.onRemoveBookmark = this.onRemoveBookmark.bind(this) + this.state = { + title: props.bookmarkName, + location: props.location, + parentFolderId: props.parentFolderId, + isDisabled: props.isDisabled + } + } + + componentDidMount () { + setImmediate(() => { + this.bookmarkName.select() + }) + } + + onKeyDown (e) { + switch (e.keyCode) { + case KeyCodes.ENTER: + this.onSave() + break + case KeyCodes.ESC: + this.onClose() + break + } + } + + onClose () { + windowActions.onBookmarkClose() + } + + updateButtonStatus (newValue) { + if (newValue !== this.state.isDisabled) { + this.setState({ + isDisabled: newValue + }) + } + } + + onNameChange (e) { + let title = e.target.value + + this.setState({ + title: title + }) + + this.updateButtonStatus(!isBookmarkNameValid(title, this.state.location, this.props.isFolder)) + } + + onLocationChange (e) { + let location = e.target.value + + this.setState({ + location: location + }) + + this.updateButtonStatus(!isBookmarkNameValid(this.state.title, location, this.props.isFolder)) + } + + onParentFolderChange (e) { + this.setState({ + parentFolderId: ~~e.target.selectedIndex + }) + } + + onSave () { + // First check if the title of the bookmarkDetail is set + if (this.state.isDisabled) { + return false + } + + // show bookmark if hidden + if (!this.props.hasBookmarks && !getSetting(settings.SHOW_BOOKMARKS_TOOLBAR)) { + appActions.changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, true) + } + + const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK + let data = Immutable.fromJS({ + parentFolderId: this.state.parentFolderId, + title: this.props.currentTitle + }) + + // handle title input + let title = this.state.title + if (typeof title === 'string' && UrlUtil.isURL(title)) { + const punycodeUrl = UrlUtil.getPunycodeUrl(title) + if (punycodeUrl.replace(/\/$/, '') !== title) { + title = punycodeUrl + } + } + + if (this.props.currentTitle !== title || !title) { + data = data.set('customTitle', title || 0) + } + + // handle location input + let location = this.state.location + if (typeof location === 'string') { + const punycodeUrl = UrlUtil.getPunycodeUrl(location) + if (punycodeUrl.replace(/\/$/, '') !== location) { + location = punycodeUrl + } + } + data = data.set('location', location) + + if (this.props.editMode) { + appActions.editBookmark(data, tag) + } else { + appActions.addBookmark(data, tag) + } + + this.onClose() + } + + onRemoveBookmark () { + const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK + // TODO check if you need to add folderId as prop or you can use editKey + appActions.removeSite(Immutable.fromJS({ + parentFolderId: this.props.parentFolderId, + location: this.props.location, + partitionNumber: this.props.partitionNumber + }), tag) + this.onClose() + } + + render () { + return
+ +
+
+
+ { + !this.props.isFolder + ?
+
+
+
+ : null + } +
+
+
+
+ + { + this.props.editMode + ? + : + } + + +
+ } +} + +const styles = StyleSheet.create({ + bookmarkHanger__label: { + display: 'block', + marginBottom: `calc(${globalStyles.spacing.dialogInsideMargin} / 3)` + }, + bookmarkHanger__marginRow: { + marginTop: `calc(${globalStyles.spacing.dialogInsideMargin} / 2)` + }, + + bookmark__sectionWrapper: { + display: 'flex', + flexFlow: 'column nowrap' + } +}) + +module.exports = AddEditBookmarkForm diff --git a/app/renderer/components/bookmarks/addEditBookmarkHanger.js b/app/renderer/components/bookmarks/addEditBookmarkHanger.js index f3147d50371..186dbad42a6 100644 --- a/app/renderer/components/bookmarks/addEditBookmarkHanger.js +++ b/app/renderer/components/bookmarks/addEditBookmarkHanger.js @@ -8,150 +8,63 @@ const Immutable = require('immutable') // Components const ReduxComponent = require('../reduxComponent') const Dialog = require('../common/dialog') -const BrowserButton = require('../common/browserButton') +const AddEditBookmarkForm = require('./addEditBookmarkForm') // Actions const appActions = require('../../../../js/actions/appActions') const windowActions = require('../../../../js/actions/windowActions') // Constants -const KeyCodes = require('../../../common/constants/keyCodes') const siteTags = require('../../../../js/constants/siteTags') const settings = require('../../../../js/constants/settings') // Utils const cx = require('../../../../js/lib/classSet') const siteUtil = require('../../../../js/state/siteUtil') -const UrlUtil = require('../../../../js/lib/urlutil') const {getSetting} = require('../../../../js/settings') const {bookmarkHangerHeading, displayBookmarkName, isBookmarkNameValid} = require('../../../common/lib/bookmarkUtil') const {StyleSheet, css} = require('aphrodite/no-important') const globalStyles = require('../styles/global') -const commonStyles = require('../styles/commonStyles') const { CommonFormBookmarkHanger, - CommonFormSection, - CommonFormDropdown, - CommonFormTextbox, - CommonFormButtonWrapper, - CommonFormBottomWrapper, - commonFormStyles + CommonFormBottomWrapper } = require('../common/commonForm') class AddEditBookmarkHanger extends React.Component { constructor (props) { super(props) - this.onNameChange = this.onNameChange.bind(this) - this.onLocationChange = this.onLocationChange.bind(this) - this.onParentFolderChange = this.onParentFolderChange.bind(this) - this.onKeyDown = this.onKeyDown.bind(this) this.onClose = this.onClose.bind(this) this.onClick = this.onClick.bind(this) - this.onSave = this.onSave.bind(this) this.onViewBookmarks = this.onViewBookmarks.bind(this) - this.onRemoveBookmark = this.onRemoveBookmark.bind(this) } - - setDefaultFocus () { - this.bookmarkName.select() - this.bookmarkName.focus() - } - componentDidMount () { - // Automatically save if this is triggered by the url star - if (!this.props.isModal && !this.props.shouldShowLocation) { - this.onSave(false) - } - this.bookmarkName.value = displayBookmarkName(this.props.currentDetail) - this.setDefaultFocus() - } - - onKeyDown (e) { - switch (e.keyCode) { - case KeyCodes.ENTER: - this.onSave() - break - case KeyCodes.ESC: - this.onClose() - break - } + this.addBookmark() } onClose () { - windowActions.setBookmarkDetail() + windowActions.onBookmarkClose() } onClick (e) { e.stopPropagation() } - onNameChange (e) { - let currentDetail = this.props.currentDetail - let name = e.target.value - if (typeof name === 'string' && UrlUtil.isURL(name)) { - const punycodeUrl = UrlUtil.getPunycodeUrl(name) - if (punycodeUrl.replace(/\/$/, '') !== name) { - name = punycodeUrl - } - } - if (currentDetail.get('title') === name && name) { - currentDetail = currentDetail.delete('customTitle') - } else { - // '' is reserved for the default customTitle value of synced bookmarks, - // so replace '' with 0 if the user is deleting the customTitle. - // Note that non-string bookmark titles fail isBookmarkNameValid so they - // are not saved. - currentDetail = currentDetail.set('customTitle', name || 0) - } - if (this.bookmarkName.value !== name) { - this.bookmarkName.value = name - } - windowActions.setBookmarkDetail(currentDetail, this.props.originalDetail, this.props.destinationDetail, this.props.shouldShowLocation, !this.props.isModal) - } - - onLocationChange (e) { - let location = e.target.value - if (typeof location === 'string') { - const punycodeUrl = UrlUtil.getPunycodeUrl(location) - if (punycodeUrl.replace(/\/$/, '') !== location) { - location = punycodeUrl - } + addBookmark () { + if (!this.props.isAdded || this.props.isFolder) { + return false } - const currentDetail = this.props.currentDetail.set('location', location) - windowActions.setBookmarkDetail(currentDetail, this.props.originalDetail, this.props.destinationDetail, this.props.shouldShowLocation, !this.props.isModal) - } - - onParentFolderChange (e) { - const currentDetail = this.props.currentDetail.set('parentFolderId', Number(e.target.value)) - windowActions.setBookmarkDetail(currentDetail, this.props.originalDetail, this.props.destinationDetail, undefined, !this.props.isModal) - } - showToolbarOnFirstBookmark () { if (!this.props.hasBookmarks && !getSetting(settings.SHOW_BOOKMARKS_TOOLBAR)) { appActions.changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, true) } - } - - onSave (closeDialog = true) { - // First check if the title of the currentDetail is set - if (!this.props.isBookmarkNameValid) { - return false - } - this.showToolbarOnFirstBookmark() - const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK - appActions.addSite(this.props.currentDetail, tag, this.props.originalDetail, this.props.destinationDetail) - if (closeDialog) { - this.onClose() - } - } - - onRemoveBookmark () { - const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK - appActions.removeSite(this.props.currentDetail, tag) - this.onClose() + appActions.addBookmark(Immutable.fromJS({ + title: this.props.bookmarkName, + location: this.props.location, + folderId: this.props.parentId + }), siteTags.BOOKMARK) } onViewBookmarks () { @@ -161,9 +74,10 @@ class AddEditBookmarkHanger extends React.Component { mergeProps (state, ownProps) { const currentWindow = state.get('currentWindow') - const bookmarkDetail = currentWindow.get('bookmarkDetail', new Immutable.Map()) - const currentDetail = bookmarkDetail.get('currentDetail', new Immutable.Map()) - const originalDetail = bookmarkDetail.get('originalDetail') + const bookmarkDetail = currentWindow.get('bookmarkDetail', Immutable.Map()) + const siteDetail = bookmarkDetail.get('siteDetail', Immutable.Map()) + const editMode = bookmarkDetail.has('editKey') + const isAdded = bookmarkDetail.get('isAdded', false) const props = {} // used in renderer @@ -171,20 +85,24 @@ class AddEditBookmarkHanger extends React.Component { props.withHomeButton = getSetting(settings.SHOW_HOME_BUTTON) // Fake a folderId property so that the bookmark is considered a bookmark folder. // This is ImmutableJS so it doesn't actually set a value, it just returns a new one. - props.isFolder = siteUtil.isFolder(currentDetail.set('folderId', 0)) - props.shouldShowLocation = bookmarkDetail.get('shouldShowLocation') - props.heading = bookmarkHangerHeading(originalDetail, props.isFolder, props.shouldShowLocation) - props.location = currentDetail.get('location') - props.parentFolderId = currentDetail.get('parentFolderId') - props.hasOriginalDetail = !!originalDetail - props.displayBookmarkName = displayBookmarkName(currentDetail) - props.isBookmarkNameValid = isBookmarkNameValid(currentDetail, props.isFolder) - props.folders = siteUtil.getFolders(state.get('sites'), currentDetail.get('folderId')) // TODO (nejc) improve, primitives only + props.isFolder = siteUtil.isFolder(siteDetail.set('folderId', 0)) + props.heading = bookmarkHangerHeading(editMode, props.isFolder, isAdded) + props.location = siteDetail.get('location') + props.parentFolderId = siteDetail.get('parentFolderId') + props.partitionNumber = siteDetail.get('partitionNumber') + props.bookmarkName = displayBookmarkName(siteDetail) + props.currentTitle = siteDetail.get('title') + props.isBookmarkNameValid = isBookmarkNameValid( + siteDetail.get('title'), + siteDetail.get('location'), + props.isFolder, + siteDetail.get('customTitle') + ) + props.folders = siteUtil.getFolders(state.get('sites'), siteDetail.get('folderId')) // TODO (nejc) improve, primitives only + props.editMode = editMode // used in functions - props.currentDetail = currentDetail // TODO (nejc) improve, primitives only - props.originalDetail = originalDetail // TODO (nejc) improve, primitives only - props.destinationDetail = bookmarkDetail.get('destinationDetail') // TODO (nejc) improve, primitives only + props.isAdded = isAdded props.hasBookmarks = state.get('sites').some( (site) => siteUtil.isBookmark(site) || siteUtil.isFolder(site) ) @@ -211,89 +129,17 @@ class AddEditBookmarkHanger extends React.Component { [css(styles.commonFormSection)]: true, [css(styles.commonFormTitle)]: true })} data-l10n-id={this.props.heading} /> - -
-
-
- { - !this.props.isFolder && this.props.shouldShowLocation - ?
-
-
-
- : null - } -
-
-
-
- - { - this.props.hasOriginalDetail - ? - : - } - - + { !this.props.isModal ? @@ -351,18 +197,6 @@ const styles = StyleSheet.create({ bookmarkHanger__withHomeButton: { left: '83px' }, - bookmarkHanger__label: { - display: 'block', - marginBottom: `calc(${globalStyles.spacing.dialogInsideMargin} / 3)` - }, - bookmarkHanger__marginRow: { - marginTop: `calc(${globalStyles.spacing.dialogInsideMargin} / 2)` - }, - - bookmark__sectionWrapper: { - display: 'flex', - flexFlow: 'column nowrap' - }, bookmark__bottomWrapper: { display: 'flex', justifyContent: 'center' diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js index 158b47f7f74..dd280bf105d 100644 --- a/app/renderer/components/navigation/navigationBar.js +++ b/app/renderer/components/navigation/navigationBar.js @@ -19,7 +19,6 @@ const windowActions = require('../../../../js/actions/windowActions') const appActions = require('../../../../js/actions/appActions') // Constants -const siteTags = require('../../../../js/constants/siteTags') const messages = require('../../../../js/constants/messages') const settings = require('../../../../js/constants/settings') @@ -28,15 +27,11 @@ const tabState = require('../../../common/state/tabState') const publisherState = require('../../../common/lib/publisherUtil') const frameStateUtil = require('../../../../js/state/frameStateUtil') -// Store -const windowStore = require('../../../../js/stores/windowStore') - // Utils const cx = require('../../../../js/lib/classSet') const {getBaseUrl} = require('../../../../js/lib/appUrlUtil') const siteUtil = require('../../../../js/state/siteUtil') const eventUtil = require('../../../../js/lib/eventUtil') -const UrlUtil = require('../../../../js/lib/urlutil') const {getSetting} = require('../../../../js/settings') const contextMenus = require('../../../../js/contextMenus') @@ -51,22 +46,14 @@ class NavigationBar extends React.Component { this.onReloadLongPress = this.onReloadLongPress.bind(this) } - get activeFrame () { - return windowStore.getFrame(this.props.activeFrameKey) - } - onToggleBookmark () { const editing = this.props.isBookmarked - // show the AddEditBookmarkHanger control; saving/deleting takes place there - let siteDetail = siteUtil.getDetailFromFrame(this.activeFrame, siteTags.BOOKMARK) - const key = siteUtil.getSiteKey(siteDetail) - if (key !== null) { - siteDetail = siteDetail.set('parentFolderId', this.props.sites.getIn([key, 'parentFolderId'])) - siteDetail = siteDetail.set('customTitle', this.props.sites.getIn([key, 'customTitle'])) + if (editing) { + windowActions.editBookmark(true, this.props.bookmarkKey) + } else { + windowActions.onBookmarkAdded(true, this.props.bookmarkKey) } - siteDetail = siteDetail.set('location', UrlUtil.getLocationIfPDF(siteDetail.get('location'))) - windowActions.setBookmarkDetail(siteDetail, siteDetail, null, editing, true) } onReload (e) { @@ -137,8 +124,8 @@ class NavigationBar extends React.Component { // used in other functions 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.bookmarkKey = siteUtil.getSiteKey(activeFrame) props.showHomeButton = !props.titleMode && getSetting(settings.SHOW_HOME_BUTTON) return props diff --git a/docs/appActions.md b/docs/appActions.md index 16041894cea..24f15a943ec 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -171,7 +171,7 @@ Notifies that a tab has been closed -### addSite(siteDetail, tag, originalSiteDetail, destinationIsParent, skipSync) +### addSite(siteDetail, tag, skipSync) Adds a site to the site list @@ -181,11 +181,6 @@ Adds a site to the site list **tag**: `string`, A tag to associate with the site. e.g. bookmarks. -**originalSiteDetail**: `string`, If specified, the original site detail to edit / overwrite. - -**destinationIsParent**: `boolean`, Whether or not the destinationDetail should be considered the new parent. - The details of the old entries will be modified if this is set, otherwise only the tag will be added. - **skipSync**: `boolean`, Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync) diff --git a/docs/windowActions.md b/docs/windowActions.md index 5355ab83404..25a7be5c44d 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -386,22 +386,23 @@ Dispatches a message to set the find-in-page details. -### setBookmarkDetail(currentDetail, originalDetail, destinationDetail, shouldShowLocation, isBookmarkHanger) +### addBookmark() -Dispatches a message to set add/edit bookmark details -If set, also indicates that add/edit is shown +Used for displaying bookmark hanger +when adding bookmark site or folder -**Parameters** -**currentDetail**: `Object`, Properties of the bookmark to change to -**originalDetail**: `Object`, Properties of the bookmark to edit +### editBookmark() + +Used for displaying bookmark hanger +when editing bookmark site or folder + -**destinationDetail**: `Object`, Will move the added bookmark to the specified position -**shouldShowLocation**: `boolean`, Whether or not to show the URL input +### onBookmarkClose() -**isBookmarkHanger**: `boolean`, true if triggered from star icon in nav bar +Used for closing a bookmark dialog diff --git a/js/about/aboutActions.js b/js/about/aboutActions.js index 746fe3f5b21..1992015338f 100644 --- a/js/about/aboutActions.js +++ b/js/about/aboutActions.js @@ -4,7 +4,6 @@ const messages = require('../constants/messages') const appDispatcher = require('../dispatcher/appDispatcher') -const windowConstants = require('../constants/windowConstants') const appConstants = require('../constants/appConstants') const ExtensionConstants = require('../../app/common/constants/extensionConstants') const ipc = window.chrome.ipcRenderer @@ -356,51 +355,6 @@ const aboutActions = { }) }, - /** - * Show the "Add Bookmark" control - * @param {Object} siteDetail - object bound to add/edit control - */ - showAddBookmark: function (siteDetail) { - aboutActions.dispatchAction({ - actionType: windowConstants.WINDOW_SET_BOOKMARK_DETAIL, - currentDetail: siteDetail, - originalDetail: null, - destinationDetail: null, - shouldShowLocation: true - }) - }, - - /** - * Show the "Add Bookmark" control for a folder - * @param {Object} siteDetail - object bound to add/edit control - */ - showAddBookmarkFolder: function (siteDetail) { - aboutActions.dispatchAction({ - actionType: windowConstants.WINDOW_SET_BOOKMARK_DETAIL, - currentDetail: siteDetail - }) - }, - - /** - * Dispatches a message to set add/edit bookmark details - * If set, also indicates that add/edit is shown - * @param {Object} currentDetail - Properties of the bookmark to change to - * @param {Object} originalDetail - Properties of the bookmark to edit - * @param {Object} destinationDetail - Will move the added bookmark to the specified position - * @param {boolean} shouldShowLocation - Whether or not to show the URL input - * @param {boolean} isBookmarkHanger - true if triggered from star icon in nav bar - */ - setBookmarkDetail: function (currentDetail, originalDetail, destinationDetail, shouldShowLocation, isBookmarkHanger) { - aboutActions.dispatchAction({ - actionType: windowConstants.WINDOW_SET_BOOKMARK_DETAIL, - currentDetail, - originalDetail, - destinationDetail, - shouldShowLocation, - isBookmarkHanger - }) - }, - /** * Dispatch a message to set default browser */ diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js index a19a7f835c9..e0de03a3634 100644 --- a/js/about/bookmarks.js +++ b/js/about/bookmarks.js @@ -16,6 +16,7 @@ const SortableTable = require('../../app/renderer/components/common/sortableTabl const siteUtil = require('../state/siteUtil') const formatUtil = require('../../app/common/lib/formatUtil') const {iconSize} = require('../constants/config') +const windowActions = require('../actions/windowActions') const ipc = window.chrome.ipcRenderer @@ -201,7 +202,7 @@ class BookmarkTitleHeader extends ImmutableComponent { parentFolderId: this.props.selectedFolderId, tags: [siteTags.BOOKMARK] }) - aboutActions.showAddBookmark(newBookmark) + windowActions.addBookmark(newBookmark) } render () { return
@@ -444,7 +445,7 @@ class AboutBookmarks extends React.Component { parentFolderId: this.state.selectedFolderId, tags: [siteTags.BOOKMARK_FOLDER] }) - aboutActions.showAddBookmarkFolder(newFolder) + windowActions.addBookmark(newFolder) } clearSelection () { this.refs.bookmarkList.clearSelection() diff --git a/js/about/newtab.js b/js/about/newtab.js index dde0fc0ec55..e9a2b83010b 100644 --- a/js/about/newtab.js +++ b/js/about/newtab.js @@ -21,6 +21,7 @@ const config = require('../constants/config') const backgrounds = require('../data/backgrounds') const {random} = require('../../app/common/lib/randomUtil') const NewPrivateTab = require('./newprivatetab') +const windowActions = require('../actions/windowActions') const ipc = window.chrome.ipcRenderer @@ -96,7 +97,7 @@ class NewTabPage extends React.Component { }).size > 0 } isBookmarked (siteProps) { - return siteUtil.isSiteBookmarked(this.topSites, siteProps) + return siteUtil.isBookmark(siteProps) } get gridLayout () { const sizeToCount = {large: 18, medium: 12, small: 6} @@ -171,8 +172,13 @@ class NewTabPage extends React.Component { onToggleBookmark (siteProps) { const siteDetail = siteUtil.getDetailFromFrame(siteProps, siteTags.BOOKMARK) const editing = this.isBookmarked(siteProps) - // PDFJS will not show up in new tab - aboutActions.setBookmarkDetail(siteDetail, siteDetail, null, editing) + const key = siteUtil.getSiteKey(siteDetail) + + if (editing) { + windowActions.editBookmark(false, key) + } else { + windowActions.onBookmarkAdded(false, key) + } } onPinnedTopSite (siteProps) { diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 40ebbb7ad4b..063e3c93727 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -237,18 +237,13 @@ const appActions = { * Adds a site to the site list * @param {Object} siteDetail - Properties of the site in question, can also be an array of siteDetail * @param {string} tag - A tag to associate with the site. e.g. bookmarks. - * @param {string} originalSiteDetail - If specified, the original site detail to edit / overwrite. - * @param {boolean} destinationIsParent - Whether or not the destinationDetail should be considered the new parent. - * The details of the old entries will be modified if this is set, otherwise only the tag will be added. * @param {boolean} skipSync - Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync) */ - addSite: function (siteDetail, tag, originalSiteDetail, destinationDetail, skipSync) { + addSite: function (siteDetail, tag, skipSync) { dispatch({ actionType: appConstants.APP_ADD_SITE, siteDetail, tag, - originalSiteDetail, - destinationDetail, skipSync }) }, @@ -1469,6 +1464,23 @@ const appActions = { actionType: appConstants.APP_SWIPE_RIGHT, percent }) + }, + + addBookmark: function (siteDetail, tag) { + dispatch({ + actionType: appConstants.APP_ADD_BOOKMARK, + siteDetail, + tag + }) + }, + + editBookmark: function (siteDetail, tag, editKey) { + dispatch({ + actionType: appConstants.APP_EDIT_BOOKMARK, + siteDetail, + tag, + editKey + }) } } diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 7802c6fbbd8..c769af491d4 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -492,22 +492,43 @@ const windowActions = { }, /** - * Dispatches a message to set add/edit bookmark details - * If set, also indicates that add/edit is shown - * @param {Object} currentDetail - Properties of the bookmark to change to - * @param {Object} originalDetail - Properties of the bookmark to edit - * @param {Object} destinationDetail - Will move the added bookmark to the specified position - * @param {boolean} shouldShowLocation - Whether or not to show the URL input - * @param {boolean} isBookmarkHanger - true if triggered from star icon in nav bar - */ - setBookmarkDetail: function (currentDetail, originalDetail, destinationDetail, shouldShowLocation, isBookmarkHanger) { - dispatch({ - actionType: windowConstants.WINDOW_SET_BOOKMARK_DETAIL, - currentDetail, - originalDetail, - destinationDetail, - shouldShowLocation, - isBookmarkHanger + * Used for displaying bookmark hanger + * when adding bookmark site or folder + */ + addBookmark: function (siteDetail) { + dispatch({ + actionType: windowConstants.WINDOW_ON_ADD_BOOKMARK, + siteDetail + }) + }, + + /** + * Used for displaying bookmark hanger + * when editing bookmark site or folder + */ + editBookmark: function (isHanger, editKey) { + dispatch({ + actionType: windowConstants.WINDOW_ON_EDIT_BOOKMARK, + editKey, + isHanger + }) + }, + + onBookmarkAdded: function (isHanger, editKey, siteDetail) { + dispatch({ + actionType: windowConstants.WINDOW_ON_BOOKMARK_ADDED, + siteDetail, + editKey, + isHanger + }) + }, + + /** + * Used for closing a bookmark dialog + */ + onBookmarkClose: function () { + dispatch({ + actionType: windowConstants.WINDOW_ON_BOOKMARK_CLOSE }) }, diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index de32438fb0a..eeff2c95247 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -137,7 +137,9 @@ const appConstants = { APP_ON_CANCEL_BROWSING_DATA: _, APP_SET_SKIP_SYNC: _, APP_SWIPE_LEFT: _, - APP_SWIPE_RIGHT: _ + APP_SWIPE_RIGHT: _, + APP_ADD_BOOKMARK: _, + APP_EDIT_BOOKMARK: _ } module.exports = mapValuesByKeys(appConstants) diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 85c77e7c427..eec5b2daf69 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -104,7 +104,11 @@ const windowConstants = { WINDOW_ON_CERT_ERROR: _, WINDOW_ON_TAB_PAGE_CONTEXT_MENU: _, WINDOW_ON_FRAME_BOOKMARK: _, - WINDOW_ON_STOP: _ + WINDOW_ON_STOP: _, + WINDOW_ON_ADD_BOOKMARK: _, + WINDOW_ON_EDIT_BOOKMARK: _, + WINDOW_ON_BOOKMARK_CLOSE: _, + WINDOW_ON_BOOKMARK_ADDED: _ } module.exports = mapValuesByKeys(windowConstants) diff --git a/js/contextMenus.js b/js/contextMenus.js index 6c282607932..1e19f5466c0 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -45,7 +45,6 @@ const isLinux = process.platform === 'linux' /** * Obtains an add bookmark menu item - * @param {object} Detail of the bookmark to initialize with */ const addBookmarkMenuItem = (label, siteDetail, closestDestinationDetail, isParent) => { return { @@ -58,8 +57,9 @@ const addBookmarkMenuItem = (label, siteDetail, closestDestinationDetail, isPare if (siteDetail.constructor !== Immutable.Map) { siteDetail = Immutable.fromJS(siteDetail) } + siteDetail = siteDetail.set('location', urlUtil.getLocationIfPDF(siteDetail.get('location'))) - windowActions.setBookmarkDetail(siteDetail, siteDetail, closestDestinationDetail, true) + windowActions.addBookmark(siteDetail) } } } @@ -72,7 +72,7 @@ const addFolderMenuItem = (closestDestinationDetail, isParent) => { if (isParent) { emptyFolder = emptyFolder.set('parentFolderId', closestDestinationDetail && (closestDestinationDetail.get('folderId') || closestDestinationDetail.get('parentFolderId'))) } - windowActions.setBookmarkDetail(emptyFolder, undefined, closestDestinationDetail, false) + windowActions.addBookmark(emptyFolder) } } } @@ -295,7 +295,10 @@ function siteDetailTemplateInit (siteDetail, activeFrame) { template.push( { label: locale.translation(isFolder ? 'editFolder' : 'editBookmark'), - click: () => windowActions.setBookmarkDetail(siteDetail, siteDetail, null, true) + click: () => { + const editKey = siteUtil.getSiteKey(siteDetail) + windowActions.editBookmark(false, editKey) + } }, CommonMenu.separatorMenuItem) } diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index d7ac67d6b71..1013918710b 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -257,35 +257,33 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId, order) => } /** - * Adds or updates the specified siteDetail in appState.sites. + * Adds the specified siteDetail in appState.sites. * * Examples of updating: * - editing bookmark in add/edit modal * - when timestamp is added (history entry) * - moving bookmark to/from a folder * - * @param sites The application state's Immutable site list - * @param siteDetails The site details object to add or update - * @param tag The tag to add for this site + * @param state - The application state + * @param siteDetail - The site details object to add or update + * @param tag - The tag to add for this site * See siteTags.js for supported types. No tag means just a history item - * @param originalSiteDetail If specified, use when searching site list - * @param {boolean=} skipSync - True if this site was downloaded by sync and + * @param oldKey - key of a changed site + * @param {boolean} skipSync - True if this site was downloaded by sync and * does not to be re-uploaded * @return The new state Immutable object */ -module.exports.addSite = function (state, siteDetail, tag, originalSiteDetail, skipSync) { +module.exports.addSite = function (state, siteDetail, tag, oldKey, skipSync) { let sites = state.get('sites') // Get tag from siteDetail object if not passed via tag param if (tag === undefined) { tag = siteDetail.getIn(['tags', 0]) } - let originalSiteKey - if (originalSiteDetail) { - originalSiteKey = module.exports.getSiteKey(originalSiteDetail) + if (!oldKey) { + oldKey = module.exports.getSiteKey(siteDetail) } - const oldKey = originalSiteKey || module.exports.getSiteKey(siteDetail) const oldSite = oldKey !== null ? sites.get(oldKey) : null let folderId = siteDetail.get('folderId') @@ -314,6 +312,7 @@ module.exports.addSite = function (state, siteDetail, tag, originalSiteDetail, s location = UrlUtil.getLocationIfPDF(site.get('location')) site = site.set('location', location) } + const oldLocation = (oldSite && oldSite.get('location')) || site.get('location') state = siteCache.removeLocationSiteKey(state, oldLocation, oldKey) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 2fc501419f4..84c909649ca 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -450,6 +450,8 @@ const handleAppAction = (action) => { nativeImage.copyDataURL(action.dataURL, action.html, action.text) break case appConstants.APP_ADD_SITE: + case appConstants.APP_ADD_BOOKMARK: + case appConstants.APP_EDIT_BOOKMARK: const oldSiteSize = appState.get('sites').size appState = aboutNewTabState.setSites(appState, action) appState = aboutHistoryState.setHistory(appState, action) diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 088396799e2..21332e88b24 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -248,7 +248,8 @@ const applyReducers = (state, action, immutableAction) => [ const immediatelyEmittedActions = [ windowConstants.WINDOW_SET_NAVBAR_INPUT, windowConstants.WINDOW_SET_FIND_DETAIL, - windowConstants.WINDOW_SET_BOOKMARK_DETAIL, + windowConstants.WINDOW_ON_ADD_BOOKMARK, + windowConstants.WINDOW_ON_EDIT_BOOKMARK, windowConstants.WINDOW_AUTOFILL_POPUP_HIDDEN, windowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL, windowConstants.WINDOW_SET_POPUP_WINDOW_DETAIL, @@ -443,17 +444,42 @@ const doAction = (action) => { } break } - case windowConstants.WINDOW_SET_BOOKMARK_DETAIL: - if (!action.currentDetail && !action.originalDetail) { - windowState = windowState.delete('bookmarkDetail') - } else { - windowState = windowState.mergeIn(['bookmarkDetail'], { - currentDetail: action.currentDetail, - originalDetail: action.originalDetail, - destinationDetail: action.destinationDetail, - shouldShowLocation: action.shouldShowLocation, - isBookmarkHanger: action.isBookmarkHanger - }) + case windowConstants.WINDOW_ON_ADD_BOOKMARK: + windowState = windowState.setIn(['bookmarkDetail'], Immutable.fromJS({ + siteDetail: action.siteDetail, + isBookmarkHanger: false + })) + break + case windowConstants.WINDOW_ON_BOOKMARK_CLOSE: + windowState = windowState.delete('bookmarkDetail') + break + case windowConstants.WINDOW_ON_EDIT_BOOKMARK: + const siteDetail = appStoreRenderer.state.getIn(['sites', action.editKey]) + + windowState = windowState.setIn(['bookmarkDetail'], Immutable.fromJS({ + siteDetail: siteDetail, + editKey: action.editKey, + isBookmarkHanger: action.isHanger + })) + break + case windowConstants.WINDOW_ON_BOOKMARK_ADDED: + { + let editKey = action.editKey + const site = appStoreRenderer.state.getIn(['sites', editKey]) + let siteDetail = action.siteDetail + + if (site) { + siteDetail = site + } + + siteDetail = siteDetail.set('location', UrlUtil.getLocationIfPDF(siteDetail.get('location'))) + + windowState = windowState.setIn(['bookmarkDetail'], Immutable.fromJS({ + siteDetail: siteDetail, + editKey: editKey, + isBookmarkHanger: action.isHanger, + isAdded: true + })) } break case windowConstants.WINDOW_AUTOFILL_SELECTION_CLICKED: diff --git a/test/bookmark-components/bookmarksTest.js b/test/bookmark-components/bookmarksTest.js index 37825b2fe2c..14ffbda5097 100644 --- a/test/bookmark-components/bookmarksTest.js +++ b/test/bookmark-components/bookmarksTest.js @@ -78,14 +78,18 @@ describe('bookmark tests', function () { .typeText(bookmarkNameInput, 'https://www.brave.com') .keys(Brave.keys.END) .keys('а') - .waitForInputText(bookmarkNameInput, 'https://www.brave.xn--com-8cd/') .setValue(bookmarkLocationInput, '') .waitForInputText(bookmarkLocationInput, '') .typeText(bookmarkLocationInput, 'https://www.brave.com') .keys(Brave.keys.END) .keys('а') - .waitForInputText(bookmarkLocationInput, 'https://www.brave.xn--com-8cd/') - .click(removeButton) + .click(doneButton) + .windowByUrl(Brave.browserWindowUrl) + .waitUntil(function () { + return this.getAppState().then((val) => { + return val.value.sites['https://www.brave.xn--com-8cd/|0|0'].customTitle === 'https://www.brave.xn--com-8cd/' + }) + }) }) it('custom title and location can be backspaced', function * () { yield this.app.client diff --git a/test/lib/brave.js b/test/lib/brave.js index b52f1b2eb73..f8634e661f8 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -484,9 +484,9 @@ var exports = { return this.waitUntil(function () { return this.getWindowState().then((val) => { const bookmarkDetailLocation = val.value && val.value.bookmarkDetail && - val.value.bookmarkDetail.currentDetail && val.value.bookmarkDetail.currentDetail.location - const bookmarkDetailTitle = (val.value && val.value.bookmarkDetail && val.value.bookmarkDetail.currentDetail && - val.value.bookmarkDetail.currentDetail.customTitle) || val.value.bookmarkDetail.currentDetail.title + val.value.bookmarkDetail.siteDetail && val.value.bookmarkDetail.siteDetail.location + const bookmarkDetailTitle = (val.value && val.value.bookmarkDetail && val.value.bookmarkDetail.siteDetail && + val.value.bookmarkDetail.siteDetail.customTitle) || val.value.bookmarkDetail.siteDetail.title const ret = bookmarkDetailLocation === location && bookmarkDetailTitle === title logVerbose('waitForBookmarkDetail("' + location + '", "' + title + '") => ' + ret + ' (bookmarkDetailLocation = ' + bookmarkDetailLocation + ', bookmarkDetailTitle = ' + bookmarkDetailTitle + ')') diff --git a/test/unit/app/common/lib/bookmarkUtilTest.js b/test/unit/app/common/lib/bookmarkUtilTest.js index 034f5629468..a93f6302b93 100644 --- a/test/unit/app/common/lib/bookmarkUtilTest.js +++ b/test/unit/app/common/lib/bookmarkUtilTest.js @@ -15,39 +15,30 @@ require('../../../braveUnit') describe('bookmarkUtil test', function () { describe('bookmarkHangerHeading', function () { - const detail = makeImmutable({ - location: 'https://ww.brave.com' + it('returns default if isFolder and editKey are not provided', function () { + assert.equal(bookmarkUtil.bookmarkHangerHeading(), 'bookmarkCreateNew') }) - it('returns default if isFolder and shouldShowLocation are not provided', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(detail), 'bookmarkAdded') + it('if bookmark was newly added', function () { + assert.equal(bookmarkUtil.bookmarkHangerHeading(false, false, true), 'bookmarkAdded') }) describe('is folder', function () { - it('returns editing mode when shouldShowLocation is not provided', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(detail, true, true), 'bookmarkFolderEditing') + it('returns editing mode when in edit mode', function () { + assert.equal(bookmarkUtil.bookmarkHangerHeading(true, true), 'bookmarkFolderEditing') }) - - it('returns editing mode when shouldShowLocation is true', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(detail, true, true), 'bookmarkFolderEditing') - }) - - it('returns add mode when shouldShowLocation is false', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(detail, true, false), 'bookmarkFolderAdding') + it('returns edit mode when in edit mode', function () { + assert.equal(bookmarkUtil.bookmarkHangerHeading(false, true), 'bookmarkFolderAdding') }) }) describe('is bookmark', function () { - it('returns add mode when shouldShowLocation is false', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(detail, false, false), 'bookmarkAdded') - }) - - it('returns create mode when details is empty', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(makeImmutable({}), false, true), 'bookmarkCreateNew') + it('returns create mode when not in edit mode', function () { + assert.equal(bookmarkUtil.bookmarkHangerHeading(false, false), 'bookmarkCreateNew') }) - it('returns edit mode when details is provided', function () { - assert.equal(bookmarkUtil.bookmarkHangerHeading(detail, false, true), 'bookmarkEdit') + it('returns edit mode when in edit mode', function () { + assert.equal(bookmarkUtil.bookmarkHangerHeading(true, false), 'bookmarkEdit') }) }) }) @@ -95,60 +86,41 @@ describe('bookmarkUtil test', function () { describe('isBookmarkNameValid', function () { describe('for folder', function () { it('title and custom title is not provided', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - location: 'https://ww.brave.com' - }), true), false) + assert.equal(bookmarkUtil.isBookmarkNameValid(), false) }) it('title and custom title are null', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - title: null, - customTitle: null - }), true), false) + assert.equal(bookmarkUtil.isBookmarkNameValid(null, null, true), false) }) it('title is empty string', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - title: '' - }), true), false) + assert.equal(bookmarkUtil.isBookmarkNameValid('', null, true), false) }) it('title is null, but customTitle is ok', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - title: null, - customTitle: 'custom brave' - }), true), true) + assert.equal(bookmarkUtil.isBookmarkNameValid(null, null, true, 'custom brave'), true) }) it('title and customTitle are ok', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - title: 'brave', - customTitle: 'custom brave' - }), true), true) + assert.equal(bookmarkUtil.isBookmarkNameValid('brave', null, true, 'custom brave'), true) }) }) describe('for bookmark', function () { it('location is not provided', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({}), true), false) + assert.equal(bookmarkUtil.isBookmarkNameValid(null, null, false), false) }) it('location is null', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - location: null - }), false), false) + assert.equal(bookmarkUtil.isBookmarkNameValid(null, null, false), false) }) it('location is empty string', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - location: '' - }), false), false) + assert.equal(bookmarkUtil.isBookmarkNameValid(null, '', false), false) }) it('location is provided', function () { - assert.equal(bookmarkUtil.isBookmarkNameValid(makeImmutable({ - location: 'https://ww.brave.com' - }), false), true) + assert.equal(bookmarkUtil.isBookmarkNameValid(null, 'https://www.brave.com', false), true) }) }) }) diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index d78cb75d09c..f45653f3ca0 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -273,7 +273,8 @@ describe('siteUtil', function () { let state = emptyState state = siteUtil.addSite(state, folderMinFields, siteTags.BOOKMARK_FOLDER) const folderMinFieldsWithId = folderMinFields.set('folderId', 1) - state = siteUtil.addSite(state, folderMinFieldsWithId, siteTags.BOOKMARK_FOLDER, folderMinFieldsWithId.set('folderId', 9)) + const oldKey = siteUtil.getSiteKey(folderMinFieldsWithId.set('folderId', 9)) + state = siteUtil.addSite(state, folderMinFieldsWithId, siteTags.BOOKMARK_FOLDER, false, oldKey) const processedKey = siteUtil.getSiteKey(folderMinFieldsWithId) const folderId = state.getIn(['sites', processedKey, 'folderId']) assert.equal(folderId, 1) @@ -418,7 +419,7 @@ describe('siteUtil', function () { }) let sites = {} sites[oldSiteKey] = oldSiteDetail.toJS() - const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteKey) const expectedSiteKey = siteUtil.getSiteKey(expectedSiteDetail) let expectedSites = {} expectedSites[expectedSiteKey] = expectedSiteDetail.toJS() @@ -443,7 +444,7 @@ describe('siteUtil', function () { }) let sites = {} sites[oldSiteKey] = oldSiteDetail.toJS() - const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteKey) const expectedSites = {} const expectedSiteKey = siteUtil.getSiteKey(newSiteDetail) expectedSites[expectedSiteKey] = newSiteDetail.set('order', 0).set('objectId', undefined).toJS() @@ -455,6 +456,7 @@ describe('siteUtil', function () { location: testUrl1, title: 'a brave title' }) + const oldSiteKey = siteUtil.getSiteKey(oldSiteDetail) const newSiteDetail = Immutable.fromJS({ tags: [siteTags.BOOKMARK], location: testUrl1, @@ -462,7 +464,7 @@ describe('siteUtil', function () { }) const sites = Immutable.fromJS([oldSiteDetail]) - const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteKey) const expectedSites = sites assert.deepEqual( state.getIn(['sites', 0, 'lastAccessedTime']), @@ -501,7 +503,7 @@ describe('siteUtil', function () { }) let sites = {} sites[oldSiteKey] = oldSiteDetail.toJS() - const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteKey) const expectedSiteKey = siteUtil.getSiteKey(expectedSiteDetail) let expectedSites = {} expectedSites[expectedSiteKey] = expectedSiteDetail.set('objectId', undefined).toJS() @@ -536,14 +538,14 @@ describe('siteUtil', function () { title: 'new title', customTitle: 'new customTitle' }) - const siteKey = siteUtil.getSiteKey(oldSiteDetail) + const oldSiteKey = siteUtil.getSiteKey(oldSiteDetail) const sites = { - [siteKey]: oldSiteDetail + [oldSiteKey]: oldSiteDetail } - const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail, true) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteKey, true) mockery.deregisterMock('./stores/appStoreRenderer') mockery.disable() - assert.equal(state.getIn(['sites', siteKey, 'skipSync']), true) + assert.equal(state.getIn(['sites', oldSiteKey, 'skipSync']), true) }) }) })