Merge pull request #1404 from creative-commoners/pulls/6/unused-prop

ENH Remove unused prop from ElementEditor

Author: GuySartorelli

client/dist/js/bundle.js

@@ -26,30 +26,27 @@ const ElementEditor = ({
   elementTypes,
   allowedElements,
   sharedObject,
-  isLoading,
   actions,
   forceRefetchElements,
 }) => {
   const [dragging, setDragging] = useState(false);
   const [elements, setElements] = useState(null);
-  // isLoadingState is unused, possibly because isLoading prop is being incorrectly used
-  // setIsLoadingState is still used to update the state internally
-  // eslint-disable-next-line no-unused-vars
-  const [isLoadingState, setIsLoadingState] = useState(true);
+  const [loading, setLoading] = useState(true);
+  const [showLoadingIndicator, setShowLoadingIndicator] = useState(false);
 
   /**
    * Make an API call to read all elements endpoint (areaID)
    */
   const fetchElements = (doSetLoadingState = true) => {
     if (doSetLoadingState) {
-      setIsLoadingState(true);
+      setLoading(true);
     }
     const url = `${getConfig().controllerLink.replace(/\/$/, '')}/api/readElements/${areaId}`;
     return backend.get(url)
       .then(async (response) => {
         const responseJson = await response.json();
         setElements(responseJson);
-        setIsLoadingState(false);
+        setLoading(false);
         // refresh preview
         const preview = window.jQuery('.cms-preview');
         if (preview) {
@@ -59,7 +56,7 @@ const ElementEditor = ({
       })
       .catch(async (err) => {
         setElements([]);
-        setIsLoadingState(false);
+        setLoading(false);
         const message = await getJsonErrorMessage(err);
         actions.toasts.error(message);
         actions.editor.reloadComplete(areaId);
@@ -111,6 +108,27 @@ const ElementEditor = ({
     setElements(sortedElements);
   };
 
+  /**
+   * Delay loading indicator to prevent FOUT
+   */
+  useEffect(() => {
+    let timeoutId;
+    if (!loading) {
+      setShowLoadingIndicator(false);
+    } else {
+      timeoutId = setTimeout(() => {
+        setShowLoadingIndicator(true);
+      }, 300);
+    }
+    // Cleanup timeout - this will run on unmount, and whenever dependency changes, i.e. loading state changes
+    //
+    return () => {
+      if (timeoutId) {
+        clearTimeout(timeoutId);
+      }
+    };
+  }, [loading]);
+
   /**
    * Fetch elements if null or forcing a refetch, e.g. moving a block from one elemental area to another
    * in the same parent DataObject record.
@@ -153,7 +171,7 @@ const ElementEditor = ({
         dragging={dragging}
         sharedObject={sharedObject}
         elements={elements}
-        isLoading={isLoading}
+        isLoading={showLoadingIndicator}
       />
     </ElementEditorContext.Provider>
   </div>;

client/src/components/ElementEditor/ElementEditor.js

@@ -104,7 +104,7 @@ const mockEvent = {
 function makeProps(obj = {}) {
   return {
     ToolbarComponent: ({ elementTypes }) => <div data-testid="test-toolbar" data-elementtypes={elementTypes.map(type => type.class).join(',')} />,
-    ListComponent: ({ elements, onDragEnd }) => <div className="test-list">
+    ListComponent: ({ elements, onDragEnd, isLoading }) => <div className="test-list" data-is-loading={isLoading}>
       {elements.map(element => <div id={`Element${element.id}`} key={element.id} onClick={() => onDragEnd(mockEvent)}>{element.title}</div>)}
     </div>,
     areaId: 8,
@@ -550,3 +550,60 @@ test('ElementEditor preserves dragging state during sort request', async () => {
   await screen.findByTestId('test-toolbar');
   expect(timesReloaded).toBe(2);
 });
+
+test('ElementEditor does not show loading indicator for fast operations (< 300ms)', async () => {
+  const { container } = render(<ElementEditor {...makeProps()}/>);
+  // Immediately resolve - operation completes in < 300ms
+  resolveBackendGet(createJsonResponse());
+  await screen.findByTestId('test-toolbar');
+  const listComponent = container.querySelector('.test-list');
+  expect(listComponent.getAttribute('data-is-loading')).toBe('false');
+});
+
+test('ElementEditor shows loading indicator for slow operations (> 300ms)', async () => {
+  jest.useFakeTimers();
+  const { container } = render(<ElementEditor {...makeProps()}/>);
+  // Wait for 300ms
+  jest.advanceTimersByTime(300);
+  const listComponent = container.querySelector('.test-list');
+  // Should show loading after 300ms
+  expect(listComponent).toBeNull(); // Still returning null since elements is null
+  // Resolve the backend call
+  resolveBackendGet(createJsonResponse());
+  await screen.findByTestId('test-toolbar');
+  jest.useRealTimers();
+});
+
+test('ElementEditor clears loading indicator timeout on fast completion', async () => {
+  jest.useFakeTimers();
+  const { container } = render(<ElementEditor {...makeProps()}/>);
+  // Advance time by 100ms (less than 300ms debounce)
+  jest.advanceTimersByTime(100);
+  // Resolve quickly
+  resolveBackendGet(createJsonResponse());
+  // Advance past debounce time
+  jest.advanceTimersByTime(250);
+  await screen.findByTestId('test-toolbar');
+  const listComponent = container.querySelector('.test-list');
+  // Should not show loading since it completed before 300ms
+  expect(listComponent.getAttribute('data-is-loading')).toBe('false');
+  jest.useRealTimers();
+});
+
+test('ElementEditor debounced loading works on refetch', async () => {
+  jest.useFakeTimers();
+  const { rerender, container } = render(<ElementEditor {...makeProps()}/>);
+  resolveBackendGet(createJsonResponse());
+  jest.advanceTimersByTime(300);
+  await screen.findByTestId('test-toolbar');
+  expect(timesReloaded).toBe(1);
+  // Trigger refetch
+  rerender(<ElementEditor {...makeProps({ forceRefetchElements: true })}/>);
+  // Complete quickly
+  resolveBackendGet(createJsonResponse());
+  jest.advanceTimersByTime(100);
+  await screen.findByTestId('test-toolbar');
+  const listComponent = container.querySelector('.test-list');
+  expect(listComponent.getAttribute('data-is-loading')).toBe('false');
+  jest.useRealTimers();
+});