Use switchControl text properties instead of sibling label elements

closes brave#8243 as this ensures all remaining uses of switchControl gain the built-in label click handler. covers the case-sensitivity toggle in findbar, as well as all the switches on the preferences page.
dfperry5 · Aug 18, 2017 · 62099e3 · 62099e3
1 parent c072e1f
commit 62099e3
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 131 deletions.
diff --git a/app/renderer/components/common/settings.js b/app/renderer/components/common/settings.js
@@ -90,8 +90,8 @@ class SettingCheckbox extends ImmutableComponent {
 
     const labelClass = cx({
       [css(settingCheckboxStyles.label)]: this.props.small,
-      [this.props.labelClassName]: !!this.props.labelClassName
-
+      [this.props.labelClassName]: !!this.props.labelClassName,
+      [css(settingCheckboxStyles.expansiveRightText)]: !this.props.compact
     })
 
     return <div {...props} data-test-id={this.props.dataTestId}>
@@ -101,11 +101,9 @@ class SettingCheckbox extends ImmutableComponent {
         onClick={this.onClick}
         checkedOn={this.props.checked !== undefined ? this.props.checked : getSetting(this.props.prefKey, this.props.settings)}
         className={this.props.switchClassName}
-      />
-      <label className={labelClass}
-        data-l10n-id={this.props.dataL10nId}
-        data-l10n-args={this.props.dataL10nArgs}
-        htmlFor={this.props.prefKey}
+        customRightTextClassName={labelClass}
+        rightl10nId={this.props.dataL10nId}
+        rightl10nArgs={this.props.dataL10nArgs}
       />
       {this.props.options}
     </div>
@@ -115,6 +113,9 @@ class SettingCheckbox extends ImmutableComponent {
 const settingCheckboxStyles = StyleSheet.create({
   label: {
     fontSize: 'smaller'
+  },
+  expansiveRightText: {
+    paddingLeft: '9px'
   }
 })
 

diff --git a/app/renderer/components/main/findbar.js b/app/renderer/components/main/findbar.js
@@ -4,6 +4,7 @@
 
 const React = require('react')
 const Immutable = require('immutable')
+const {StyleSheet, css} = require('aphrodite')
 
 // Components
 const ReduxComponent = require('../reduxComponent')
@@ -24,6 +25,7 @@ const {getTextColorForBackground} = require('../../../../js/lib/color')
 const frameStateUtil = require('../../../../js/state/frameStateUtil')
 const {getSetting} = require('../../../../js/settings')
 const debounce = require('../../../../js/lib/debounce')
+const cx = require('../../../../js/lib/classSet')
 
 // Styles
 const globalStyles = require('../styles/global')
@@ -224,16 +226,18 @@ class FindBar extends React.Component {
 
   render () {
     let findBarStyle = {}
-    let findBarTextStyle = {}
+    let findBarTextStyles = {}
 
     if (this.props.backgroundColor) {
+      findBarTextStyles = StyleSheet.create({
+        matchingTextColor: {
+          color: this.props.textColor
+        }
+      })
       findBarStyle = {
         background: this.props.backgroundColor,
         color: this.props.textColor
       }
-      findBarTextStyle = {
-        color: this.props.textColor
-      }
     }
 
     return <div className='findBar' style={findBarStyle} onBlur={this.onBlur}>
@@ -275,16 +279,15 @@ class FindBar extends React.Component {
           id='caseSensitivityCheckbox'
           checkedOn={this.props.isCaseSensitive}
           onClick={this.onCaseSensitivityChange}
-        />
-        <label
-          htmlFor='caseSensitivityCheckbox'
-          data-l10n-id='caseSensitivity'
-          style={findBarTextStyle}
+          customRightTextClassName={css(findBarTextStyles.matchingTextColor)}
+          rightl10nId='caseSensitivity'
         />
       </div>
       <span
-        className='closeButton'
-        style={findBarTextStyle}
+        className={cx({
+          closeButton: true,
+          [css(findBarTextStyles.matchingTextColor)]: true
+        })}
         onClick={this.onFindHide}
       >+</span>
     </div>

diff --git a/app/renderer/components/preferences/payment/ledgerTable.js b/app/renderer/components/preferences/payment/ledgerTable.js
@@ -264,7 +264,7 @@ class LedgerTable extends ImmutableComponent {
       <div className={css(styles.hideExcludedSites)}>
         <div className={css(styles.columnOffset)} />
         <div className={css(styles.rightColumn)}>
-          <SettingCheckbox small
+          <SettingCheckbox small compact
             dataL10nId='hideExcluded'
             prefKey={settings.PAYMENTS_SITES_HIDE_EXCLUDED}
             settings={this.props.settings}

diff --git a/app/renderer/components/preferences/paymentsTab.js b/app/renderer/components/preferences/paymentsTab.js
@@ -196,9 +196,11 @@ class PaymentsTab extends ImmutableComponent {
           styles.flexAlignCenter,
           styles.paymentsSwitches
         )}>
-          <div className={css(styles.flexAlignEnd, styles.switchWrap)} data-test-id='enablePaymentsSwitch'>
+          <div className={css(styles.flexAlignCenter, styles.switchWrap)} data-test-id='enablePaymentsSwitch'>
             <span className={css(styles.switchWrap__switchSpan)} data-l10n-id='off' />
-            <SettingCheckbox dataL10nId='on'
+            <SettingCheckbox
+              dataL10nId='on'
+              compact
               prefKey={settings.PAYMENTS_ENABLED}
               settings={this.props.settings}
               onChangeSetting={this.props.onChangeSetting}
@@ -226,6 +228,7 @@ class PaymentsTab extends ImmutableComponent {
               <div className={css(styles.switchWrap__autoSuggestSwitch)}>
                 <div className={css(styles.flexAlignCenter, styles.autoSuggestSwitch__subtext)}>
                   <SettingCheckbox dataL10nId='autoSuggestSites'
+                    compact
                     prefKey={settings.PAYMENTS_SITES_AUTO_SUGGEST}
                     settings={this.props.settings}
                     disabled={!enabled}
@@ -280,10 +283,6 @@ const styles = StyleSheet.create({
     display: 'flex',
     alignItems: 'center'
   },
-  flexAlignEnd: {
-    display: 'flex',
-    alignItems: 'flex-end'
-  },
 
   paymentsContainer: {
     position: 'relative',
@@ -307,11 +306,6 @@ const styles = StyleSheet.create({
   switchWrap: {
     width: paymentStyles.width.tableCell
   },
-  switchWrap__switchControl: {
-    // TODO: Refactor switchControls.less
-    paddingTop: '0 !important',
-    paddingBottom: '0 !important'
-  },
   switchWrap__switchSpan: {
     color: '#999',
     fontWeight: 'bold'
@@ -340,8 +334,7 @@ const styles = StyleSheet.create({
   },
   autoSuggestSwitch__moreInfoBtnSuggest: {
     position: 'relative',
-    top: '-1px',
-    left: '8px',
+    left: '3px',
     cursor: 'pointer',
 
     // TODO: refactor preferences.less to remove !important

diff --git a/less/about/preferences.less b/less/about/preferences.less
@@ -272,105 +272,6 @@ table.sortableTable {
   color: @braveOrange;
 }
 
-.prefTabContainer .switchControl {
-  display: inline-block;
-  vertical-align: middle;
-  align-items: center;
-  padding: 5px;
-  cursor: pointer;
-
-  &.disabled {
-    .switchBackground {
-      opacity: 0.3;
-    }
-    .switchControlRightText, .switchControlLeftText {
-      opacity: 0.3;
-    }
-    cursor: default;
-  }
-
-  &:not(.disabled) {
-    .switchBackground.switchedOn {
-      background-color: @switchBG_on;
-      .switchIndicator {
-        right: 2px;
-      }
-    }
-  }
-
-  &.large {
-    .switchBackground {
-      height: 26px;
-      width: 60px;
-      .switchIndicator {
-        height: 22px;
-        width: 22px;
-      }
-    }
-  }
-
-  &.small {
-    .switchBackground {
-      height: @smallSwitchHeight;
-      width: @smallSwitchWidth;
-      .switchIndicator {
-        height: @smallSwitchNubDiameter;
-        width: @smallSwitchNubDiameter;
-        top: 1px;
-        right: calc(@smallSwitchWidth - @smallSwitchNubDiameter - @switchNubRightMargin);
-      }
-    }
-  }
-
-  > label {
-    vertical-align: middle;
-  }
-
-  .switchControlText {
-    margin: auto 0;
-  }
-
-  .switchControlLeftText {
-    margin: auto 5px auto 0;
-  }
-
-  .switchControlRightText {
-    margin: auto 0 auto 5px;
-  }
-
-  .switchControlTopText {
-    margin: 0 auto 5px auto;
-  }
-
-  .switchControlRight {
-    display: flex;
-  }
-
-  .switchMiddle {
-    display: flex;
-    flex-direction: column;
-  }
-
-  .switchBackground {
-    background-color: @switchBG_off;
-    border-radius: 12px;
-    height: @switchHeight;
-    position: relative;
-    width: @switchWidth;
-    box-shadow: @switchShadow;
-
-    .switchIndicator {
-      background-color: white;
-      border-radius: 100%;
-      height: @switchNubDiameter;
-      position: absolute;
-      top: @switchNubTopMargin;
-      width: @switchNubDiameter;
-      box-shadow: @switchNubShadow;
-    }
-  }
-}
-
 .widevineInfo {
   line-height: 1.3em;
 

diff --git a/less/switchControls.less b/less/switchControls.less
@@ -36,6 +36,19 @@
       }
     }
   }
+
+  &.small {
+    .switchBackground {
+      height: @smallSwitchHeight;
+      width: @smallSwitchWidth;
+      .switchIndicator {
+        height: @smallSwitchNubDiameter;
+        width: @smallSwitchNubDiameter;
+        top: 1px;
+        right: calc(@smallSwitchWidth - @smallSwitchNubDiameter - @switchNubRightMargin);
+      }
+    }
+  }
 
   .switchControlText {
     margin: auto 0;