Merge pull request #7985 from luixxiul/siteInfo-refactoring

Refactor siteInfo.js with Aphrodite
brave · Apr 6, 2017 · 4337315 · 4337315
2 parents aea8ac9 + 5481104
commit 4337315
Show file tree

Hide file tree

Showing 7 changed files with 182 additions and 94 deletions.
diff --git a/app/renderer/components/messageBox.js b/app/renderer/components/messageBox.js
@@ -11,8 +11,10 @@ const appActions = require('../../../js/actions/appActions')
 const KeyCodes = require('../../common/constants/keyCodes')
 const config = require('../../../js/constants/config')
 const {makeImmutable} = require('../../common/state/immutableUtil')
+
 const {StyleSheet, css} = require('aphrodite')
 const commonStyles = require('./styles/commonStyles')
+const globalStyles = require('./styles/global')
 
 class MessageBox extends ImmutableComponent {
   constructor () {
@@ -119,6 +121,8 @@ class MessageBox extends ImmutableComponent {
         {
           this.showSuppress
             ? <SwitchControl
+              // TODO: refactor SwitchControl
+              className={css(commonStyles.noPaddingLeft)}
               rightl10nId='preventMoreAlerts'
               checkedOn={this.suppress}
               onClick={this.onSuppressChanged} />
@@ -143,21 +147,19 @@ const styles = StyleSheet.create({
   title: {
     fontWeight: 'bold',
     fontSize: '12pt',
-    marginBottom: '1em',
-    marginTop: '0.5em',
+    marginBottom: globalStyles.spacing.dialogInsideMargin,
     '-webkit-user-select': 'text'
   },
   body: {
-    marginTop: '1.5em',
+    marginTop: globalStyles.spacing.dialogInsideMargin,
     minWidth: '425px',
-    marginBottom: '1.5em',
+    marginBottom: globalStyles.spacing.dialogInsideMargin,
     '-webkit-user-select': 'text'
   },
   buttons: {
     display: 'flex',
     justifyContent: 'flex-end',
-    marginTop: '1.5em',
-    marginBottom: '0.5em'
+    marginTop: globalStyles.spacing.dialogInsideMargin
   }
 })
 

diff --git a/app/renderer/components/styles/commonStyles.js b/app/renderer/components/styles/commonStyles.js
@@ -20,13 +20,16 @@ const styles = StyleSheet.create({
     padding: '0.4em',
     width: '100%'
   },
+
+  // Dialogs
   flyoutDialog: {
     background: globalStyles.color.toolbarBackground,
     borderRadius: globalStyles.radius.borderRadius,
     boxShadow: '2px 2px 8px #3b3b3b',
     color: '#000',
     fontSize: '13px',
-    padding: '10px 30px',
+    // Issue #7949
+    padding: `${globalStyles.spacing.dialogInsideMargin} 30px`,
     position: 'absolute',
     top: globalStyles.spacing.dialogTopOffset
   },

diff --git a/js/components/siteInfo.js b/js/components/siteInfo.js
@@ -15,6 +15,10 @@ const messages = require('../constants/messages')
 const siteUtil = require('../state/siteUtil')
 const platformUtil = require('../../app/common/lib/platformUtil')
 
+const {StyleSheet, css} = require('aphrodite/no-important')
+const globalStyles = require('../../app/renderer/components/styles/global')
+const commonStyles = require('../../app/renderer/components/styles/commonStyles')
+
 class SiteInfo extends ImmutableComponent {
   constructor () {
     super()
@@ -67,81 +71,150 @@ class SiteInfo extends ImmutableComponent {
     let l10nArgs = {
       partitionNumber: this.partitionNumber
     }
+
     let secureIcon
     if (this.isSecure === true && !this.runInsecureContent) {
       // fully secure
-      secureIcon = <li><span
-        className={cx({
-          fa: true,
-          'fa-lock': true,
-          extendedValidation: this.isExtendedValidation
-        })} /><span data-l10n-id='secureConnection' /></li>
+      secureIcon =
+        <div className={css(styles.secureIcon__wrapper)}>
+          <span className={cx({
+            fa: true,
+            'fa-lock': true,
+            [css(styles.secureIcon__fa)]: true,
+            [css(styles.secureIcon__extendedValidation)]: this.isExtendedValidation
+          })} />
+          <span className={css(styles.secureIcon__label)}
+            data-test-id='secureConnection'
+            data-l10n-id='secureConnection'
+          />
+        </div>
     } else if (this.isSecure === 1 && !this.runInsecureContent) {
       // passive mixed content loaded
-      secureIcon = <li><span className='fa fa-unlock' /><span data-l10n-id='partiallySecureConnection' /></li>
+      secureIcon =
+        <div className={css(styles.secureIcon__wrapper)}>
+          <span className={cx({
+            fa: true,
+            'fa-unlock': true,
+            [css(styles.secureIcon__fa)]: true
+          })} />
+          <span className={css(styles.secureIcon__label)}
+            data-test-id='partiallySecureConnection'
+            data-l10n-id='partiallySecureConnection'
+          />
+        </div>
     } else {
       // insecure
-      secureIcon = <li><span className='fa fa-unlock' /><span data-l10n-id='insecureConnection' /></li>
+      secureIcon =
+        <div className={css(styles.secureIcon__wrapper)}>
+          <span className={cx({
+            fa: true,
+            'fa-unlock': true,
+            [css(styles.secureIcon__fa)]: true
+          })} />
+          <span className={css(styles.secureIcon__label)}
+            data-test-id='insecureConnection'
+            data-l10n-id='insecureConnection'
+          />
+        </div>
     }
 
     let partitionInfo
     if (this.partitionNumber) {
-      partitionInfo = <li><span className='fa fa-user' />
-        <span data-l10n-args={JSON.stringify(l10nArgs)} data-l10n-id='sessionInfo' /></li>
+      partitionInfo =
+        <div className={css(styles.secureIcon__wrapper)}>
+          <span className={cx({
+            fa: true,
+            'fa-user': true,
+            [css(styles.secureIcon__fa)]: true
+          })} />
+          <span className={css(styles.secureIcon__label)}
+            data-test-id='partitionNumber'
+            data-l10n-args={JSON.stringify(l10nArgs)}
+            data-l10n-id='sessionInfo'
+          />
+        </div>
     }
 
     let connectionInfo = null
     let viewCertificateButton = null
+
     // TODO(Anthony): Hide it until muon support linux
     if (!platformUtil.isLinux()) {
       viewCertificateButton =
-        <Button l10nId='viewCertificate' className='primaryButton viewCertificate' onClick={this.onViewCertificate} />
+        <div className={css(styles.flexJustifyEnd, styles.viewCertificateButton__wrapper)}>
+          <Button l10nId='viewCertificate'
+            className='primaryButton'
+            testId='viewCertificate'
+            onClick={this.onViewCertificate}
+          />
+        </div>
     }
 
     if (this.maybePhishingLocation) {
       connectionInfo =
-        <div className='connectionInfo' data-l10n-id='phishingConnectionInfo' />
+        <div className={css(styles.connectionInfo__wrapper)}>
+          <div data-l10n-id='phishingConnectionInfo' data-test-id='phishingConnectionInfo' />
+        </div>
     } else if (this.isBlockedRunInsecureContent) {
       connectionInfo =
-        <li>
-          <ul>
-            <li><span className='runInsecureContentWarning' data-l10n-id='runInsecureContentWarning' /></li>
-            <li>
-              <Button l10nId='allowRunInsecureContent' className='whiteButton allowRunInsecureContentButton' onClick={this.onAllowRunInsecureContent} />
-              <Button l10nId='dismissAllowRunInsecureContent' className='primaryButton dismissAllowRunInsecureContentButton' onClick={this.props.onHide} />
-            </li>
-          </ul>
-        </li>
+        <div className={css(styles.connectionInfo__wrapper)}>
+          <div data-test-id='runInsecureContentWarning' data-l10n-id='runInsecureContentWarning' />
+          <div className={css(styles.flexJustifyEnd, styles.viewCertificateButton__wrapper)}>
+            <Button l10nId='allowRunInsecureContent'
+              className='whiteButton'
+              testId='allowRunInsecureContentButton'
+              onClick={this.onAllowRunInsecureContent}
+            />
+            <Button l10nId='dismissAllowRunInsecureContent'
+              className='primaryButton'
+              testId='dismissAllowRunInsecureContentButton'
+              onClick={this.props.onHide}
+            />
+          </div>
+        </div>
     } else if (this.runInsecureContent) {
       connectionInfo =
-        <li>
-          <ul>
-            <li><span className='denyRunInsecureContentWarning' data-l10n-id='denyRunInsecureContentWarning' /></li>
-            <li>
-              <Button l10nId='denyRunInsecureContent' className='primaryButton denyRunInsecureContentButton' onClick={this.onDenyRunInsecureContent} />
-              <Button l10nId='dismissDenyRunInsecureContent' className='whiteButton dismissDenyRunInsecureContentButton' onClick={this.props.onHide} />
-            </li>
-          </ul>
-        </li>
+        <div className={css(styles.connectionInfo__wrapper)}>
+          <div data-test-id='denyRunInsecureContentWarning' data-l10n-id='denyRunInsecureContentWarning' />
+          <div className={css(styles.flexJustifyEnd, styles.viewCertificateButton__wrapper)}>
+            <Button l10nId='dismissDenyRunInsecureContent'
+              className='whiteButton'
+              testId='dismissDenyRunInsecureContentButton'
+              onClick={this.props.onHide}
+            />
+            <Button l10nId='denyRunInsecureContent'
+              className='primaryButton'
+              testId='denyRunInsecureContentButton'
+              onClick={this.onDenyRunInsecureContent}
+            />
+          </div>
+        </div>
     } else if (this.isSecure === true) {
       connectionInfo =
-        <div>
-          <div className='connectionInfo' data-l10n-id='secureConnectionInfo' />
+        <div className={css(styles.connectionInfo__wrapper)}>
+          <div data-l10n-id='secureConnectionInfo' />
           {viewCertificateButton}
         </div>
     } else if (this.isSecure === 1) {
       connectionInfo =
-        <div>
-          <div className='connectionInfo' data-l10n-id='partiallySecureConnectionInfo' />
+        <div className={css(styles.connectionInfo__wrapper)}>
+          <div data-l10n-id='partiallySecureConnectionInfo' />
           {viewCertificateButton}
         </div>
     } else {
       connectionInfo =
-        <div className='connectionInfo' data-l10n-id='insecureConnectionInfo' />
+        <div className={css(styles.connectionInfo__wrapper)}>
+          <div data-l10n-id='insecureConnectionInfo' />
+        </div>
     }
 
     return <Dialog onHide={this.props.onHide} className='siteInfo' isClickDismiss>
-      <ul onClick={(e) => e.stopPropagation()}>
+      <div onClick={(e) => e.stopPropagation()}
+        className={cx({
+          [css(commonStyles.flyoutDialog)]: true,
+          [css(styles.siteInfo__wrapper)]: true,
+          [css(styles.siteInfo__wrapper__large)]: (this.isBlockedRunInsecureContent || this.runInsecureContent)
+        })}>
         {
           secureIcon
         }
@@ -151,7 +224,7 @@ class SiteInfo extends ImmutableComponent {
         {
           connectionInfo
         }
-      </ul>
+      </div>
     </Dialog>
   }
 }
@@ -161,4 +234,46 @@ SiteInfo.propTypes = {
   onHide: React.PropTypes.func
 }
 
+const styles = StyleSheet.create({
+  flexJustifyEnd: {
+    display: 'flex',
+    justifyContent: 'flex-end'
+  },
+
+  secureIcon__wrapper: {
+    display: 'flex',
+    alignItems: 'center'
+  },
+  secureIcon__fa: {
+    position: 'absolute'
+  },
+  secureIcon__extendedValidation: {
+    color: '#008000'
+  },
+  secureIcon__label: {
+    position: 'relative',
+    left: globalStyles.spacing.dialogInsideMargin
+  },
+
+  viewCertificateButton__wrapper: {
+    marginTop: globalStyles.spacing.dialogInsideMargin
+  },
+
+  connectionInfo__wrapper: {
+    display: 'flex',
+    flexFlow: 'column nowrap',
+    margin: `${globalStyles.spacing.dialogInsideMargin} 0 0 ${globalStyles.spacing.dialogInsideMargin}`
+  },
+
+  siteInfo__wrapper: {
+    maxHeight: '300px',
+    maxWidth: '400px',
+    width: 'auto'
+  },
+  siteInfo__wrapper__large: {
+    // temporary workaround
+    maxWidth: '500px'
+  }
+})
+
 module.exports = SiteInfo
diff --git a/less/dialogs.less b/less/dialogs.less
@@ -83,7 +83,3 @@
     }
   }
 }
-.connectionInfo {
-  max-width: 400px;
-  padding: 10px;
-}
diff --git a/less/forms.less b/less/forms.less
@@ -18,34 +18,6 @@ select {
   top: @dialogTopOffset;
 }
 
-.siteInfo {
-  >ul {
-    .flyoutDialog;
-    text-align: left;
-    width: auto;
-
-    li {
-      list-style-type: none;
-      padding: 10px 0;
-
-      .fa {
-        padding-right: 10px;
-      }
-      &.extendedValidation {
-        color: green;
-      }
-
-      ul {
-        max-height: 300px;
-        overflow-y: auto;
-        margin-top: -20px;
-        padding: 0 10px;
-        max-width: 400px;
-      }
-    }
-  }
-}
-
 .clearBrowsingDataPanel {
   .clearBrowsingData {
     .flyoutDialog;

diff --git a/test/lib/selectors.js b/test/lib/selectors.js
@@ -91,10 +91,10 @@ module.exports = {
   paymentsWelcomePage: '[data-test-id="paymentsMessage"]',
   autofillAddressPanel: '.autofillAddressPanel',
   autofillCreditCardPanel: '.autofillCreditCardPanel',
-  allowRunInsecureContentButton: '.allowRunInsecureContentButton',
-  dismissAllowRunInsecureContentButton: '.dismissAllowRunInsecureContentButton',
-  denyRunInsecureContentButton: '.denyRunInsecureContentButton',
-  dismissDenyRunInsecureContentButton: '.dismissDenyRunInsecureContentButton',
+  allowRunInsecureContentButton: '[data-test-id="allowRunInsecureContentButton"]',
+  dismissAllowRunInsecureContentButton: '[data-test-id="dismissAllowRunInsecureContentButton"]',
+  denyRunInsecureContentButton: '[data-test-id="denyRunInsecureContentButton"]',
+  dismissDenyRunInsecureContentButton: '[data-test-id="dismissDenyRunInsecureContentButton"]',
   tabsToolbar: '.tabsToolbar',
   hamburgerMenu: '.menuButton',
   contextMenu: '.contextMenu',