From f7778d78ce2d858379b7872e4c9e83422e4c9ff5 Mon Sep 17 00:00:00 2001
From: Pete Miller <miller.pete@gmail.com>
Date: Sun, 13 Aug 2017 19:07:41 -0700
Subject: [PATCH 1/2] SwitchControl text to left, right or top of control now
 toggles checked state

Partial resolution for #8243 since many usages do not use the built-in text
but render their own text separately. A subsequent commit will address converting these
usages.

This also converts text from span to label for accessbility and more appropriate renderer defaults (the cursor).
---
 .../components/common/switchControl.js        | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/app/renderer/components/common/switchControl.js b/app/renderer/components/common/switchControl.js
index 7a1c990cd09..43fc29c663d 100644
--- a/app/renderer/components/common/switchControl.js
+++ b/app/renderer/components/common/switchControl.js
@@ -36,18 +36,19 @@ class SwitchControl extends ImmutableComponent {
     >
       {
         this.props.leftl10nId && this.props.topl10nId
-        ? <div className='switchControlText'><div className='switchControlLeftText'><div className='switchSpacer'>&nbsp;</div><span className='switchControlLeftText' data-l10n-id={this.props.leftl10nId} /></div></div>
+        ? <div className='switchControlText'><div className='switchControlLeftText'><div className='switchSpacer'>&nbsp;</div><label className='switchControlLeftText' onClick={this.onClick} data-l10n-id={this.props.leftl10nId} /></div></div>
         : (this.props.leftl10nId
-          ? <span className='switchControlLeftText' data-l10n-id={this.props.leftl10nId} />
+          ? <label className='switchControlLeftText' onClick={this.onClick} data-l10n-id={this.props.leftl10nId} />
           : null)
       }
       <div className='switchMiddle'>
         {
           this.props.topl10nId
-          ? <span className={cx({
+          ? <label className={cx({
             switchControlTopText: true,
             [this.props.customTopTextClassName]: !!this.props.customTopTextClassName
           })}
+            onClick={this.onClick}
             data-l10n-id={this.props.topl10nId} />
           : null
         }
@@ -67,34 +68,36 @@ class SwitchControl extends ImmutableComponent {
         ? <div className='switchControlText'>
           <div className='switchControlRightText'>
             <div className='switchSpacer'>&nbsp;</div>
-            <span className={cx({
+            <label className={cx({
               switchControlRightText: true,
               [this.props.customRightTextClassName]: !!this.props.customRightTextClassName
             })}
+              onClick={this.onClick}
               data-l10n-id={this.props.rightl10nId}
               data-l10n-args={this.props.rightl10nArgs}
-            >{this.props.rightText || ''}</span>
+            >{this.props.rightText || ''}</label>
           </div>
         </div>
         : <div className='switchControlRight'>
           {
             (this.props.rightl10nId || this.props.rightText) && !this.props.onInfoClick
-            ? <span className={cx({
+            ? <label className={cx({
               switchControlRightText: true,
               [this.props.customRightTextClassName]: !!this.props.customRightTextClassName
             })}
+              onClick={this.onClick}
               data-l10n-id={this.props.rightl10nId}
               data-l10n-args={this.props.rightl10nArgs}
-            >{this.props.rightText || ''}</span>
+            >{this.props.rightText || ''}</label>
           : null
         }
           {
             (this.props.rightl10nId || this.props.rightText) && this.props.onInfoClick
             ? <div className='switchControlRightText'>
-              <span data-l10n-id={this.props.rightl10nId}
+              <label onClick={this.onClick} data-l10n-id={this.props.rightl10nId}
                 data-l10n-args={this.props.rightl10nArgs}>
                 {this.props.rightText}
-              </span>
+              </label>
               <span className={cx({
                 fa: true,
                 'fa-question-circle': true,

From b0460f4763b134c7a12cf542a0f4d131ddcb994a Mon Sep 17 00:00:00 2001
From: Pete Miller <miller.pete@gmail.com>
Date: Mon, 14 Aug 2017 00:03:51 -0700
Subject: [PATCH 2/2] Use switchControl text properties instead of sibling
 label elements

closes #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.
---
 app/renderer/components/common/settings.js    | 15 +--
 app/renderer/components/main/findbar.js       | 25 ++---
 .../preferences/payment/ledgerTable.js        |  2 +-
 .../components/preferences/paymentsTab.js     | 19 ++--
 less/about/preferences.less                   | 99 -------------------
 less/switchControls.less                      | 13 +++
 6 files changed, 42 insertions(+), 131 deletions(-)

diff --git a/app/renderer/components/common/settings.js b/app/renderer/components/common/settings.js
index 60c6d7772dc..f7220da80f8 100644
--- 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
index d0cbfb8814c..8a4c2edcd54 100644
--- 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
index 270c5ce7778..98ec64a0bf7 100644
--- 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
index a025ed45f12..55ab4c71535 100644
--- 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
index f03510da67a..0ac11ac8522 100644
--- 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
index 597b8266e41..ad85e541622 100644
--- 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;