Misc polish

- Fix urlbarForm_noScriptDisabled - Fix test errors - Apply navigationBarButtonContainer to browserAction - Apply BEM style to browserActionBadge - Apply align-items to the button separator - Rename urlbarForm_isPublisherButtonEnabled to urlbarForm_urlBarEnd - Remove animation:none from navigationBar__urlBarEnd - Split URLBarIcon
brave · Jul 12, 2017 · 7d71108 · 7d71108
1 parent 2d7d7fa
commit 7d71108
Show file tree

Hide file tree

Showing 15 changed files with 135 additions and 117 deletions.
diff --git a/app/renderer/components/common/browserButton.js b/app/renderer/components/common/browserButton.js
@@ -209,13 +209,12 @@ const styles = StyleSheet.create({
     }
   },
 
+  // This should be included in navigationBarButtonContainer
   browserButton_extensionItem: {
-    WebkitAppRegion: 'no-drag',
     backgroundSize: 'contain',
+    backgroundRepeat: 'no-repeat',
     height: '17px',
-    margin: '4px 0 0 0',
-    opacity: '0.85',
-    backgroundRepeat: 'no-repeat'
+    opacity: '0.85'
   },
 
   browserButton_groupedItem: {

diff --git a/app/renderer/components/navigation/browserAction.js b/app/renderer/components/navigation/browserAction.js
@@ -5,11 +5,12 @@
 const React = require('react')
 const electron = require('electron')
 const ipc = electron.ipcRenderer
-const {StyleSheet, css} = require('aphrodite')
+const {StyleSheet} = require('aphrodite/no-important')
 const Immutable = require('immutable')
 
 // Components
 const ReduxComponent = require('../reduxComponent')
+const NavigationBarButtonContainer = require('./buttons/navigationBarButtonContainer')
 const {BrowserButton} = require('../common/browserButton')
 const BrowserActionBadge = require('./browserActionBadge')
 
@@ -81,7 +82,9 @@ class BrowserAction extends React.Component {
 
   render () {
     // TODO(bridiver) should have some visual notification of hover/press
-    return <div className={css(styles.browserActionButton)}>
+    return <NavigationBarButtonContainer
+      isSquare
+      containerFor={styles.browserActionButton}>
       <BrowserButton
         extensionItem
         l10nId='browserActionButton'
@@ -101,7 +104,7 @@ class BrowserAction extends React.Component {
         ? <BrowserActionBadge text={this.props.text} color={this.props.color} />
         : null
       }
-    </div>
+    </NavigationBarButtonContainer>
   }
 }
 

diff --git a/app/renderer/components/navigation/browserActionBadge.js b/app/renderer/components/navigation/browserActionBadge.js
@@ -3,7 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const React = require('react')
-const {StyleSheet, css} = require('aphrodite')
+const {StyleSheet, css} = require('aphrodite/no-important')
 const ImmutableComponent = require('../immutableComponent')
 const globalStyles = require('../styles/global')
 
@@ -30,11 +30,11 @@ class BrowserActionBadge extends ImmutableComponent {
       ref='badge'
       className={css(
         styles.browserActionBadge,
-        this.centered && styles.centered,
+        this.centered && styles.browserActionBadge_centered,
         // delay badge show-up.
         // this is also set for braveryPanel badge
         // in a way that both can appear at the same time.
-        styles.subtleShowUp
+        styles.browserActionBadge_subtleShowUp
       )}
       style={{backgroundColor: this.props.color || globalStyles.color.braveMediumOrange}}
       >{this.props.text}</div>
@@ -56,14 +56,16 @@ const styles = StyleSheet.create({
     minWidth: '10px',
     userSelect: 'none'
   },
-  centered: {
+
+  browserActionBadge_centered: {
     left: '50%',
     transform: 'translateX(-50%)',
     maxWidth: 'calc(100% - 5px)',
     overflow: 'hidden',
     textOverflow: 'ellipsis'
   },
-  subtleShowUp: globalStyles.animations.subtleShowUp
+
+  browserActionBadge_subtleShowUp: globalStyles.animations.subtleShowUp
 })
 
 module.exports = BrowserActionBadge
diff --git a/app/renderer/components/navigation/buttons/navigationBarButtonContainer.js b/app/renderer/components/navigation/buttons/navigationBarButtonContainer.js
@@ -17,7 +17,7 @@ class NavigationBarButtonContainer extends React.Component {
   render () {
     return (
       <div className={css(
-        // Used for bookmarkButtonContainer, PublisherToggle, noScriptInfo, and UrlBarIcon.
+        // Used for bookmarkButtonContainer, PublisherToggle, noScriptInfo, UrlBarIcon, and BrowserAction
         this.props.isSquare && styles.container_square,
 
         // isNested and isStandalone should not be called at the same time

diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js
@@ -152,7 +152,7 @@ class NavigationBar extends React.Component {
         : null
       }
       {
-        this.props.showHomeButton
+        !this.props.titleMode && this.props.showHomeButton
         ? (
           <NavigationBarButtonContainer isStandalone onNavigationBarChrome>
             <HomeButton />
@@ -173,7 +173,7 @@ class NavigationBar extends React.Component {
       }
       <UrlBar titleMode={this.props.titleMode} />
       {
-        this.props.showPublisherToggle
+        !this.props.titleMode && this.props.showPublisherToggle
         ? (
           <NavigationBarButtonContainer isSquare isNested
             containerFor={styles.navigationBar__urlBarEnd}
@@ -228,14 +228,11 @@ const styles = StyleSheet.create({
 
  // Applies for the end urlBar nested button
  // currently for PublisherToggle
+ // TODO: remove animation of the toggle when titleMode is enabled
   navigationBar__urlBarEnd: {
     borderLeft: 'none',
     borderTopLeftRadius: 0,
-    borderBottomLeftRadius: 0,
-
-    // TODO (Suguru): Refactor navigationBar.less to remove !important.
-    // See the wildcard style under '#navigationBar'.
-    animation: 'none !important'
+    borderBottomLeftRadius: 0
   }
 })
 

diff --git a/app/renderer/components/navigation/navigator.js b/app/renderer/components/navigation/navigator.js
@@ -488,7 +488,7 @@ const styles = StyleSheet.create({
 
   topLevelEndButtons: {
     display: 'flex',
-    flexDirection: 'row',
+    alignItems: 'center',
     position: 'relative'
   },
 
@@ -504,7 +504,8 @@ const styles = StyleSheet.create({
   topLevelEndButtons__buttonSeparator: {
     width: '1px',
     borderLeft: '1px solid #e2e2e2',
-    margin: '4px 3px 4px 3px'
+    margin: '0 3px',
+    height: globalStyles.navigationBar.urlbarForm.height
   },
 
   braveMenuButton: {

diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js
@@ -394,25 +394,27 @@ class UrlBar extends React.Component {
     return ''
   }
 
-  get UrlBarIcon () {
-    return <UrlBarIcon
-      activateSearchEngine={this.props.activateSearchEngine}
-      active={this.props.isActive}
-      isSecure={this.props.isSecure}
-      isHTTPPage={this.props.isHTTPPage}
-      loading={this.props.loading}
-      location={this.props.location}
-      searchSelectEntry={this.props.searchSelectEntry}
-      title={this.props.title}
-      titleMode={this.props.titleMode}
-      isSearching={this.props.location !== this.props.urlbarLocation}
-      activeTabShowingMessageBox={this.props.activeTabShowingMessageBox}
-    />
+  get URLBarIcon () {
+    return <NavigationBarButtonContainer isSquare>
+      <UrlBarIcon
+        activateSearchEngine={this.props.activateSearchEngine}
+        active={this.props.isActive}
+        isSecure={this.props.isSecure}
+        isHTTPPage={this.props.isHTTPPage}
+        loading={this.props.loading}
+        location={this.props.location}
+        searchSelectEntry={this.props.searchSelectEntry}
+        title={this.props.title}
+        titleMode={this.props.titleMode}
+        isSearching={this.props.location !== this.props.urlbarLocation}
+        activeTabShowingMessageBox={this.props.activeTabShowingMessageBox}
+      />
+    </NavigationBarButtonContainer>
   }
 
   // BEM Level: urlbarForm__titleBar
   get titleBar () {
-    return <div id='titleBar' className={css(styles.titleBar)}>
+    return <div id='titleBar' data-test-id='titleBar' className={css(styles.titleBar)}>
       <span className={css(styles.titleBar__host)}>{this.props.hostValue}</span>
       <span>{this.props.hostValue && this.titleValue ? ' | ' : ''}</span>
       <span>{this.titleValue}</span>
@@ -457,7 +459,7 @@ class UrlBar extends React.Component {
     return <legend className={css(
       styles.legend,
       !!this.props.isFocused && styles.legend_isFocused,
-      this.props.isPublisherButtonEnabled && styles.legend_isPublisherButtonEnabled
+      this.props.publisherButtonVisible && styles.legend_urlBarEnd
     )} />
   }
 
@@ -566,13 +568,12 @@ class UrlBar extends React.Component {
     return <form
       className={cx({
         urlbarForm: true,
-        [css(styles.urlbarForm, this.props.isWideURLbarEnabled && styles.urlbarForm_wide, this.props.titleMode && styles.urlbarForm_titleMode, !this.props.titleMode && styles.urlbarForm_notTitleMode, !this.props.showNoScriptInfo && styles.urlbarForm_noScriptDisabled, this.props.publisherButtonVisible && styles.urlbarForm_isPublisherButtonEnabled)]: true
+        // currently publisherButtonVisible is the only element under urlbarForm_urlBarEnd
+        [css(styles.urlbarForm, this.props.isWideURLbarEnabled && styles.urlbarForm_wide, this.props.titleMode && styles.urlbarForm_titleMode, !this.props.titleMode && styles.urlbarForm_notTitleMode, !this.props.showNoScriptInfo && styles.urlbarForm_noScriptDisabled, this.props.publisherButtonVisible && styles.urlbarForm_urlBarEnd)]: true
       })}
       action='#'
       id='urlbar'>
-      <NavigationBarButtonContainer isSquare>
-        <UrlBarIcon titleMode={this.props.titleMode} />
-      </NavigationBarButtonContainer>
+      {this.URLBarIcon}
       {
         this.props.titleMode
         ? this.titleBar
@@ -647,8 +648,8 @@ const styles = StyleSheet.create({
     paddingRight: '10px'
   },
 
-  // ref: navigationBar__buttonContainer_addPublisherButtonContainer on publisherToggle.js
-  urlbarForm_isPublisherButtonEnabled: {
+  // ref: navigationBar__urlBarEnd on navigationBarButtonContainer.js
+  urlbarForm_urlBarEnd: {
     borderTopRightRadius: 0,
     borderBottomRightRadius: 0
   },
@@ -719,7 +720,7 @@ const styles = StyleSheet.create({
     }
   },
 
-  legend_isPublisherButtonEnabled: {
+  legend_urlBarEnd: {
     ':before': {
       borderRadius: 0
     }

diff --git a/test/app/renderer/components/messageBoxTest.js b/test/app/renderer/components/messageBoxTest.js
@@ -2,7 +2,7 @@
 
 const Brave = require('../../../lib/brave')
 const {
-  urlInput, backButton, forwardButton,
+  urlInput, backButton, forwardButton, forwardButtonDisabled,
   msgBoxSuppress, msgBoxSuppressTrue, msgBoxMessage, msgBoxTitle
 } = require('../../../lib/selectors')
 const assert = require('assert')
@@ -220,7 +220,7 @@ describe('MessageBox component tests', function () {
         yield this.app.client
           .windowByUrl(Brave.browserWindowUrl)
           .leftClick(backButton)
-          .waitForElementCount(forwardButton + '[disabled]', 0)
+          .waitForElementCount(forwardButtonDisabled, 0)
 
         // click forward button
         yield this.app.client

diff --git a/test/lib/selectors.js b/test/lib/selectors.js
@@ -16,9 +16,9 @@ module.exports = {
   tabPage1: '[data-tab-page="0"]',
   tabPage2: '[data-tab-page="1"]',
   closeTab: '[data-test-id="closeTabIcon"]',
-  urlbarIcon: '[data-test-id="urlbarIcon"]',
+  urlBarIcon: '[data-test-id="urlBarIcon"]',
   urlBarSuggestions: '.urlBarSuggestions',
-  titleBar: '#titleBar',
+  titleBar: '[data-test-id="titleBar"]',
   navigatorBookmarked: '[data-test-id="bookmarked"]',
   navigatorNotBookmarked: '[data-test-id="notBookmarked"]',
   bookmarksToolbar: '[data-test-id="bookmarksToolbar"]',
@@ -62,8 +62,10 @@ module.exports = {
   noScriptAllowTempButton: '[data-l10n-id="allowScriptsTemp"]',
   noScriptAllowOnceButton: '[data-l10n-id="allowScriptsOnce"]',
   noScriptAllowButton: '[data-l10n-id="allowScripts"]',
-  backButton: '.topLevelStartButtons .backButton',
-  forwardButton: '.topLevelStartButtons .forwardButton',
+  backButton: '[data-test-id="backButton"]',
+  backButtonDisabled: '[data-test-id="backButtonDisabled"]',
+  forwardButton: '[data-test-id="forwardButton"]',
+  forwardButtonDisabled: '[data-test-id="forwardButtonDisabled"]',
   reloadButton: '[data-test-id="reloadButton"]',
   homeButton: '[data-test-id="homeButton"]',
   clearBrowsingDataButton: '[data-test-id="clearBrowsingDataButton"]',