PR Tweaks

src/amo/components/ReportAbuseButton/index.js

@@ -1,12 +1,12 @@
 import classNames from 'classnames';
 import { oneLine } from 'common-tags';
-import defaultDebounce from 'lodash.debounce';
 import React from 'react';
 import { connect } from 'react-redux';
 import Textarea from 'react-textarea-autosize';
 import { compose } from 'redux';
 
 import { withErrorHandler } from 'core/errorHandler';
+import type { ErrorHandlerType } from 'core/errorHandler';
 import translate from 'core/i18n/translate';
 import log from 'core/logger';
 import {
@@ -23,26 +23,26 @@ import './styles.scss';
 
 
 type PropTypes = {
-  abuseReport: {| message: string, reporter: Object | null |},
+  abuseReport: {|
+    message: string,
+    reporter: Object | null,
+  |},
   addon: Object | null,
-  debounce: any,
-  dispatch: any,
-  errorHandler: any,
+  dispatch: Function,
+  errorHandler: ErrorHandlerType,
   loading: bool,
   i18n: Object,
 };
 
 export class ReportAbuseButtonBase extends React.Component {
-  static defaultProps = {
-    debounce: defaultDebounce,
-  };
-
-  cancelReport = (event) => {
+  dismissReportUI = (event) => {
     event.preventDefault();
 
     const { addon, dispatch, loading } = this.props;
 
     if (loading) {
+      log.debug(
+        "Ignoring dismiss click because we're submitting the abuse report");
       return;
     }
 
@@ -52,12 +52,11 @@ export class ReportAbuseButtonBase extends React.Component {
   sendReport = (event) => {
     event.preventDefault();
 
-    // The button isn't clickable if there is no content, but because we
-    // debounce the checks we just verify there's a message to send.
+    // The button isn't clickable if there is no content, but just in case:
+    // we verify there's a message to send.
     if (!this.textarea.value.length) {
-      log.debug(oneLine`User managed to click submit button before
-        disableAbuseButtonUI() was called because of debouncing textarea
-        onChange event. Ignoring this onClick event.`);
+      log.debug(oneLine`User managed to click submit button while textarea
+        was empty. Ignoring this onClick/sendReport event.`);
       return;
     }
 
@@ -70,7 +69,7 @@ export class ReportAbuseButtonBase extends React.Component {
     }));
   }
 
-  showMore = (event) => {
+  showReportUI = (event) => {
     event.preventDefault();
 
     const { addon, dispatch } = this.props;
@@ -79,16 +78,18 @@ export class ReportAbuseButtonBase extends React.Component {
     this.textarea.focus();
   }
 
-  textareaChange = this.props.debounce(() => {
+  textareaChange = () => {
     const { abuseReport, addon, dispatch } = this.props;
 
     // Don't dispatch the UI update if the button is already visible.
-    if (this.textarea.value.length && !abuseReport.buttonEnabled) {
+    // We also test for `value.trim()` so the user can't submit an
+    // empty report full of spaces.
+    if (this.textarea.value.trim().length && !abuseReport.buttonEnabled) {
       dispatch(enableAbuseButtonUI({ addon }));
-    } else if (!this.textarea.value.length) {
+    } else if (!this.textarea.value.trim().length) {
       dispatch(disableAbuseButtonUI({ addon }));
     }
-  }, 100, { trailing: true })
+  }
 
   props: PropTypes;
 
@@ -145,7 +146,7 @@ export class ReportAbuseButtonBase extends React.Component {
         <div className="ReportAbuseButton--preview">
           <Button
             className="ReportAbuseButton-show-more Button--report"
-            onClick={this.showMore}
+            onClick={this.showReportUI}
           >
             {i18n.gettext('Report this add-on for abuse')}
           </Button>
@@ -178,11 +179,11 @@ export class ReportAbuseButtonBase extends React.Component {
 
           <div className="ReportAbuseButton-buttons">
             <a
-              className={classNames('ReportAbuseButton-cancel-report', {
-                'ReportAbuseButton-cancel-report--disabled': loading,
+              className={classNames('ReportAbuseButton-dismiss-report', {
+                'ReportAbuseButton-dismiss-report--disabled': loading,
               })}
               href="#cancel"
-              onClick={this.cancelReport}
+              onClick={this.dismissReportUI}
             >
               {i18n.gettext('Dismiss')}
             </a>
@@ -216,5 +217,5 @@ export const mapStateToProps = (state, ownProps) => {
 export default compose(
   connect(mapStateToProps),
   translate(),
-  withErrorHandler({ name: 'AddonAbuseReport' }),
+  withErrorHandler({ name: 'ReportAbuseButton' }),
 )(ReportAbuseButtonBase);

src/amo/components/ReportAbuseButton/styles.scss

@@ -32,6 +32,11 @@
   font-size: $font-size-m-smaller;
   line-height: 1.4;
   margin: 5px auto;
+  // This is the height of two lines of text. Often the placeholder text
+  // will span two lines–because this element will grow/shrink based on
+  // the contents of the text inside it, we set a minimum height so it
+  // won't shrink when the two-line placeholder is replaced with a single
+  // character when the user starts typing.
   min-height: 51px;
   padding: 5px;
   resize: none;
@@ -43,11 +48,11 @@
   justify-content: space-between;
 }
 
-.ReportAbuseButton-cancel-report {
+.ReportAbuseButton-dismiss-report {
   align-self: center;
 }
 
-.ReportAbuseButton-cancel-report--disabled:link {
+.ReportAbuseButton-dismiss-report--disabled:link {
   color: $neutral-base-color;
   cursor: not-allowed;
 }

src/core/css/inc/mixins.scss

@@ -176,6 +176,7 @@ $default-arrow-margin: 3px;
   background: $background;
   border: 1px solid $border-color;
   border-radius: $border-radius;
+  cursor: pointer;
   display: inline-block;
   margin: 0;
   min-height: 39px;

src/core/reducers/abuse.js

@@ -8,7 +8,7 @@ export const LOAD_ADDON_ABUSE_REPORT = 'LOAD_ADDON_ABUSE_REPORT';
 export const SEND_ADDON_ABUSE_REPORT = 'SEND_ADDON_ABUSE_REPORT';
 export const SHOW_ADDON_ABUSE_REPORT_UI = 'SHOW_ADDON_ABUSE_REPORT_UI';
 
-type DisableAddonAbuseButtonUIType = { addon: AddonType };
+type DisableAddonAbuseButtonUIType = {| addon: AddonType |};
 
 export function disableAbuseButtonUI(
   { addon }: DisableAddonAbuseButtonUIType = {}

tests/unit/amo/components/TestReportAbuseButton.js

@@ -27,7 +27,6 @@ describe(__filename, () => {
     return mount(
       <ReportAbuseButton
         addon={addon}
-        debounce={(callback) => (...args) => callback(...args)}
         errorHandler={errorHandler}
         i18n={getFakeI18nInst()}
         store={store}
@@ -87,7 +86,8 @@ describe(__filename, () => {
 
     root.find('.ReportAbuseButton-show-more').simulate('click');
 
-    root.find('.ReportAbuseButton-cancel-report').simulate('click', fakeEvent);
+    root.find('.ReportAbuseButton-dismiss-report')
+      .simulate('click', fakeEvent);
 
     sinon.assert.called(fakeEvent.preventDefault);
     expect(root.find('.ReportAbuseButton--is-expanded')).toHaveLength(0);
@@ -133,15 +133,15 @@ describe(__filename, () => {
     root.find('.ReportAbuseButton-show-more').simulate('click');
     expect(root.find('.ReportAbuseButton--is-expanded')).toHaveLength(1);
 
-    const cancelButton = root.find('.ReportAbuseButton-cancel-report');
+    const dismissButton = root.find('.ReportAbuseButton-dismiss-report');
     const sendButton = root.find('.ReportAbuseButton-send-report');
 
-    expect(cancelButton)
-      .toHaveClassName('ReportAbuseButton-cancel-report--disabled');
+    expect(dismissButton)
+      .toHaveClassName('ReportAbuseButton-dismiss-report--disabled');
     expect(sendButton.prop('disabled')).toEqual(true);
     expect(sendButton.prop('children')).toEqual('Sending abuse report');
 
-    cancelButton.simulate('click', fakeEvent);
+    dismissButton.simulate('click', fakeEvent);
     sinon.assert.called(fakeEvent.preventDefault);
     expect(root.find('.ReportAbuseButton--is-expanded')).toHaveLength(1);
   });
@@ -225,6 +225,22 @@ describe(__filename, () => {
     sinon.assert.neverCalledWith(dispatchSpy, enableAbuseButtonUI({ addon }));
   });
 
+  it('does not dispatch enableAbuseButtonUI if textarea is whitespace', () => {
+    const addon = { ...fakeAddon, slug: 'which-browser' };
+    const { store } = dispatchClientMetadata();
+    const dispatchSpy = sinon.spy(store, 'dispatch');
+    const root = renderMount({ addon, store });
+
+    // This simulates entering text into the textarea.
+    const textarea = root.find('.ReportAbuseButton-textarea textarea');
+    textarea.node.value = '      ';
+    textarea.simulate('change');
+
+    sinon.assert.neverCalledWith(dispatchSpy, enableAbuseButtonUI({ addon }));
+    expect(root.find('.ReportAbuseButton-send-report').prop('disabled'))
+      .toEqual(true);
+  });
+
   it('disables the submit button if there is no text in the textarea', () => {
     const addon = { ...fakeAddon, slug: 'which-browser' };
     const { store } = dispatchClientMetadata();
@@ -244,11 +260,27 @@ describe(__filename, () => {
       .toEqual(true);
   });
 
+  it('disables the submit button if there is empty text in the textarea', () => {
+    const addon = { ...fakeAddon, slug: 'which-browser' };
+    const { store } = dispatchClientMetadata();
+    const dispatchSpy = sinon.spy(store, 'dispatch');
+    const root = renderMount({ addon, store });
+
+    // This simulates entering text into the textarea.
+    const textarea = root.find('.ReportAbuseButton-textarea textarea');
+    textarea.node.value = 'Opera did it first!';
+    textarea.simulate('change');
+
+    textarea.node.value = '        ';
+    textarea.simulate('change');
+
+    sinon.assert.calledWith(dispatchSpy, disableAbuseButtonUI({ addon }));
+    expect(root.find('.ReportAbuseButton-send-report').prop('disabled'))
+      .toEqual(true);
+  });
+
   // This is a bit of a belt-and-braces approach, as the button that
   // activates this function is disabled when the textarea is empty.
-  // But because it's debounced this just makes sure the dispatch won't
-  // be called if the textarea is empty but this function manages to be
-  // called.
   it('does not allow dispatch if there is no content in the textarea', () => {
     const addon = { ...fakeAddon, slug: 'this-should-not-happen' };
     const fakeEvent = createFakeEvent();