FIX Don't change scroll position when loading (#171)

client/src/components/LinkField/LinkField.js

@@ -178,28 +178,27 @@ const LinkField = ({
   const renderModal = !saveRecordFirst && Boolean(editingID);
   const saveRecordFirstText = i18n._t('LinkField.SAVE_RECORD_FIRST', 'Cannot add links until the record has been saved');
 
-  if (loading && !saveRecordFirst) {
-    return <div className="link-field__loading"><Loading/></div>;
-  }
-
-  return <LinkFieldContext.Provider value={{ ownerID, ownerClass, ownerRelation, actions }}>
-    { saveRecordFirst && <div className="link-field__save-record-first">{saveRecordFirstText}</div>}
-    { renderPicker && <LinkPicker
-        onModalSuccess={onModalSuccess}
-        onModalClosed={onModalClosed}
-        types={types}
-        canCreate={canCreate}
-      /> }
-        <div> { renderLinks() } </div>
-    { renderModal && <LinkModalContainer
-        types={types}
-        typeKey={data[editingID]?.typeKey}
-        isOpen={Boolean(editingID)}
-        onSuccess={onModalSuccess}
-        onClosed={onModalClosed}
-        linkID={editingID}
-      />
-    }
+  return <LinkFieldContext.Provider value={{ ownerID, ownerClass, ownerRelation, actions, loading }}>
+    <div className="link-field__container">
+      { saveRecordFirst && <div className="link-field__save-record-first">{saveRecordFirstText}</div>}
+      { loading && !saveRecordFirst && <Loading containerClass="link-field__loading"/> }
+      { renderPicker && <LinkPicker
+          onModalSuccess={onModalSuccess}
+          onModalClosed={onModalClosed}
+          types={types}
+          canCreate={canCreate}
+        /> }
+          <div> { renderLinks() } </div>
+      { renderModal && <LinkModalContainer
+          types={types}
+          typeKey={data[editingID]?.typeKey}
+          isOpen={Boolean(editingID)}
+          onSuccess={onModalSuccess}
+          onClosed={onModalClosed}
+          linkID={editingID}
+        />
+      }
+    </div>
   </LinkFieldContext.Provider>;
 };
 

client/src/components/LinkField/LinkField.scss

@@ -1,12 +1,22 @@
-.link-field__save-record-first {
-  padding-top: 7px;
+.link-field__container {
+  // Gives an anchor point for the loading elements, which is necessary for
+  // it to work nicely between breakpoints
+  position: relative;
 }
 
+// Needed to avoid the loading overlay expanding beyond the width of the actual field holder
 .link-field__loading {
-  .cms-content-loading-spinner {
-    position: initial;
-  }
-  .ui-widget-overlay-light {
-    opacity: 0;
+  position: absolute;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+
+  .cms-content-loading-overlay {
+    position: relative;
   }
 }
+
+.link-field__save-record-first {
+  padding-top: 7px;
+}

client/src/components/LinkField/tests/LinkField-test.js

@@ -1,6 +1,6 @@
 /* global jest, test */
 import React from 'react';
-import { render, screen } from '@testing-library/react';
+import { render, act } from '@testing-library/react';
 import { Component as LinkField } from '../LinkField';
 
 let doResolve;
@@ -57,12 +57,11 @@ test('LinkField will render save-record-first div if ownerID is 0', async () =>
 test('LinkField will render loading indicator if ownerID is not 0', async () => {
   const { container } = render(<LinkField {...makeProps({
     ownerID: 1
-
   })}
   />);
   expect(container.querySelectorAll('.link-field__save-record-first')).toHaveLength(0);
   expect(container.querySelectorAll('.link-field__loading')).toHaveLength(1);
-  expect(container.querySelectorAll('.link-picker')).toHaveLength(0);
+  expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
 });
 
 test('LinkField will render link-picker if ownerID is not 0 and has finished loading', async () => {
@@ -71,7 +70,10 @@ test('LinkField will render link-picker if ownerID is not 0 and has finished loa
   })}
   />);
   doResolve();
-  await screen.findByText('Add Link');
+  // Short wait - we can't use screen.find* because we're waiting for something to be removed, not added to the DOM
+  await act(async () => {
+    await new Promise((resolve) => setTimeout(resolve, 100));
+  });
   expect(container.querySelectorAll('.link-field__save-record-first')).toHaveLength(0);
   expect(container.querySelectorAll('.link-field__loading')).toHaveLength(0);
   expect(container.querySelectorAll('.link-picker')).toHaveLength(1);

client/src/components/LinkPicker/LinkPickerMenu.js

@@ -1,14 +1,18 @@
 /* eslint-disable */
 import i18n from 'i18n';
-import React, { useState } from 'react';
+import React, { useContext, useState } from 'react';
 import PropTypes from 'prop-types';
 import { Dropdown, DropdownToggle, DropdownMenu, DropdownItem } from 'reactstrap';
+import { LinkFieldContext } from 'components/LinkField/LinkField';
 import LinkType from 'types/LinkType';
 
 const LinkPickerMenu = ({ types, onSelect }) => {
   const [isOpen, setIsOpen] = useState(false);
   const toggle = () => setIsOpen(prevState => !prevState);
+  const { loading } = useContext(LinkFieldContext);
+
   return <Dropdown
+    disabled={loading}
     isOpen={isOpen}
     toggle={toggle}
     className="link-picker__menu"

client/src/components/LinkPicker/LinkPickerTitle.js

@@ -1,9 +1,10 @@
 /* eslint-disable */
 import classnames from 'classnames';
 import i18n from 'i18n';
-import React from 'react';
+import React, { useContext } from 'react';
 import PropTypes from 'prop-types';
 import {Button} from 'reactstrap';
+import { LinkFieldContext } from 'components/LinkField/LinkField';
 
 const stopPropagation = (fn) => (e) => {
   e.nativeEvent.stopImmediatePropagation();
@@ -40,6 +41,7 @@ const LinkPickerTitle = ({
   onClick,
   canDelete
 }) => {
+  const { loading } = useContext(LinkFieldContext);
   const classes = {
     'link-picker__link': true,
     'form-control': true,
@@ -52,7 +54,7 @@ const LinkPickerTitle = ({
     ? i18n._t('LinkField.DELETE', 'Delete')
     : i18n._t('LinkField.ARCHIVE', 'Archive');
   return <div className={className}>
-    <Button className={`link-picker__button ${typeIcon}`}  color="secondary" onClick={stopPropagation(onClick)}>
+    <Button disabled={loading} className={`link-picker__button ${typeIcon}`} color="secondary" onClick={stopPropagation(onClick)}>
       <div className="link-picker__link-detail">
       <div className="link-picker__title">
         <span className="link-picker__title-text">{title}</span>
@@ -65,7 +67,7 @@ const LinkPickerTitle = ({
       </div>
     </Button>
     {canDelete &&
-      <Button className="link-picker__delete" color="link" onClick={stopPropagation(() => onDelete(id))}>{deleteText}</Button>
+      <Button disabled={loading} className="link-picker__delete" color="link" onClick={stopPropagation(() => onDelete(id))}>{deleteText}</Button>
     }
   </div>
 };

client/src/components/LinkPicker/tests/LinkPicker-test.js

@@ -1,39 +1,58 @@
 /* global jest, test */
 import React from 'react';
-import { render } from '@testing-library/react';
+import { render, fireEvent, act } from '@testing-library/react';
+import { LinkFieldContext } from 'components/LinkField/LinkField';
 import LinkPicker from '../LinkPicker';
 
 function makeProps(obj = {}) {
   return {
     types: { phone: { key: 'phone', title: 'Phone', icon: 'font-icon-phone' } },
+    canCreate: true,
     onModalSuccess: () => {},
     onModalClosed: () => {},
     ...obj
   };
 }
 
 test('LinkPickerMenu render() should display toggle if can create', () => {
-  const { container } = render(<LinkPicker {...makeProps({
-    canCreate: true
-  })}
-  />);
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPicker {...makeProps({ canCreate: true })} />
+  </LinkFieldContext.Provider>);
   expect(container.querySelectorAll('.link-picker__menu-toggle')).toHaveLength(1);
   expect(container.querySelectorAll('.link-picker__cannot-create')).toHaveLength(0);
 });
 
 test('LinkPickerMenu render() should display cannot create message if cannot create', () => {
-  const { container } = render(<LinkPicker {...makeProps({
-    canCreate: false
-  })}
-  />);
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPicker {...makeProps({ canCreate: false })} />
+  </LinkFieldContext.Provider>);
   expect(container.querySelectorAll('.link-picker__menu-toggle')).toHaveLength(0);
   expect(container.querySelectorAll('.link-picker__cannot-create')).toHaveLength(1);
 });
 
 test('LinkPickerMenu render() should display link type icon if can create', () => {
-  const { container } = render(<LinkPicker {...makeProps({
-    canCreate: true
-  })}
-  />);
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPicker {...makeProps({ canCreate: true })} />
+  </LinkFieldContext.Provider>);
   expect(container.querySelectorAll('.link-picker__menu-icon.font-icon-phone')).toHaveLength(1);
 });
+
+test('LinkPickerMenu should open dropdown on click when not loading', async () => {
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPicker {...makeProps()} />
+  </LinkFieldContext.Provider>);
+  await act(async () => {
+    await fireEvent.click(container.querySelector('button.link-picker__menu-toggle'));
+  });
+  expect(container.querySelectorAll('.dropdown-menu.show')).toHaveLength(1);
+});
+
+test('LinkPickerMenu should not open dropdown on click while loading', async () => {
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: true }}>
+    <LinkPicker {...makeProps()} />
+  </LinkFieldContext.Provider>);
+  await act(async () => {
+    await fireEvent.click(container.querySelector('button.link-picker__menu-toggle'));
+  });
+  expect(container.querySelectorAll('.dropdown-menu.show')).toHaveLength(0);
+});

client/src/components/LinkPicker/tests/LinkPickerTitle-test.js

@@ -1,7 +1,8 @@
 /* global jest, test */
 
 import React from 'react';
-import { render } from '@testing-library/react';
+import { render, fireEvent } from '@testing-library/react';
+import { LinkFieldContext } from 'components/LinkField/LinkField';
 import LinkPickerTitle from '../LinkPickerTitle';
 
 function makeProps(obj = {}) {
@@ -12,33 +13,75 @@ function makeProps(obj = {}) {
     versionState: 'draft',
     typeTitle: 'Phone',
     typeIcon: 'font-icon-phone',
+    canDelete: true,
     onDelete: () => {},
     onClick: () => {},
     ...obj
   };
 }
 
 test('LinkPickerTitle render() should display clear button if can delete', () => {
-  const { container } = render(<LinkPickerTitle {...makeProps({
-    canDelete: true
-  })}
-  />);
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPickerTitle {...makeProps({ canDelete: true })} />
+  </LinkFieldContext.Provider>);
   expect(container.querySelectorAll('.link-picker__delete')).toHaveLength(1);
   expect(container.querySelectorAll('.font-icon-phone')).toHaveLength(1);
 });
 
 test('LinkPickerTitle render() should not display clear button if cannot delete', () => {
-  const { container } = render(<LinkPickerTitle {...makeProps({
-    canDelete: false
-  })}
-  />);
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPickerTitle {...makeProps({ canDelete: false })} />
+  </LinkFieldContext.Provider>);
   expect(container.querySelectorAll('.link-picker__delete')).toHaveLength(0);
 });
 
 test('LinkPickerTitle render() should display link type icon', () => {
-  const { container } = render(<LinkPickerTitle {...makeProps({
-    canDelete: false
-  })}
-  />);
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPickerTitle {...makeProps({ canDelete: false })} />
+  </LinkFieldContext.Provider>);
   expect(container.querySelectorAll('.font-icon-phone')).toHaveLength(1);
 });
+
+test('LinkPickerTitle delete button should fire the onDelete callback when not loading', async () => {
+  const mockOnDelete = jest.fn();
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPickerTitle {...makeProps({
+      canDelete: true,
+      onDelete: mockOnDelete,
+    })}
+    />
+  </LinkFieldContext.Provider>);
+  fireEvent.click(container.querySelector('button.link-picker__delete'));
+  expect(mockOnDelete).toHaveBeenCalledTimes(1);
+});
+
+test('LinkPickerTitle delete button should not fire the onDelete callback while loading', () => {
+  const mockOnDelete = jest.fn();
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: true }}>
+    <LinkPickerTitle {...makeProps({
+      canDelete: true,
+      onDelete: mockOnDelete,
+    })}
+    />
+  </LinkFieldContext.Provider>);
+  fireEvent.click(container.querySelector('button.link-picker__delete'));
+  expect(mockOnDelete).toHaveBeenCalledTimes(0);
+});
+
+test('LinkPickerTitle main button should fire the onClick callback when not loading', async () => {
+  const mockOnClick = jest.fn();
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
+    <LinkPickerTitle {...makeProps({ onClick: mockOnClick })} />
+  </LinkFieldContext.Provider>);
+  fireEvent.click(container.querySelector('button.link-picker__button'));
+  expect(mockOnClick).toHaveBeenCalledTimes(1);
+});
+
+test('LinkPickerTitle main button should not fire the onClick callback while loading', async () => {
+  const mockOnClick = jest.fn();
+  const { container } = render(<LinkFieldContext.Provider value={{ loading: true }}>
+    <LinkPickerTitle {...makeProps({ onClick: mockOnClick })} />
+  </LinkFieldContext.Provider>);
+  fireEvent.click(container.querySelector('button.link-picker__button'));
+  expect(mockOnClick).toHaveBeenCalledTimes(0);
+});