Refactor about:passwords with Aphrodite

Closes #7335 - passwords.less is removed - PasswordsTh, PasswordsTr (HeadTr), and PasswordsTd (ActionsTd) are introduced - 'fa fa-times' is added to appIcons as 'remove' - appIcons are alphabetized Auditors: Test Plan: 1. Remember a password by logging in GitHub 2. Open about:passwords 3. Make sure styling is not broken
brave · Feb 24, 2017 · 80b8a44 · 80b8a44
1 parent d9234b5
commit 80b8a44
Show file tree

Hide file tree

Showing 4 changed files with 172 additions and 114 deletions.
diff --git a/app/renderer/components/styles/global.js b/app/renderer/components/styles/global.js
@@ -157,14 +157,16 @@ const globalStyles = {
     tabTitle: '12px'
   },
   appIcons: {
-    loading: 'fa fa-spinner fa-spin',
-    defaultIcon: 'fa fa-file-o',
+    clipboard: 'fa fa-clipboard',
     closeTab: 'fa fa-times-circle',
-    private: 'fa fa-eye',
+    defaultIcon: 'fa fa-file-o',
+    loading: 'fa fa-spinner fa-spin',
     newSession: 'fa fa-user',
-    volumeOn: 'fa fa-volume-up',
+    private: 'fa fa-eye',
+    refresh: 'fa fa-refresh',
+    remove: 'fa fa-times',
     volumeOff: 'fa fa-volume-off',
-    refresh: 'fa fa-refresh'
+    volumeOn: 'fa fa-volume-up'
   }
 }
 

diff --git a/js/about/passwords.js b/js/about/passwords.js
@@ -9,9 +9,52 @@ const aboutActions = require('./aboutActions')
 
 const ipc = window.chrome.ipcRenderer
 
-require('../../less/about/passwords.less')
+const {StyleSheet, css} = require('aphrodite/no-important')
+const globalStyles = require('../../app/renderer/components/styles/global')
+
+require('../../less/about/common.less')
 require('../../node_modules/font-awesome/css/font-awesome.css')
 
+class PasswordsTh extends React.Component {
+  render () {
+    return <th className={css(styles.passwordsTh)} {...this.props} />
+  }
+}
+
+class PasswordsTr extends React.Component {
+  render () {
+    const className = css(
+      styles.passwordsTr,
+      this.props['data-isHead'] && styles.isHead
+    )
+
+    return <tr className={className} {...this.props} />
+  }
+}
+
+class HeadTr extends React.Component {
+  render () {
+    return <PasswordsTr data-isHead='true' {...this.props} />
+  }
+}
+
+class PasswordsTd extends React.Component {
+  render () {
+    const className = css(
+      styles.passwordsTd,
+      this.props['data-isAction'] && styles.isAction
+    )
+
+    return <td className={className} {...this.props} />
+  }
+}
+
+class ActionsTd extends React.Component {
+  render () {
+    return <PasswordsTd data-isAction='true' {...this.props} />
+  }
+}
+
 class SiteItem extends React.Component {
   constructor () {
     super()
@@ -23,13 +66,18 @@ class SiteItem extends React.Component {
   }
 
   render () {
-    return <tr className='passwordItem'>
-      <td className='passwordActions'>
-        <span className='passwordAction fa fa-times' title='Remove site'
-          onClick={this.onDelete} />
-      </td>
-      <td className='passwordOrigin'>{this.props.site}</td>
-    </tr>
+    return <PasswordsTr data-test-id='passwordItem'>
+      <ActionsTd>
+        <span className={css(styles.passwordAction)}>
+          <span className={globalStyles.appIcons.remove}
+            data-test-id='passwordAction'
+            title='Remove site'
+            onClick={this.onDelete}
+            style={{backgroundColor: 'inherit'}} />
+        </span>
+      </ActionsTd>
+      <PasswordsTd data-test-id='passwordOrigin'>{this.props.site}</PasswordsTd>
+    </PasswordsTr>
   }
 }
 
@@ -84,21 +132,31 @@ class PasswordItem extends React.Component {
 
   render () {
     const password = this.props.password
-    return <tr className='passwordItem'>
-      <td className='passwordActions'>
-        <span className='passwordAction fa fa-times' title='Delete password'
-          onClick={this.onDelete} />
-      </td>
-      <td className='passwordOrigin'>{password.get('origin')}</td>
-      <td className='passwordUsername'>{password.get('username')}</td>
-      <td className='passwordPlaintext'>
+    return <PasswordsTr data-test-id='passwordItem'>
+      <ActionsTd data-test-id='passwordActions'>
+        <span className={css(styles.passwordAction)}>
+          <span className={globalStyles.appIcons.remove}
+            data-test-id='passwordAction'
+            title='Delete password'
+            onClick={this.onDelete}
+            style={{backgroundColor: 'inherit'}} />
+        </span>
+      </ActionsTd>
+      <PasswordsTd data-test-id='passwordOrigin'>{password.get('origin')}</PasswordsTd>
+      <PasswordsTd data-test-id='passwordUsername'>{password.get('username')}</PasswordsTd>
+      <PasswordsTd data-test-id='passwordPlaintext'>
         {'*'.repeat(password.get('encryptedPassword').length)}
-      </td>
-      <td className='passwordActions'>
-        <span className='passwordAction fa fa-clipboard' title='Copy password to clipboard'
-          onClick={this.onCopy} />
-      </td>
-    </tr>
+      </PasswordsTd>
+      <ActionsTd data-test-id='passwordActions'>
+        <span className={css(styles.passwordAction)}>
+          <span className={globalStyles.appIcons.clipboard}
+            data-test-id='passwordAction'
+            title='Copy password to clipboard'
+            onClick={this.onCopy}
+            style={{backgroundColor: 'inherit'}} />
+        </span>
+      </ActionsTd>
+    </PasswordsTr>
   }
 }
 
@@ -154,15 +212,15 @@ class AboutPasswords extends React.Component {
     ? null
     : <div>
       <h2 data-l10n-id='savedPasswords' />
-      <div className='passwordsPageContent'>
-        <table className='passwordsList'>
+      <div className={css(styles.passwordsPageContent)}>
+        <table className={css(styles.passwordsList)}>
           <thead>
-            <tr>
-              <th />
-              <th data-l10n-id='passwordsSite' />
-              <th data-l10n-id='passwordsUsername' />
-              <th data-l10n-id='passwordsPassword' />
-            </tr>
+            <HeadTr>
+              <PasswordsTh />
+              <PasswordsTh data-l10n-id='passwordsSite' />
+              <PasswordsTh data-l10n-id='passwordsUsername' />
+              <PasswordsTh data-l10n-id='passwordsPassword' />
+            </HeadTr>
           </thead>
           <tbody>
             {
@@ -173,8 +231,10 @@ class AboutPasswords extends React.Component {
             }
           </tbody>
         </table>
-        <div className='passwordsPageFooter'>
-          <span data-l10n-id='clearPasswords'
+        <div className={css(styles.passwordsPageFooter)}>
+          <span className={css(styles.passwordsPageFooterClear)}
+            data-test-id='passwordsPageFooterClear'
+            data-l10n-id='clearPasswords'
             onClick={this.onClear} />
         </div>
       </div>
@@ -184,8 +244,8 @@ class AboutPasswords extends React.Component {
     ? null
     : <div>
       <h2 data-l10n-id='passwordSites' />
-      <div className='passwordsPageContent'>
-        <table className='passwordsList'>
+      <div className={css(styles.passwordsPageContent)}>
+        <table className={css(styles.passwordsList)}>
           <tbody>
             {
               this.state.disabledSiteDetails.map((item, site) =>
@@ -196,9 +256,9 @@ class AboutPasswords extends React.Component {
       </div>
     </div>
 
-    return <div className='passwordsPage'>
+    return <div className={css(styles.passwordsPage)}>
       <h1 data-l10n-id='passwordsTitle' />
-      <div className='passwordInstructions' data-l10n-id='passwordDisableInstructions' />
+      <div className={css(styles.passwordInstructions)} data-l10n-id='passwordDisableInstructions' />
       {
         this.isPasswordsEmpty && this.isSitesEmpty
           ? <div data-l10n-id='noPasswordsSaved' />
@@ -208,4 +268,72 @@ class AboutPasswords extends React.Component {
   }
 }
 
+const itemPadding = '8px'
+
+const styles = StyleSheet.create({
+  passwordAction: {
+    cursor: 'pointer',
+    padding: itemPadding,
+
+    // lighten(@highlightBlue, 20%);
+    ':hover': {
+      backgroundColor: '#9cd4fe'
+    }
+  },
+  passwordsPage: {
+    margin: '20px'
+  },
+  passwordsPageContent: {
+    borderTop: `1px solid ${globalStyles.color.chromeBorderColor}`
+  },
+  passwordsList: {
+    paddingTop: '10px',
+    overflow: 'hidden'
+  },
+  passwordInstructions: {
+    borderTop: `1px solid ${globalStyles.color.chromeBorderColor}`,
+    paddingTop: '10px',
+    paddingBottom: '20px',
+    fontSize: '18px',
+    color: 'grey'
+  },
+  passwordsPageFooter: {
+    padding: '10px',
+    marginBottom: '20px'
+  },
+  passwordsPageFooterClear: {
+    color: 'grey',
+    cursor: 'pointer',
+    textDecoration: 'underline'
+  },
+
+  passwordsTh: {
+    padding: itemPadding,
+    textAlign: 'left'
+  },
+  passwordsTr: {
+    cursor: 'default',
+    overflow: 'hidden',
+    whiteSpace: 'nowrap',
+    padding: '12px',
+    WebkitUserSelect: 'none',
+
+    // lighten(@highlightBlue, 30%);
+    ':hover': {
+      backgroundColor: '#ceeaff'
+    }
+  },
+  isHead: {
+    ':hover': {
+      backgroundColor: 'inherit'
+    }
+  },
+  passwordsTd: {
+    padding: itemPadding
+  },
+  isAction: {
+    padding: 0
+  }
+})
+
 module.exports = <AboutPasswords />
diff --git a/less/about/passwords.less b/less/about/passwords.less
diff --git a/test/components/notificationBarTest.js b/test/components/notificationBarTest.js
@@ -116,7 +116,7 @@ describe('notificationBar', function () {
       }).click('button=Yes')
       .tabByIndex(0)
       .loadUrl('about:passwords')
-      .waitForExist('tr.passwordItem')
+      .waitForExist('[data-test-id="passwordItem"]')
       .windowByUrl(Brave.browserWindowUrl)
       .tabByIndex(0)
       .loadUrl(this.loginUrl4)