From 83c08eea47d1b9edf3c2e60fe604903a4be28da9 Mon Sep 17 00:00:00 2001
From: Suguru Hirahara <luixxiul@users.noreply.github.com>
Date: Tue, 18 Apr 2017 02:47:44 +0900
Subject: [PATCH] Refactor widevinePanel.js and widevineInfo.js

Closes #8028
Addresses #7989

- Removed isClickDismiss from the dialog
  Installing Widevine influences globally, not specific to a tab which opens the page which requires Widevine. Removing isClickDismiss would clarify to the users that the third party software which we discourage from using is going to be installed on the browser right now.

- Applied commonForm.js to them in order to make styling consistent
- Added CommonFormLarge to commonForm.js
- Added globalStyles.dialogWidth and globalStyles.LargeWidth to global.js
- Added noMargin and noPadding to commonStyles.js
- Added margin-top to the bottom text block on widevineInfo.js
- Added testIds to the buttons and the dialog, modifying dialog.js
  - data-test-id as testId
  - data-test2-id as test2Id
- Added widevinePanelDialog as a testId
- Set cursor:pointer

Auditors:

Test Plan:
1. Visit netflix.com
2. Click outside of the dialog
3. Make sure the dialog is not closed
4. Click "Install and Allow"
5. Log in to the account
6. Make sure movies can be played
---
 app/renderer/components/commonForm.js         | 13 +++-
 .../components/styles/commonStyles.js         |  6 ++
 app/renderer/components/styles/global.js      |  2 +
 app/renderer/components/widevineInfo.js       | 35 ++++++++---
 app/renderer/components/widevinePanel.js      | 59 ++++++++++++++-----
 js/components/dialog.js                       |  2 +
 less/forms.less                               |  4 --
 7 files changed, 95 insertions(+), 26 deletions(-)

diff --git a/app/renderer/components/commonForm.js b/app/renderer/components/commonForm.js
index d3e9ba49d18..a9999108095 100644
--- a/app/renderer/components/commonForm.js
+++ b/app/renderer/components/commonForm.js
@@ -18,6 +18,12 @@ class CommonForm extends ImmutableComponent {
   }
 }
 
+class CommonFormLarge extends ImmutableComponent {
+  render () {
+    return <div className={css(commonStyles.flyoutDialog, styles.CommonForm, styles.CommonFormLarge)} {...this.props} />
+  }
+}
+
 class CommonFormDropdown extends ImmutableComponent {
   render () {
     return <FormDropdown data-isCommonForm='true' {...this.props} />
@@ -79,7 +85,7 @@ const styles = StyleSheet.create({
     top: '40px',
     cursor: 'default',
     width: '100%',
-    maxWidth: '422px',
+    maxWidth: globalStyles.spacing.dialogWidth,
     userSelect: 'none'
 
     // Need a general solution
@@ -88,6 +94,10 @@ const styles = StyleSheet.create({
     // maxHeight: '100%'
   },
 
+  CommonFormLarge: {
+    maxWidth: globalStyles.spacing.dialogLargeWidth
+  },
+
   CommonFormClickable: {
     color: '#5b5b5b',
 
@@ -146,6 +156,7 @@ const commonFormStyles = StyleSheet.create({
 
 module.exports = {
   CommonForm,
+  CommonFormLarge,
   CommonFormDropdown,
   CommonFormTextbox,
   CommonFormClickable,
diff --git a/app/renderer/components/styles/commonStyles.js b/app/renderer/components/styles/commonStyles.js
index 9ff64232c89..fbb29e1729f 100644
--- a/app/renderer/components/styles/commonStyles.js
+++ b/app/renderer/components/styles/commonStyles.js
@@ -146,6 +146,9 @@ const styles = StyleSheet.create({
   },
 
   // margin/padding
+  noMargin: {
+    margin: 0
+  },
   noMarginTop: {
     marginTop: 0
   },
@@ -158,6 +161,9 @@ const styles = StyleSheet.create({
   noMarginRight: {
     marginRight: 0
   },
+  noPadding: {
+    padding: 0
+  },
   noPaddingTop: {
     paddingTop: 0
   },
diff --git a/app/renderer/components/styles/global.js b/app/renderer/components/styles/global.js
index 56a2413500e..7bfc0a2c67d 100644
--- a/app/renderer/components/styles/global.js
+++ b/app/renderer/components/styles/global.js
@@ -132,6 +132,8 @@ const globalStyles = {
     iconSize: '16px',
     closeIconSize: '13px',
     narrowIconSize: '12px',
+    dialogWidth: '422px',
+    dialogLargeWidth: '600px',
     dialogTopOffset: '30px',
     dialogInsideMargin: '18px',
     paymentsMargin: '20px',
diff --git a/app/renderer/components/widevineInfo.js b/app/renderer/components/widevineInfo.js
index 1b9915e8990..d12cdb1afbd 100644
--- a/app/renderer/components/widevineInfo.js
+++ b/app/renderer/components/widevineInfo.js
@@ -11,6 +11,11 @@
 const React = require('react')
 const ImmutableComponent = require('./immutableComponent')
 const appConfig = require('../../../js/constants/appConfig')
+const cx = require('../../../js/lib/classSet')
+
+const {StyleSheet, css} = require('aphrodite/no-important')
+
+const {CommonFormSection} = require('./commonForm')
 
 class WidevineInfo extends ImmutableComponent {
   constructor () {
@@ -29,23 +34,39 @@ class WidevineInfo extends ImmutableComponent {
     })
   }
   render () {
-    return <div className='widevineInfo'>
-      <div className='subtext'>
+    return <div data-test-id='widevineInfo'>
+      <CommonFormSection>
         <span data-l10n-id='enableWidevineSubtext' />
-        <span className='fa fa-info-circle'
+        <span className={cx({
+          fa: true,
+          'fa-info-circle': true,
+          [css(styles.cursor)]: true
+        })}
+          data-test-id='onMoreInfo'
           onClick={this.onMoreInfo}
           title={appConfig.widevine.moreInfoUrl}
         />
-      </div>
-      <div className='subtext'>
+      </CommonFormSection>
+      <CommonFormSection>
         <span data-l10n-id='enableWidevineSubtext2' />
-        <span className='fa fa-info-circle'
+        <span className={cx({
+          fa: true,
+          'fa-info-circle': true,
+          [css(styles.cursor)]: true
+        })}
+          data-test-id='onViewLicense'
           onClick={this.onViewLicense}
           title={appConfig.widevine.licenseUrl}
         />
-      </div>
+      </CommonFormSection>
     </div>
   }
 }
 
+const styles = StyleSheet.create({
+  cursor: {
+    cursor: 'pointer'
+  }
+})
+
 module.exports = WidevineInfo
diff --git a/app/renderer/components/widevinePanel.js b/app/renderer/components/widevinePanel.js
index d8737b9db8f..b4434542859 100644
--- a/app/renderer/components/widevinePanel.js
+++ b/app/renderer/components/widevinePanel.js
@@ -13,6 +13,16 @@ const windowActions = require('../../../js/actions/windowActions')
 const appActions = require('../../../js/actions/appActions')
 const siteUtil = require('../../../js/state/siteUtil')
 
+const {
+  CommonFormLarge,
+  CommonFormTitle,
+  CommonFormButtonWrapper,
+  CommonFormBottomWrapper
+} = require('./commonForm')
+
+const {StyleSheet, css} = require('aphrodite/no-important')
+const commonStyles = require('./styles/commonStyles')
+
 class ImportBrowserDataPanel extends ImmutableComponent {
   constructor () {
     super()
@@ -38,28 +48,49 @@ class ImportBrowserDataPanel extends ImmutableComponent {
     if (isLinux) {
       return null
     }
-    return <Dialog onHide={this.props.onHide} className='commonDialog' isClickDismiss>
-      <div className='commonForm' onClick={(e) => e.stopPropagation()}>
-        <h2 className='formSection commonFormTitle' data-l10n-id='widevinePanelTitle' />
-        <div className='formSection'>
-          <WidevineInfo createTabRequestedAction={appActions.createTabRequested} />
-        </div>
-        <div className='formSection commonFormButtons'>
-          <Button l10nId='cancel' className='whiteButton' onClick={this.props.onHide} />
-          <Button l10nId='installAndAllow' className='primaryButton' onClick={this.onInstallAndAllow} />
-        </div>
-        <div className='formSection commonFormBottom'>
-          <div className='commonFormButtonGroup'>
+    /*
+       Removed 'isClickDismiss' from Dialog.
+       Installing Widevine influences globally, not specific to a tab,
+       like Adobe Flash. Removing isClickDismiss would make it clear that
+       the third party software which we discourage from using is going
+       to be installed on the computer.
+    */
+    return <Dialog onHide={this.props.onHide} testId='widevinePanelDialog'>
+      <CommonFormLarge onClick={(e) => e.stopPropagation()}>
+        <CommonFormTitle data-l10n-id='widevinePanelTitle' />
+        <WidevineInfo createTabRequestedAction={appActions.createTabRequested} />
+        <CommonFormButtonWrapper>
+          <Button className='whiteButton'
+            l10nId='cancel'
+            testId='cancelButton'
+            onClick={this.props.onHide}
+          />
+          <Button className='primaryButton'
+            l10nId='installAndAllow'
+            testId='installAndAllowButton'
+            onClick={this.onInstallAndAllow} />
+        </CommonFormButtonWrapper>
+        <CommonFormBottomWrapper>
+          <div className={css(styles.flexJustifyCenter)}>
+            {/* TODO: refactor switchControl.js to remove commonStyles.noPadding */}
             <SwitchControl id={this.props.prefKey}
+              className={css(commonStyles.noPadding)}
               rightl10nId='rememberThisDecision'
               rightl10nArgs={JSON.stringify({origin: this.origin})}
               onClick={this.onClickRememberForNetflix}
               checkedOn={this.props.widevinePanelDetail.get('alsoAddRememberSiteSetting')} />
           </div>
-        </div>
-      </div>
+        </CommonFormBottomWrapper>
+      </CommonFormLarge>
     </Dialog>
   }
 }
 
+const styles = StyleSheet.create({
+  flexJustifyCenter: {
+    display: 'flex',
+    justifyContent: 'center'
+  }
+})
+
 module.exports = ImportBrowserDataPanel
diff --git a/js/components/dialog.js b/js/components/dialog.js
index 5bb5770ef21..59a5b6ed8ed 100644
--- a/js/components/dialog.js
+++ b/js/components/dialog.js
@@ -33,6 +33,8 @@ class Dialog extends ImmutableComponent {
   render () {
     return <div className={'dialog ' + (this.props.className || '')}
       tabIndex='-1'
+      data-test-id={this.props.testId}
+      data-test2-id={this.props.test2Id}
       ref={(node) => { this.dialog = node }}
       onKeyDown={this.onKeyDown.bind(this)}
       onClick={this.onClick.bind(this)}>
diff --git a/less/forms.less b/less/forms.less
index ba759d09f1a..f930476b602 100644
--- a/less/forms.less
+++ b/less/forms.less
@@ -98,10 +98,6 @@ select {
         background-color: #dddee0;
         text-align: center;
       }
-      .commonFormButtonGroup {
-        margin: 0 auto;
-        display: inline-block;
-      }
     }
   }
 }