From bbf14573649b9cda879844b7ed24ad6b87293e0b Mon Sep 17 00:00:00 2001
From: Suguru Hirahara <luixxiul@gmail.com>
Date: Sat, 22 Apr 2017 14:16:57 +0900
Subject: [PATCH] Refactor clearBrowsingDataPanel.js with commonForm

Closes #8192
Addresses #7989

- Added CommonFormSmall to commonForm.js
- Added data-test-id and testIds
- Removed clearBrowsingDataPanel from forms.less
- Updated test code

Auditors:

Test Plan:
1. Open Clear browsing data panel from the menu
2. Test the cancel button works
3. Reopen the panel
4. Test the clear button works
---
 app/renderer/components/commonForm.js         | 15 ++++
 app/renderer/components/styles/global.js      |  1 +
 js/about/history.js                           |  6 +-
 js/about/preferences.js                       |  6 +-
 js/components/clearBrowsingDataPanel.js       | 84 ++++++++++++++-----
 js/components/loginRequired.js                |  6 +-
 js/components/switchControl.js                |  1 +
 less/forms.less                               | 45 ----------
 .../clearBrowsingDataPanelTest.js             | 23 +++--
 test/contents/autofillTest.js                 | 18 ++--
 test/lib/selectors.js                         |  6 +-
 11 files changed, 117 insertions(+), 94 deletions(-)

diff --git a/app/renderer/components/commonForm.js b/app/renderer/components/commonForm.js
index ed5b8c54fb6..6ad0a676ffc 100644
--- a/app/renderer/components/commonForm.js
+++ b/app/renderer/components/commonForm.js
@@ -31,6 +31,16 @@ class CommonFormMedium extends ImmutableComponent {
   }
 }
 
+class CommonFormSmall extends ImmutableComponent {
+  render () {
+    return <div className={css(
+      commonStyles.flyoutDialog,
+      styles.commonForm,
+      styles.commonFormSmall
+    )} {...this.props} />
+  }
+}
+
 class CommonFormLarge extends ImmutableComponent {
   render () {
     return <div className={css(
@@ -133,6 +143,10 @@ const styles = StyleSheet.create({
     // maxHeight: '100%'
   },
 
+  commonFormSmall: {
+    maxWidth: globalStyles.spacing.dialogSmallWidth
+  },
+
   commonFormMedium: {
     maxWidth: globalStyles.spacing.dialogMediumWidth
   },
@@ -208,6 +222,7 @@ const commonFormStyles = StyleSheet.create({
 
 module.exports = {
   CommonForm,
+  CommonFormSmall,
   CommonFormMedium,
   CommonFormLarge,
   CommonFormBookmarkHanger,
diff --git a/app/renderer/components/styles/global.js b/app/renderer/components/styles/global.js
index 011fcec56fa..60ff144b3e3 100644
--- a/app/renderer/components/styles/global.js
+++ b/app/renderer/components/styles/global.js
@@ -134,6 +134,7 @@ const globalStyles = {
     closeIconSize: '13px',
     narrowIconSize: '12px',
     dialogWidth: '422px',
+    dialogSmallWidth: '350px',
     dialogMediumWidth: '500px',
     dialogLargeWidth: '600px',
     dialogTopOffset: '30px',
diff --git a/js/about/history.js b/js/about/history.js
index 6b9ac7e5a8c..34a145d8e22 100644
--- a/js/about/history.js
+++ b/js/about/history.js
@@ -216,7 +216,11 @@ class AboutHistory extends React.Component {
               : <span className='fa fa-search searchInputPlaceholder' />
             }
           </div>
-          <Button l10nId='clearBrowsingDataNow' className='primaryButton clearBrowsingDataButton' onClick={this.clearBrowsingDataNow} />
+          <Button className='primaryButton'
+            l10nId='clearBrowsingDataNow'
+            testId='clearBrowsingDataButton'
+            onClick={this.clearBrowsingDataNow}
+          />
         </div>
       </div>
 
diff --git a/js/about/preferences.js b/js/about/preferences.js
index 4f4bb67aab8..424bcefe9ae 100644
--- a/js/about/preferences.js
+++ b/js/about/preferences.js
@@ -590,7 +590,11 @@ class SecurityTab extends ImmutableComponent {
         <SettingCheckbox dataL10nId='autocompleteData' prefKey={settings.SHUTDOWN_CLEAR_AUTOCOMPLETE_DATA} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
         <SettingCheckbox dataL10nId='autofillData' prefKey={settings.SHUTDOWN_CLEAR_AUTOFILL_DATA} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
         <SettingCheckbox dataL10nId='savedSiteSettings' prefKey={settings.SHUTDOWN_CLEAR_SITE_SETTINGS} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
-        <Button l10nId='clearBrowsingDataNow' className='primaryButton clearBrowsingDataButton' onClick={this.clearBrowsingDataNow} />
+        <Button className='primaryButton'
+          l10nId='clearBrowsingDataNow'
+          testId='clearBrowsingDataButton'
+          onClick={this.clearBrowsingDataNow}
+        />
       </SettingsList>
       <DefaultSectionTitle data-l10n-id='passwordsAndForms' />
       <SettingsList>
diff --git a/js/components/clearBrowsingDataPanel.js b/js/components/clearBrowsingDataPanel.js
index 9ed61871053..4bc48458ccd 100644
--- a/js/components/clearBrowsingDataPanel.js
+++ b/js/components/clearBrowsingDataPanel.js
@@ -11,6 +11,14 @@ const appActions = require('../actions/appActions')
 const ipc = require('electron').ipcRenderer
 const messages = require('../constants/messages')
 
+const {
+  CommonFormSmall,
+  CommonFormSection,
+  CommonFormTitle,
+  CommonFormButtonWrapper,
+  CommonFormBottomWrapper
+} = require('../../app/renderer/components/commonForm')
+
 class ClearBrowsingDataPanel extends React.Component {
   constructor (props) {
     super()
@@ -42,27 +50,63 @@ class ClearBrowsingDataPanel extends React.Component {
     }
   }
   render () {
-    return <Dialog onHide={this.props.onHide} className='clearBrowsingDataPanel' isClickDismiss>
-      <div className='clearBrowsingData' onClick={(e) => e.stopPropagation()}>
-        <div className='formSection clearBrowsingDataTitle' data-l10n-id='clearBrowsingData' />
-        <div className='formSection clearBrowsingDataOptions'>
-          <SwitchControl className='browserHistorySwitch' rightl10nId='browserHistory' checkedOn={this.state.clearBrowsingDataDetail.get('browserHistory')} onClick={this.onToggleBrowserHistory} />
-          <SwitchControl rightl10nId='downloadHistory' checkedOn={this.state.clearBrowsingDataDetail.get('downloadHistory')} onClick={this.onToggleDownloadHistory} />
-          <SwitchControl rightl10nId='cachedImagesAndFiles' checkedOn={this.state.clearBrowsingDataDetail.get('cachedImagesAndFiles')} onClick={this.onToggleCachedImagesAndFiles} />
-          <SwitchControl rightl10nId='savedPasswords' checkedOn={this.state.clearBrowsingDataDetail.get('savedPasswords')} onClick={this.onToggleSavedPasswords} />
-          <SwitchControl rightl10nId='allSiteCookies' checkedOn={this.state.clearBrowsingDataDetail.get('allSiteCookies')} onClick={this.onToggleAllSiteCookies} />
-          <SwitchControl className='autocompleteDataSwitch' rightl10nId='autocompleteData' checkedOn={this.state.clearBrowsingDataDetail.get('autocompleteData')} onClick={this.onToggleAutocompleteData} />
-          <SwitchControl className='autofillDataSwitch' rightl10nId='autofillData' checkedOn={this.state.clearBrowsingDataDetail.get('autofillData')} onClick={this.onToggleAutofillData} />
-          <SwitchControl className='siteSettingsSwitch' rightl10nId='savedSiteSettings' checkedOn={this.state.clearBrowsingDataDetail.get('savedSiteSettings')} onClick={this.onToggleSavedSiteSettings} />
-        </div>
-        <div className='formSection clearBrowsingDataButtons'>
-          <Button l10nId='cancel' className='whiteButton' onClick={this.props.onHide} />
-          <Button l10nId='clear' className='primaryButton clearDataButton' onClick={this.onClear} />
-        </div>
-        <div className='formSection clearBrowsingDataWarning'>
+    return <Dialog onHide={this.props.onHide} testId='clearBrowsingDataPanel' isClickDismiss>
+      <CommonFormSmall onClick={(e) => e.stopPropagation()}>
+        <CommonFormTitle data-l10n-id='clearBrowsingData' />
+        <CommonFormSection>
+          <SwitchControl
+            rightl10nId='browserHistory'
+            testId='browserHistorySwitch'
+            checkedOn={this.state.clearBrowsingDataDetail.get('browserHistory')}
+            onClick={this.onToggleBrowserHistory} />
+          <SwitchControl
+            rightl10nId='downloadHistory'
+            checkedOn={this.state.clearBrowsingDataDetail.get('downloadHistory')}
+            onClick={this.onToggleDownloadHistory} />
+          <SwitchControl
+            rightl10nId='cachedImagesAndFiles'
+            checkedOn={this.state.clearBrowsingDataDetail.get('cachedImagesAndFiles')}
+            onClick={this.onToggleCachedImagesAndFiles} />
+          <SwitchControl
+            rightl10nId='savedPasswords'
+            checkedOn={this.state.clearBrowsingDataDetail.get('savedPasswords')}
+            onClick={this.onToggleSavedPasswords} />
+          <SwitchControl
+            rightl10nId='allSiteCookies'
+            checkedOn={this.state.clearBrowsingDataDetail.get('allSiteCookies')}
+            onClick={this.onToggleAllSiteCookies} />
+          <SwitchControl
+            rightl10nId='autocompleteData'
+            testId='autocompleteDataSwitch'
+            checkedOn={this.state.clearBrowsingDataDetail.get('autocompleteData')}
+            onClick={this.onToggleAutocompleteData} />
+          <SwitchControl
+            rightl10nId='autofillData'
+            testId='autofillDataSwitch'
+            checkedOn={this.state.clearBrowsingDataDetail.get('autofillData')}
+            onClick={this.onToggleAutofillData} />
+          <SwitchControl
+            rightl10nId='savedSiteSettings'
+            testId='siteSettingsSwitch'
+            checkedOn={this.state.clearBrowsingDataDetail.get('savedSiteSettings')}
+            onClick={this.onToggleSavedSiteSettings} />
+        </CommonFormSection>
+        <CommonFormButtonWrapper>
+          <Button className='whiteButton'
+            l10nId='cancel'
+            testId='cancelButton'
+            onClick={this.props.onHide}
+          />
+          <Button className='primaryButton'
+            l10nId='clear'
+            testId='clearDataButton'
+            onClick={this.onClear}
+          />
+        </CommonFormButtonWrapper>
+        <CommonFormBottomWrapper>
           <div data-l10n-id='clearDataWarning' />
-        </div>
-      </div>
+        </CommonFormBottomWrapper>
+      </CommonFormSmall>
     </Dialog>
   }
 }
diff --git a/js/components/loginRequired.js b/js/components/loginRequired.js
index d6ed288667e..a8537d98652 100644
--- a/js/components/loginRequired.js
+++ b/js/components/loginRequired.js
@@ -91,10 +91,10 @@ class LoginRequired extends React.Component {
         <CommonFormSection data-l10n-id='basicAuthMessage' data-l10n-args={JSON.stringify(l10nArgs)} />
         <CommonFormSection>
           <div className={css(styles.sectionWrapper)}>
-            <div data-test-id='loginLabel'
-              className={css(commonFormStyles.inputWrapper,
+            <div className={css(
+              commonFormStyles.inputWrapper,
               commonFormStyles.inputWrapper__label
-            )}>
+            )} data-test-id='loginLabel'>
               <label data-l10n-id='basicAuthUsernameLabel' htmlFor='loginUsername' />
               <label className={css(commonFormStyles.input__bottomRow)} data-l10n-id='basicAuthPasswordLabel' htmlFor='loginPassword' />
             </div>
diff --git a/js/components/switchControl.js b/js/components/switchControl.js
index b86f8d384ad..641a5f3ac54 100644
--- a/js/components/switchControl.js
+++ b/js/components/switchControl.js
@@ -31,6 +31,7 @@ class SwitchControl extends ImmutableComponent {
       hasTopText: this.props.topl10nId
     })}
       data-switch-status={this.props.checkedOn}
+      data-test-id={this.props.testId}
     >
       {
         this.props.leftl10nId && this.props.topl10nId
diff --git a/less/forms.less b/less/forms.less
index 9900d7e2a35..aef79b23eb2 100644
--- a/less/forms.less
+++ b/less/forms.less
@@ -18,51 +18,6 @@ select {
   top: @dialogTopOffset;
 }
 
-.clearBrowsingDataPanel {
-  .clearBrowsingData {
-    .flyoutDialog;
-    background-color: #f7f7f7;
-    border-radius: @borderRadius;
-    max-width: 325px;
-    padding: 0;
-    text-align: left;
-    width: 473px;
-    user-select: none;
-    cursor: default;
-    color: #3B3B3B;
-    overflow-y: auto;
-    max-height: 100%;
-
-    .clickable {
-      color: #5B5B5B;
-      &:hover {
-        color: #000;
-      }
-    }
-
-    .formSection {
-      padding: 16px 30px;
-      &.clearBrowsingDataWarning {
-        background-color: #dddee0;
-      }
-
-      &.clearBrowsingDataTitle {
-        color: #ff5000;
-        font-size: 1.2em;
-      }
-
-      &.clearBrowsingDataOptions {
-        padding: 0 30px;
-      }
-
-      &.clearBrowsingDataButtons {
-        text-align: right;
-        padding: 16px 10px;
-      }
-    }
-  }
-}
-
 .braveryPanelContainer {
   .braveryPanel {
     .flyoutDialog;
diff --git a/test/bravery-components/clearBrowsingDataPanelTest.js b/test/bravery-components/clearBrowsingDataPanelTest.js
index f5fcb389ec4..d913448c655 100644
--- a/test/bravery-components/clearBrowsingDataPanelTest.js
+++ b/test/bravery-components/clearBrowsingDataPanelTest.js
@@ -3,6 +3,7 @@
 const Brave = require('../lib/brave')
 const {urlInput, braveMenu, noScriptSwitch, braveryPanel, notificationBar, clearBrowsingDataButton, securityTab, clearDataButton} = require('../lib/selectors')
 const {getTargetAboutUrl} = require('../../js/lib/appUrlUtil')
+const aboutPreferencesUrl = getTargetAboutUrl('about:preferences')
 const {getHistory} = require('../../app/common/lib/historyUtil')
 const messages = require('../../js/constants/messages')
 
@@ -25,7 +26,7 @@ describe('Clear Browsing Panel', function () {
   function * openClearBrowsingDataPanel (client) {
     return client
       .tabByIndex(0)
-      .loadUrl(getTargetAboutUrl('about:preferences'))
+      .loadUrl(aboutPreferencesUrl)
       .waitForVisible(securityTab)
       .click(securityTab)
       .waitForVisible(clearBrowsingDataButton)
@@ -46,12 +47,11 @@ describe('Clear Browsing Panel', function () {
 
     it('saves the history switch state', function * () {
       const page1Url = Brave.server.url('page1.html')
-      const browserHistorySwitch = '.browserHistorySwitch'
       yield openClearBrowsingDataPanel(this.app.client)
       yield this.app.client
-        .waitForVisible(browserHistorySwitch)
+        .waitForVisible('[data-test-id="browserHistorySwitch"]')
         .waitForVisible(clearDataButton)
-        .click(`${browserHistorySwitch} .switchBackground`)
+        .click('[data-test-id="browserHistorySwitch"] .switchBackground')
         .click(clearDataButton)
         .waitUntil(function () {
           return this.getAppState().then((val) => {
@@ -69,7 +69,7 @@ describe('Clear Browsing Panel', function () {
         })
       yield openClearBrowsingDataPanel(this.app.client)
       yield this.app.client
-        .waitForVisible(`${browserHistorySwitch} .switchedOn`)
+        .waitForVisible('[data-test-id="browserHistorySwitch"] .switchedOn')
         .waitForVisible(clearDataButton)
         .click(clearDataButton)
         .waitUntil(function () {
@@ -99,10 +99,9 @@ describe('Clear Browsing Panel', function () {
     })
 
     it('shows clearing options', function * () {
-      const clearBrowsingDataButton = '.clearBrowsingDataButton'
       yield this.app.client
         .tabByIndex(0)
-        .loadUrl(getTargetAboutUrl('about:preferences'))
+        .loadUrl(aboutPreferencesUrl)
         .waitForVisible(securityTab)
         .click(securityTab)
         .waitForVisible(clearBrowsingDataButton)
@@ -111,13 +110,13 @@ describe('Clear Browsing Panel', function () {
     it('clears the browsing history', function * () {
       yield this.app.client
         .tabByIndex(0)
-        .loadUrl(getTargetAboutUrl('about:preferences'))
+        .loadUrl(aboutPreferencesUrl)
         .waitForVisible(securityTab)
         .click(securityTab)
         .waitForVisible(clearBrowsingDataButton)
         .click(clearBrowsingDataButton)
         .waitForBrowserWindow()
-        .waitForVisible('.browserHistorySwitch')
+        .waitForVisible('[data-test-id="browserHistorySwitch"]')
         .waitForVisible(clearDataButton)
         .click(clearDataButton)
         .waitUntil(function () {
@@ -200,14 +199,14 @@ describe('Clear Browsing Panel', function () {
     it('clears site settings and permissions', function * () {
       yield this.app.client
         .tabByIndex(0)
-        .loadUrl(getTargetAboutUrl('about:preferences'))
+        .loadUrl(aboutPreferencesUrl)
         .waitForVisible(securityTab)
         .click(securityTab)
         .waitForVisible(clearBrowsingDataButton)
         .click(clearBrowsingDataButton)
         .waitForBrowserWindow()
-        .waitForVisible('.siteSettingsSwitch')
-        .click('.siteSettingsSwitch .switchBackground')
+        .waitForVisible('[data-test-id="siteSettingsSwitch"]')
+        .click('[data-test-id="siteSettingsSwitch"] .switchBackground')
         .waitForVisible(clearDataButton)
         .click(clearDataButton)
         .waitUntil(function () {
diff --git a/test/contents/autofillTest.js b/test/contents/autofillTest.js
index 5a79446f42d..279082375ae 100644
--- a/test/contents/autofillTest.js
+++ b/test/contents/autofillTest.js
@@ -1,7 +1,7 @@
 /* global describe, it, before, beforeEach */
 
 const Brave = require('../lib/brave')
-const {urlInput, autofillAddressPanel, autofillCreditCardPanel, clearBrowsingDataButton, securityTab} = require('../lib/selectors')
+const {urlInput, autofillAddressPanel, autofillCreditCardPanel, clearBrowsingDataButton, clearDataButton, securityTab} = require('../lib/selectors')
 const {getTargetAboutUrl} = require('../../js/lib/appUrlUtil')
 
 const addAddressButton = '[data-test-id="addAddressButton"]'
@@ -417,10 +417,10 @@ describe('Autofill', function () {
         .waitForVisible(clearBrowsingDataButton)
         .click(clearBrowsingDataButton)
         .waitForBrowserWindow()
-        .waitForVisible('.autofillDataSwitch')
-        .click('.autofillDataSwitch .switchMiddle')
-        .waitForVisible('.clearDataButton')
-        .click('.clearDataButton')
+        .waitForVisible('[data-test-id="autofillDataSwitch"]')
+        .click('[data-test-id="autofillDataSwitch"] .switchMiddle')
+        .waitForVisible(clearDataButton)
+        .click(clearDataButton)
     })
     it('does not autofill in regular tab', function * () {
       yield this.app.client
@@ -591,10 +591,10 @@ describe('Autofill', function () {
         .waitForVisible(clearBrowsingDataButton)
         .click(clearBrowsingDataButton)
         .waitForBrowserWindow()
-        .waitForVisible('.autocompleteDataSwitch')
-        .click('.autocompleteDataSwitch .switchMiddle')
-        .waitForVisible('.clearDataButton')
-        .click('.clearDataButton')
+        .waitForVisible('[data-test-id="autocompleteDataSwitch"]')
+        .click('[data-test-id="autocompleteDataSwitch"] .switchMiddle')
+        .waitForVisible(clearDataButton)
+        .click(clearDataButton)
       })
       it('does not autofill in regular tab', function * () {
         yield this.app.client
diff --git a/test/lib/selectors.js b/test/lib/selectors.js
index 89387429081..88c6f247932 100644
--- a/test/lib/selectors.js
+++ b/test/lib/selectors.js
@@ -60,9 +60,9 @@ module.exports = {
   backButton: '.backforward .backButton',
   forwardButton: '.backforward .forwardButton',
   reloadButton: '.reloadButton',
-  clearBrowsingDataPanel: '.clearBrowsingDataPanel',
-  clearBrowsingDataButton: '.clearBrowsingDataButton',
-  clearDataButton: '.clearDataButton',
+  clearBrowsingDataButton: '[data-test-id="clearBrowsingDataButton"]',
+  clearBrowsingDataPanel: '[data-test-id="clearBrowsingDataPanel"]',
+  clearDataButton: '[data-test-id="clearDataButton"]',
   securityTab: '[data-l10n-id="security"]',
   paymentsTab: '[data-l10n-id="payments"]',
   syncTab: '[data-l10n-id="sync"]',