Refactors addEditBookmarHanger and addEditBookmark to redux

Resolves #8927 Auditors: @bsclifton Test Plan: - try to bookmark a page
brave · May 18, 2017 · ca78189 · ca78189
1 parent 22ce3f8
commit ca78189
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 79 deletions.
diff --git a/app/renderer/components/bookmarks/addEditBookmark.js b/app/renderer/components/bookmarks/addEditBookmark.js
diff --git a/app/renderer/components/bookmarks/addEditBookmarkHanger.js b/app/renderer/components/bookmarks/addEditBookmarkHanger.js
@@ -5,7 +5,7 @@
 const React = require('react')
 
 // Components
-const ImmutableComponent = require('../immutableComponent')
+const ReduxComponent = require('../reduxComponent')
 const Dialog = require('../../../../js/components/dialog')
 const BrowserButton = require('../common/browserButton')
 
@@ -22,7 +22,7 @@ const settings = require('../../../../js/constants/settings')
 const cx = require('../../../../js/lib/classSet')
 const siteUtil = require('../../../../js/state/siteUtil')
 const UrlUtil = require('../../../../js/lib/urlutil')
-const getSetting = require('../../../../js/settings').getSetting
+const {getSetting} = require('../../../../js/settings')
 
 const {StyleSheet, css} = require('aphrodite/no-important')
 const globalStyles = require('../styles/global')
@@ -38,7 +38,7 @@ const {
   commonFormStyles
 } = require('../common/commonForm')
 
-class AddEditBookmarkHanger extends ImmutableComponent {
+class AddEditBookmarkHanger extends React.Component {
   constructor () {
     super()
     this.onNameChange = this.onNameChange.bind(this)
@@ -51,20 +51,23 @@ class AddEditBookmarkHanger extends ImmutableComponent {
     this.onViewBookmarks = this.onViewBookmarks.bind(this)
     this.onRemoveBookmark = this.onRemoveBookmark.bind(this)
   }
+
   get bookmarkNameValid () {
     const title = this.props.currentDetail.get('title') || this.props.currentDetail.get('customTitle')
     const location = this.props.currentDetail.get('location')
     return this.isFolder
       ? (typeof title === 'string' && title.trim().length > 0)
       : (typeof location === 'string' && location.trim().length > 0)
   }
+
   get displayBookmarkName () {
     const customTitle = this.props.currentDetail.get('customTitle')
     if (customTitle !== undefined && customTitle !== '') {
       return customTitle || ''
     }
     return this.props.currentDetail.get('title') || ''
   }
+
   get heading () {
     if (this.isFolder) {
       return this.props.shouldShowLocation
@@ -77,33 +80,26 @@ class AddEditBookmarkHanger extends ImmutableComponent {
         : 'bookmarkEdit'
       : 'bookmarkAdded'
   }
+
   get isFolder () {
     // 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.
     return siteUtil.isFolder(this.props.currentDetail.set('folderId', 0))
   }
+
   setDefaultFocus () {
     this.bookmarkName.select()
     this.bookmarkName.focus()
   }
-  updateFolders (props) {
-    this.folders = siteUtil.getFolders(this.props.sites, props.currentDetail.get('folderId'))
-  }
-  componentWillMount () {
-    this.updateFolders(this.props)
-  }
-  componentWillUpdate (nextProps) {
-    if (this.props.sites !== nextProps.sites) {
-      this.updateFolders(nextProps)
-    }
-  }
+
   componentDidMount () {
     // Automatically save if this is triggered by the url star
     if (!this.props.isModal && !this.props.shouldShowLocation) {
       this.onSave(false)
     }
     this.setDefaultFocus()
   }
+
   onKeyDown (e) {
     switch (e.keyCode) {
       case KeyCodes.ENTER:
@@ -114,12 +110,15 @@ class AddEditBookmarkHanger extends ImmutableComponent {
         break
     }
   }
+
   onClose () {
     windowActions.setBookmarkDetail()
   }
+
   onClick (e) {
     e.stopPropagation()
   }
+
   onNameChange (e) {
     let currentDetail = this.props.currentDetail
     let name = e.target.value
@@ -140,6 +139,7 @@ class AddEditBookmarkHanger extends ImmutableComponent {
     }
     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') {
@@ -151,18 +151,18 @@ class AddEditBookmarkHanger extends ImmutableComponent {
     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 () {
-    const hasBookmarks = this.props.sites.find(
-      (site) => siteUtil.isBookmark(site) || siteUtil.isFolder(site)
-    )
-    if (!hasBookmarks && !getSetting(settings.SHOW_BOOKMARKS_TOOLBAR)) {
+    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.bookmarkNameValid) {
@@ -175,15 +175,37 @@ class AddEditBookmarkHanger extends ImmutableComponent {
       this.onClose()
     }
   }
+
   onRemoveBookmark () {
     const tag = this.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK
     appActions.removeSite(this.props.currentDetail, tag)
     this.onClose()
   }
+
   onViewBookmarks () {
     this.onClose()
     appActions.createTabRequested({url: 'about:bookmarks'})
   }
+
+  mergeProps (state, dispatchProps, ownProps) {
+    const currentWindow = state.get('currentWindow')
+    const bookmarkDetail = currentWindow.get('bookmarkDetail')
+
+    const props = {}
+    props.currentDetail = bookmarkDetail.get('currentDetail') // TODO (nejc) improve, primitives only
+    props.originalDetail = bookmarkDetail.get('originalDetail') // TODO (nejc) improve, primitives only
+    props.destinationDetail = bookmarkDetail.get('destinationDetail') // TODO (nejc) improve, primitives only
+    props.shouldShowLocation = bookmarkDetail.get('shouldShowLocation')
+    props.withHomeButton = getSetting(settings.SHOW_HOME_BUTTON)
+    props.folders = siteUtil.getFolders(state.get('sites'), props.currentDetail.get('folderId'))
+    props.hasBookmarks = state.get('sites').find(
+      (site) => siteUtil.isBookmark(site) || siteUtil.isFolder(site)
+    )
+    props.isModal = ownProps.isModal
+
+    return props
+  }
+
   render () {
     const arrowUp = this.props.isModal
       ? null
@@ -264,7 +286,7 @@ class AddEditBookmarkHanger extends ImmutableComponent {
                 onChange={this.onParentFolderChange} >
                 <option value='0' data-l10n-id='bookmarksToolbar' />
                 {
-                  this.folders.map((folder) => <option value={folder.folderId}>{folder.label}</option>)
+                  this.props.folders.map((folder) => <option value={folder.folderId}>{folder.label}</option>)
                 }
               </CommonFormDropdown>
             </div>
@@ -368,4 +390,4 @@ const styles = StyleSheet.create({
   }
 })
 
-module.exports = AddEditBookmarkHanger
+module.exports = ReduxComponent.connect(AddEditBookmarkHanger)
diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js
@@ -28,7 +28,6 @@ const PublisherToggle = require('../publisherToggle')
 // State
 const tabState = require('../../../common/state/tabState')
 const frameStateUtil = require('../../../../js/state/frameStateUtil')
-const menuBarState = require('../../../common/state/menuBarState')
 
 class NavigationBar extends React.Component {
   constructor (props) {
@@ -142,17 +141,16 @@ class NavigationBar extends React.Component {
     const props = {}
 
     props.navbar = navbar
-    props.sites = state.get('sites')
+    props.sites = state.get('sites') // TODO(nejc) remove, primitives only
     props.activeFrameKey = activeFrameKey
     props.location = location
     props.title = title
     props.partitionNumber = activeFrame.get('partitionNumber') || 0
     props.isSecure = activeFrame.getIn(['security', 'isSecure'])
     props.loading = loading
-    props.bookmarkDetail = bookmarkDetail
+    props.showBookmarkHanger = bookmarkDetail && bookmarkDetail.get('isBookmarkHanger')
     props.mouseInTitlebar = mouseInTitlebar
     props.settings = state.get('settings')
-    props.menubarVisible = menuBarState.isMenuBarVisible(currentWindow)
     props.siteSettings = state.get('siteSettings')
     props.synopsis = state.getIn(['publisherInfo', 'synopsis']) || new Immutable.Map()
     props.activeTabShowingMessageBox = activeTabShowingMessageBox
@@ -176,14 +174,8 @@ class NavigationBar extends React.Component {
         titleMode: this.props.titleMode
       })}>
       {
-        this.props.bookmarkDetail && this.props.bookmarkDetail.get('isBookmarkHanger')
-        ? <AddEditBookmarkHanger sites={this.props.sites}
-          currentDetail={this.props.bookmarkDetail.get('currentDetail')}
-          originalDetail={this.props.bookmarkDetail.get('originalDetail')}
-          destinationDetail={this.props.bookmarkDetail.get('destinationDetail')}
-          shouldShowLocation={this.props.bookmarkDetail.get('shouldShowLocation')}
-          withHomeButton={getSetting(settings.SHOW_HOME_BUTTON)}
-          />
+        this.props.showBookmarkHanger
+        ? <AddEditBookmarkHanger />
         : null
       }
       {
@@ -234,7 +226,6 @@ class NavigationBar extends React.Component {
       <UrlBar
         titleMode={this.props.titleMode}
         onStop={this.onStop}
-        menubarVisible={this.props.menubarVisible}
         />
       {
         isSourceAboutUrl(this.props.location)

diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js
@@ -26,6 +26,7 @@ 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 urlParse = require('../../../common/urlParse')
@@ -513,6 +514,7 @@ class UrlBar extends React.Component {
     props.autocompleteEnabled = urlbar.getIn(['suggestions', 'autocompleteEnabled'])
     props.searchURL = searchURL
     props.searchShortcut = searchShortcut
+    props.menubarVisible = menuBarState.isMenuBarVisible(currentWindow)
 
     return props
   }

diff --git a/js/components/main.js b/js/components/main.js
@@ -31,7 +31,7 @@ const ImportBrowserDataPanel = require('../../app/renderer/components/importBrow
 const WidevinePanel = require('../../app/renderer/components/widevinePanel')
 const AutofillAddressPanel = require('../../app/renderer/components/autofill/autofillAddressPanel')
 const AutofillCreditCardPanel = require('../../app/renderer/components/autofill/autofillCreditCardPanel')
-const AddEditBookmark = require('../../app/renderer/components/bookmarks/addEditBookmark')
+const AddEditBookmarkHanger = require('../../app/renderer/components/bookmarks/addEditBookmarkHanger')
 const LoginRequired = require('../../app/renderer/components/loginRequired')
 const ReleaseNotes = require('../../app/renderer/components/releaseNotes')
 const BookmarksToolbar = require('../../app/renderer/components/bookmarks/bookmarksToolbar')
@@ -784,17 +784,13 @@ class Main extends ImmutableComponent {
         }
         {
           this.props.windowState.get('bookmarkDetail') && !this.props.windowState.getIn(['bookmarkDetail', 'isBookmarkHanger'])
-          ? <AddEditBookmark
-            sites={appStateSites}
-            currentDetail={this.props.windowState.getIn(['bookmarkDetail', 'currentDetail'])}
-            originalDetail={this.props.windowState.getIn(['bookmarkDetail', 'originalDetail'])}
-            destinationDetail={this.props.windowState.getIn(['bookmarkDetail', 'destinationDetail'])}
-            shouldShowLocation={this.props.windowState.getIn(['bookmarkDetail', 'shouldShowLocation'])} />
+          ? <AddEditBookmarkHanger isModal />
           : null
         }
         {
           noScriptIsVisible
-            ? <NoScriptInfo frameProps={activeFrame}
+            ? <NoScriptInfo
+              frameProps={activeFrame}
               onHide={this.onHideNoScript} />
             : null
         }