From 0d0526600d1de4bfd0d3aa39fd2ebd4456f54d10 Mon Sep 17 00:00:00 2001
From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com>
Date: Thu, 3 Dec 2020 16:03:48 -0800
Subject: [PATCH] use radio buttons for telemetry modal

This indicates the current state, lets the user exit without changing
state, and lets the user change state any time they want.

This change also improves RTL support in the telemetry modal.
---
 src/components/gui/gui.jsx                    |  3 +
 .../telemetry-modal/telemetry-modal.css       | 45 +++++----
 .../telemetry-modal/telemetry-modal.jsx       | 96 ++++++++++++-------
 src/lib/app-state-hoc.jsx                     |  2 +
 4 files changed, 92 insertions(+), 54 deletions(-)

diff --git a/src/components/gui/gui.jsx b/src/components/gui/gui.jsx
index 518198c1c..a44604898 100644
--- a/src/components/gui/gui.jsx
+++ b/src/components/gui/gui.jsx
@@ -86,6 +86,7 @@ const GUIComponent = props => {
         isPlayerOnly,
         isRtl,
         isShared,
+        isTelemetryEnabled,
         loading,
         logo,
         renderLogin,
@@ -161,6 +162,8 @@ const GUIComponent = props => {
             >
                 {telemetryModalVisible ? (
                     <TelemetryModal
+                        isRtl={isRtl}
+                        isTelemetryEnabled={isTelemetryEnabled}
                         onCancel={onTelemetryModalCancel}
                         onOptIn={onTelemetryModalOptIn}
                         onOptOut={onTelemetryModalOptOut}
diff --git a/src/components/telemetry-modal/telemetry-modal.css b/src/components/telemetry-modal/telemetry-modal.css
index 9354e8fe0..2037681c5 100644
--- a/src/components/telemetry-modal/telemetry-modal.css
+++ b/src/components/telemetry-modal/telemetry-modal.css
@@ -37,7 +37,6 @@
 .body {
     background: $ui-white;
     padding: 1.5rem 2.25rem;
-    text-align: left;
 }
 
 .privacy-policy-link {
@@ -45,38 +44,50 @@
     text-decoration: none;
 }
 
+.radioButtons {
+    justify-content: center;
+    width: fit-content;
+}
+
+/* stack the radio buttons vertically, not horizontally */
+.radioButtons label {
+    display: block;
+    margin: 0.5rem;
+}
+
 /* Confirmation buttons at the bottom of the modal */
 .button-row {
     margin: 1.5rem 0;
     font-weight: bolder;
+}
+
+[dir="ltr"] .button-row {
     text-align: right;
-    display: flex;
-    justify-content: center;
+}
+
+[dir="rtl"] .button-row {
+    text-align: left;
 }
 
 .button-row button {
     border: 1px solid $motion-primary;
     border-radius: 0.25rem;
     padding: 0.5rem 1.5rem;
-    background: white;
+    color: white;
+    background: $motion-primary;
     font-weight: bold;
     font-size: .875rem;
     cursor: pointer;
 }
 
-.button-row button.opt-in {
-    background: $motion-primary;
-    color: white;
+.button-row button:hover {
+    background: $extensions-primary;
+    box-shadow: 0 0 0 6px $motion-transparent;
 }
 
-.button-row button.opt-out {
-    color: $motion-primary;
-}
-
-[dir="ltr"] .button-row button + button {
-    margin-left: 0.5rem;
-}
-
-[dir="rtl"] .button-row button + button {
-    margin-right: 0.5rem;
+.button-row button:disabled {
+    background: $text-primary;
+    border-color: $ui-black-transparent;
+    box-shadow: none;
+    opacity: 0.25;
 }
diff --git a/src/components/telemetry-modal/telemetry-modal.jsx b/src/components/telemetry-modal/telemetry-modal.jsx
index c09fd1888..16523a634 100644
--- a/src/components/telemetry-modal/telemetry-modal.jsx
+++ b/src/components/telemetry-modal/telemetry-modal.jsx
@@ -33,25 +33,30 @@ const messages = defineMessages({
         description: 'Link to the Scratch privacy policy',
         id: 'gui.telemetryOptIn.privacyPolicyLink'
     },
-    noButton: {
-        defaultMessage: 'No, thanks',
-        description: 'Text for telemetry modal opt-out button',
-        id: 'gui.telemetryOptIn.buttonTextNo'
-    },
-    noTooltip: {
-        defaultMessage: 'Disable telemetry',
-        description: 'Tooltip for telemetry modal opt-out button',
-        id: 'gui.telemetryOptIn.buttonTooltipNo'
-    },
-    yesButton: {
-        defaultMessage: "Yes, I'd like to help improve Scratch",
+    optInText: {
+        defaultMessage: 'Share my usage data with the Scratch Team',
         description: 'Text for telemetry modal opt-in button',
-        id: 'gui.telemetryOptIn.buttonTextYes'
+        id: 'gui.telemetryOptIn.optInText'
     },
-    yesTooltip: {
+    optInTooltip: {
         defaultMessage: 'Enable telemetry',
         description: 'Tooltip for telemetry modal opt-in button',
-        id: 'gui.telemetryOptIn.buttonTooltipYes'
+        id: 'gui.telemetryOptIn.optInTooltip'
+    },
+    optOutText: {
+        defaultMessage: 'Do not share my usage data with the Scratch Team',
+        description: 'Text for telemetry modal opt-in button',
+        id: 'gui.telemetryOptIn.optOutText'
+    },
+    optOutTooltip: {
+        defaultMessage: 'Disable telemetry',
+        description: 'Tooltip for telemetry modal opt-out button',
+        id: 'gui.telemetryOptIn.optOutTooltip'
+    },
+    closeButton: {
+        defaultMessage: 'Close',
+        description: 'Text for the button which closes the telemetry modal dialog',
+        id: 'gui.telemetryOptIn.buttonClose'
     }
 });
 
@@ -60,8 +65,7 @@ class TelemetryModal extends React.PureComponent {
         super(props);
         bindAll(this, [
             'handleCancel',
-            'handleOptIn',
-            'handleOptOut'
+            'handleOptInOutChanged'
         ]);
     }
     handleCancel () {
@@ -70,19 +74,19 @@ class TelemetryModal extends React.PureComponent {
             this.props.onCancel();
         }
     }
-    handleOptIn () {
-        this.props.onRequestClose();
-        if (this.props.onOptIn) {
-            this.props.onOptIn();
-        }
-    }
-    handleOptOut () {
-        this.props.onRequestClose();
-        if (this.props.onOptOut) {
-            this.props.onOptOut();
+    handleOptInOutChanged (e) {
+        if (e.target.value === 'true') {
+            if (this.props.onOptIn) {
+                this.props.onOptIn();
+            }
+        } else if (e.target.value === 'false') {
+            if (this.props.onOptOut) {
+                this.props.onOptOut();
+            }
         }
     }
     render () {
+        const isUndecided = (typeof this.props.isTelemetryEnabled !== 'boolean');
         return (<ReactModal
             isOpen
             className={styles.modalContent}
@@ -109,20 +113,37 @@ class TelemetryModal extends React.PureComponent {
                             </a>)
                         }}
                     /></p>
+                    <Box className={styles.radioButtons}>
+                        <label>
+                            <input
+                                name="optInOut"
+                                type="radio"
+                                value="true"
+                                title={this.props.intl.formatMessage(messages.optInTooltip)}
+                                checked={this.props.isTelemetryEnabled === true}
+                                onChange={this.handleOptInOutChanged}
+                            />
+                            <FormattedMessage {...messages.optInText} />
+                        </label>
+                        <label>
+                            <input
+                                name="optInOut"
+                                type="radio"
+                                value="false"
+                                title={this.props.intl.formatMessage(messages.optOutTooltip)}
+                                checked={this.props.isTelemetryEnabled === false}
+                                onChange={this.handleOptInOutChanged}
+                            />
+                            <FormattedMessage {...messages.optOutText} />
+                        </label>
+                    </Box>
                     <Box className={styles.buttonRow}>
-                        <button
-                            className={styles.optOut}
-                            title={this.props.intl.formatMessage(messages.noTooltip)}
-                            onClick={this.handleOptOut}
-                        >
-                            <FormattedMessage {...messages.noButton} />
-                        </button>
                         <button
                             className={styles.optIn}
-                            title={this.props.intl.formatMessage(messages.yesTooltip)}
-                            onClick={this.handleOptIn}
+                            onClick={this.props.onRequestClose}
+                            disabled={isUndecided}
                         >
-                            <FormattedMessage {...messages.yesButton} />
+                            <FormattedMessage {...messages.closeButton} />
                         </button>
                     </Box>
                 </Box>
@@ -134,6 +155,7 @@ class TelemetryModal extends React.PureComponent {
 TelemetryModal.propTypes = {
     intl: intlShape.isRequired,
     isRtl: PropTypes.bool,
+    isTelemetryEnabled: PropTypes.bool, // false=disabled, true=enabled, undefined=undecided
     onCancel: PropTypes.func,
     onOptIn: PropTypes.func.isRequired,
     onOptOut: PropTypes.func.isRequired,
diff --git a/src/lib/app-state-hoc.jsx b/src/lib/app-state-hoc.jsx
index c7c076e33..ff0ee0b5b 100644
--- a/src/lib/app-state-hoc.jsx
+++ b/src/lib/app-state-hoc.jsx
@@ -97,6 +97,7 @@ const AppStateHOC = function (WrappedComponent, localesOnly) {
             const {
                 isFullScreen, // eslint-disable-line no-unused-vars
                 isPlayerOnly, // eslint-disable-line no-unused-vars
+                isTelemetryEnabled, // eslint-disable-line no-unused-vars
                 showTelemetryModal, // eslint-disable-line no-unused-vars
                 ...componentProps
             } = this.props;
@@ -114,6 +115,7 @@ const AppStateHOC = function (WrappedComponent, localesOnly) {
     AppStateWrapper.propTypes = {
         isFullScreen: PropTypes.bool,
         isPlayerOnly: PropTypes.bool,
+        isTelemetryEnabled: PropTypes.bool,
         showTelemetryModal: PropTypes.bool
     };
     return AppStateWrapper;