Added upload spinner for cover image in editor (#9041)

* Added the cover image upload spinner.

* Fixed skipped test for changing cover image.

* Added tests.

app/javascript/article-form/components/ArticleCoverImage.jsx

@@ -1,29 +1,32 @@
-import { h, Component } from 'preact';
+import { h, Component, Fragment } from 'preact';
 import PropTypes from 'prop-types';
 import { generateMainImage } from '../actions';
 import { validateFileInputs } from '../../packs/validateFileInputs';
 import { Button } from '@crayons';
+import { Spinner } from '@crayons/Spinner/Spinner';
 
 export class ArticleCoverImage extends Component {
-  constructor(props) {
-    super(props);
-    this.state = {
-      uploadError: false,
-      uploadErrorMessage: null,
-    };
-  }
+  state = {
+    uploadError: false,
+    uploadErrorMessage: null,
+    uploadingImage: false,
+  };
+
+  onImageUploadSuccess = (...args) => {
+    this.props.onMainImageUrlChange(...args);
+    this.setState({ uploadingImage: false });
+  };
 
   handleMainImageUpload = (e) => {
     e.preventDefault();
 
+    this.setState({ uploadingImage: true });
     this.clearUploadError();
-    const validFileInputs = validateFileInputs();
 
-    if (validFileInputs) {
+    if (validateFileInputs()) {
       const payload = { image: e.target.files, wrap_cloudinary: true };
-      const { onMainImageUrlChange } = this.props;
 
-      generateMainImage(payload, onMainImageUrlChange, this.onUploadError);
+      generateMainImage(payload, this.onImageUploadSuccess, this.onUploadError);
     }
   };
 
@@ -36,6 +39,7 @@ export class ArticleCoverImage extends Component {
 
   onUploadError = (error) => {
     this.setState({
+      uploadingImage: false,
       uploadError: true,
       uploadErrorMessage: error.message,
     });
@@ -51,11 +55,12 @@ export class ArticleCoverImage extends Component {
 
   render() {
     const { mainImage } = this.props;
-    const { uploadError, uploadErrorMessage } = this.state;
+    const { uploadError, uploadErrorMessage, uploadingImage } = this.state;
     const uploadLabel = mainImage ? 'Change' : 'Add a cover image';
+
     return (
       <div className="crayons-article-form__cover" role="presentation">
-        {mainImage && (
+        {!uploadingImage && mainImage && (
           <img
             src={mainImage}
             className="crayons-article-form__cover__image"
@@ -65,24 +70,32 @@ export class ArticleCoverImage extends Component {
           />
         )}
         <div className="flex items-center">
-          <Button variant="outlined" className="mr-2 whitespace-nowrap">
-            <label htmlFor="cover-image-input">{uploadLabel}</label>
-            <input
-              id="cover-image-input"
-              type="file"
-              onChange={this.handleMainImageUpload}
-              accept="image/*"
-              className="w-100 h-100 absolute left-0 right-0 top-0 bottom-0 overflow-hidden opacity-0 cursor-pointer"
-              data-max-file-size-mb="25"
-            />
-          </Button>
-          {mainImage && (
-            <Button
-              variant="ghost-danger"
-              onClick={this.triggerMainImageRemoval}
-            >
-              Remove
-            </Button>
+          {uploadingImage ? (
+            <span class="lh-base pl-1 border-0 py-2 inline-block">
+              <Spinner /> Uploading...
+            </span>
+          ) : (
+            <Fragment>
+              <Button variant="outlined" className="mr-2 whitespace-nowrap">
+                <label htmlFor="cover-image-input">{uploadLabel}</label>
+                <input
+                  id="cover-image-input"
+                  type="file"
+                  onChange={this.handleMainImageUpload}
+                  accept="image/*"
+                  className="w-100 h-100 absolute left-0 right-0 top-0 bottom-0 overflow-hidden opacity-0 cursor-pointer"
+                  data-max-file-size-mb="25"
+                />
+              </Button>
+              {mainImage && (
+                <Button
+                  variant="ghost-danger"
+                  onClick={this.triggerMainImageRemoval}
+                >
+                  Remove
+                </Button>
+              )}
+            </Fragment>
           )}
         </div>
         {uploadError && (

app/javascript/article-form/components/__tests__/ArticleCoverImage.test.jsx

@@ -1,5 +1,10 @@
 import { h } from 'preact';
-import { render, fireEvent, waitForElement } from '@testing-library/preact';
+import {
+  render,
+  fireEvent,
+  waitForElement,
+  waitForElementToBeRemoved,
+} from '@testing-library/preact';
 import { axe } from 'jest-axe';
 import fetch from 'jest-fetch-mock';
 import '@testing-library/jest-dom';
@@ -8,12 +13,6 @@ import { ArticleCoverImage } from '../ArticleCoverImage';
 global.fetch = fetch;
 
 describe('<ArticleCoverImage />', () => {
-  const fakeLinksResponse = JSON.stringify({
-    image: ['/i/changed-fake-link.jpg'],
-  });
-  const fakeErrorMessage = {
-    message: 'Some Fake Error',
-  };
   it('should have no a11y violations', async () => {
     const { container } = render(
       <ArticleCoverImage
@@ -56,43 +55,68 @@ describe('<ArticleCoverImage />', () => {
       expect(queryByText('Remove')).toBeDefined();
     });
 
-    it('allows trigger the correct function for removal', async () => {
+    it('removes an existing cover image', async () => {
       const onMainImageUrlChange = jest.fn();
-      const { getByText } = render(
+      const { getByText, queryByLabelText, queryByText } = render(
         <ArticleCoverImage
           mainImage={'/some-fake-image.jpg'}
           onMainImageUrlChange={onMainImageUrlChange}
         />,
       );
+
+      expect(queryByText(/uploading.../i)).toBeNull();
+      expect(queryByLabelText('Add a cover image')).toBeNull();
+      expect(queryByLabelText('Post cover')).toBeDefined();
+      expect(queryByLabelText('Change')).toBeDefined();
+
       const removeButton = getByText('Remove');
       removeButton.click();
+
       expect(onMainImageUrlChange).toHaveBeenCalledTimes(1);
+
       // we can't test that the image is no longer there as it doesn't get removed in this component
+      // This is handled in the article <Form /> component.
     });
 
-    it.skip('allows a user to change the image', async () => {
+    it('allows a user to change the image', async () => {
+      fetch.mockResponse(
+        JSON.stringify({
+          image: ['/i/changed-fake-link.jpg'],
+        }),
+      );
+
       const onMainImageUrlChange = jest.fn();
-      const { getByLabelText } = render(
+      const { getByLabelText, queryByLabelText, queryByText } = render(
         <ArticleCoverImage
           mainImage={'/some-fake-image.jpg'}
           onMainImageUrlChange={onMainImageUrlChange}
         />,
       );
-      const inputEl = getByLabelText('Change');
 
+      expect(queryByLabelText('Post cover')).toBeDefined();
+      expect(queryByLabelText(/remove/i)).toBeDefined();
+
+      const inputEl = getByLabelText('Change');
       const file = new File(['(⌐□_□)'], 'chucknorris.png', {
         type: 'image/png',
       });
 
-      fetch.mockResponse(fakeLinksResponse);
       fireEvent.change(inputEl, { target: { files: [file] } });
       expect(inputEl.files[0]).toEqual(file);
       expect(inputEl.files).toHaveLength(1);
-      expect(onMainImageUrlChange).toHaveBeenCalledTimes(1);
 
-      // await waitForElement(() =>
-      //   expect(onMainImageUrlChange).toHaveBeenCalledTimes(1),
-      // );
+      expect(queryByText(/uploading.../i)).toBeDefined();
+      expect(queryByLabelText('Post cover')).toBeNull();
+      expect(queryByLabelText('Change')).toBeNull();
+      expect(queryByLabelText(/remove/i)).toBeNull();
+
+      await waitForElementToBeRemoved(() => queryByText(/uploading.../i));
+
+      expect(queryByLabelText('Post cover')).toBeDefined();
+      expect(queryByLabelText('Change')).toBeDefined();
+      expect(queryByLabelText(/remove/i)).toBeDefined();
+
+      expect(onMainImageUrlChange).toHaveBeenCalledTimes(1);
     });
   });
 
@@ -114,7 +138,9 @@ describe('<ArticleCoverImage />', () => {
       type: 'image/png',
     });
 
-    fetch.mockReject(fakeErrorMessage);
+    fetch.mockReject({
+      message: 'Some Fake Error',
+    });
     fireEvent.change(inputEl, { target: { files: [file] } });
 
     await waitForElement(() => getByText(/some fake error/i));