feat: store sidebar tab and additional action in query string

Related changes: * adds "manage-team" sidebar action to trigger LibraryInfo's "Manage Team" modal * moves useStateWithUrlSearchParam up to hooks.ts so it can be used by search-manager and library-authoring * splits sidebarCurrentTab and additionalAction out of SidebarComponentInfo -- they're now managed independently as url parameters. This also simplifies setSidebarComponentInfo: we can simply set the new id + key, and so there's no need to merge the ...prev state and new state. * shortens some sidebar property names: sidebarCurrentTab => sidebarTab sidebarAdditionalAction => sidebarAction * test: Tab changes now trigger a navigate() call, which invalidates the within(sidebar) element in the tests. So using screen instead.
openedx · Dec 19, 2024 · f6b46c4 · f6b46c4
1 parent fc2f467
commit f6b46c4
Show file tree

Hide file tree

Showing 10 changed files with 153 additions and 122 deletions.
diff --git a/src/hooks.ts b/src/hooks.ts
@@ -1,6 +1,12 @@
-import { useEffect, useState } from 'react';
+import {
+  type Dispatch,
+  type SetStateAction,
+  useCallback,
+  useEffect,
+  useState,
+} from 'react';
 import { history } from '@edx/frontend-platform';
-import { useLocation } from 'react-router-dom';
+import { useLocation, useSearchParams } from 'react-router-dom';
 
 export const useScrollToHashElement = ({ isLoading }: { isLoading: boolean }) => {
   const [elementWithHash, setElementWithHash] = useState<string | null>(null);
@@ -77,3 +83,44 @@ export const useLoadOnScroll = (
     return () => { };
   }, [hasNextPage, isFetchingNextPage, fetchNextPage]);
 };
+
+/**
+ * Hook which stores state variables in the URL search parameters.
+ *
+ * It wraps useState with functions that get/set a query string
+ * search parameter when returning/setting the state variable.
+ *
+ * @param defaultValue: Type
+ *   Returned when no valid value is found in the url search parameter.
+ * @param paramName: name of the url search parameter to store this value in.
+ * @param fromString: returns the Type equivalent of the given string value,
+ *   or undefined if the value is invalid.
+ * @param toString: returns the string equivalent of the given Type value.
+ *   Return defaultValue to clear the url search paramName.
+ */
+export function useStateWithUrlSearchParam<Type>(
+  defaultValue: Type,
+  paramName: string,
+  fromString: (value: string | null) => Type | undefined,
+  toString: (value: Type) => string | undefined,
+): [value: Type, setter: Dispatch<SetStateAction<Type>>] {
+  const [searchParams, setSearchParams] = useSearchParams();
+  const returnValue: Type = fromString(searchParams.get(paramName)) ?? defaultValue;
+  // Function to update the url search parameter
+  const returnSetter: Dispatch<SetStateAction<Type>> = useCallback((value: Type) => {
+    setSearchParams((prevParams) => {
+      const paramValue: string = toString(value) ?? '';
+      const newSearchParams = new URLSearchParams(prevParams);
+      // If using the default paramValue, remove it from the search params.
+      if (paramValue === defaultValue) {
+        newSearchParams.delete(paramName);
+      } else {
+        newSearchParams.set(paramName, paramValue);
+      }
+      return newSearchParams;
+    }, { replace: true });
+  }, [setSearchParams]);
+
+  // Return the computed value and wrapped set state function
+  return [returnValue, returnSetter];
+}
diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx
@@ -431,27 +431,23 @@ describe('<LibraryAuthoringPage />', () => {
     // Click on the first collection
     fireEvent.click((await screen.findByText('Collection 1')));
 
-    const sidebar = screen.getByTestId('library-sidebar');
-
-    const { getByRole } = within(sidebar);
-
     // Click on the Details tab
-    fireEvent.click(getByRole('tab', { name: 'Details' }));
+    fireEvent.click(screen.getByRole('tab', { name: 'Details' }));
 
     // Change to a component
     fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]);
 
     // Check that the Details tab is still selected
-    expect(getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true');
+    expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true');
 
     // Click on the Previews tab
-    fireEvent.click(getByRole('tab', { name: 'Preview' }));
+    fireEvent.click(screen.getByRole('tab', { name: 'Preview' }));
 
     // Switch back to the collection
     fireEvent.click((await screen.findByText('Collection 1')));
 
     // The Manage (default) tab should be selected because the collection does not have a Preview tab
-    expect(getByRole('tab', { name: 'Manage' })).toHaveAttribute('aria-selected', 'true');
+    expect(screen.getByRole('tab', { name: 'Manage' })).toHaveAttribute('aria-selected', 'true');
   });
 
   it('can filter by capa problem type', async () => {

diff --git a/src/library-authoring/collections/CollectionInfo.tsx b/src/library-authoring/collections/CollectionInfo.tsx
@@ -27,11 +27,11 @@ const CollectionInfo = () => {
 
   const { componentPickerMode } = useComponentPickerContext();
   const { libraryId, setCollectionId } = useLibraryContext();
-  const { sidebarComponentInfo, setSidebarCurrentTab } = useSidebarContext();
+  const { sidebarComponentInfo, sidebarTab, setSidebarTab } = useSidebarContext();
 
   const tab: CollectionInfoTab = (
-    sidebarComponentInfo?.currentTab && isCollectionInfoTab(sidebarComponentInfo.currentTab)
-  ) ? sidebarComponentInfo?.currentTab : COLLECTION_INFO_TABS.Manage;
+    sidebarTab && isCollectionInfoTab(sidebarTab)
+  ) ? sidebarTab : COLLECTION_INFO_TABS.Manage;
 
   const collectionId = sidebarComponentInfo?.id;
   // istanbul ignore if: this should never happen
@@ -70,7 +70,7 @@ const CollectionInfo = () => {
         className="my-3 d-flex justify-content-around"
         defaultActiveKey={COMPONENT_INFO_TABS.Manage}
         activeKey={tab}
-        onSelect={setSidebarCurrentTab}
+        onSelect={setSidebarTab}
       >
         <Tab eventKey={COMPONENT_INFO_TABS.Manage} title={intl.formatMessage(messages.manageTabTitle)}>
           <ContentTagsDrawer

diff --git a/src/library-authoring/common/context/SidebarContext.tsx b/src/library-authoring/common/context/SidebarContext.tsx
@@ -5,6 +5,7 @@ import {
   useMemo,
   useState,
 } from 'react';
+import { useStateWithUrlSearchParam } from '../../../hooks';
 
 export enum SidebarBodyComponentId {
   AddContent = 'add-content',
@@ -32,29 +33,36 @@ export const isComponentInfoTab = (tab: string): tab is ComponentInfoTab => (
   Object.values<string>(COMPONENT_INFO_TABS).includes(tab)
 );
 
+type SidebarInfoTab = ComponentInfoTab | CollectionInfoTab;
+const toSidebarInfoTab = (tab: string): SidebarInfoTab | undefined => (
+  isComponentInfoTab(tab) || isCollectionInfoTab(tab)
+    ? tab : undefined
+);
+
 export interface SidebarComponentInfo {
   type: SidebarBodyComponentId;
   id: string;
-  /** Additional action on Sidebar display */
-  additionalAction?: SidebarAdditionalActions;
-  /** Current tab in the sidebar */
-  currentTab?: CollectionInfoTab | ComponentInfoTab;
 }
 
-export enum SidebarAdditionalActions {
+export enum SidebarActions {
   JumpToAddCollections = 'jump-to-add-collections',
+  ManageTeam = 'manage-team',
+  None = '',
 }
 
 export type SidebarContextData = {
   closeLibrarySidebar: () => void;
   openAddContentSidebar: () => void;
   openInfoSidebar: (componentId?: string, collectionId?: string) => void;
   openLibrarySidebar: () => void;
-  openCollectionInfoSidebar: (collectionId: string, additionalAction?: SidebarAdditionalActions) => void;
-  openComponentInfoSidebar: (usageKey: string, additionalAction?: SidebarAdditionalActions) => void;
+  openCollectionInfoSidebar: (collectionId: string) => void;
+  openComponentInfoSidebar: (usageKey: string) => void;
   sidebarComponentInfo?: SidebarComponentInfo;
-  resetSidebarAdditionalActions: () => void;
-  setSidebarCurrentTab: (tab: CollectionInfoTab | ComponentInfoTab) => void;
+  sidebarAction: SidebarActions;
+  setSidebarAction: (action: SidebarActions) => void;
+  resetSidebarAction: () => void;
+  sidebarTab: SidebarInfoTab;
+  setSidebarTab: (tab: SidebarInfoTab) => void;
 };
 
 /**
@@ -82,12 +90,22 @@ export const SidebarProvider = ({
     initialSidebarComponentInfo,
   );
 
-  /** Helper function to consume additional action once performed.
-    Required to redo the action.
-  */
-  const resetSidebarAdditionalActions = useCallback(() => {
-    setSidebarComponentInfo((prev) => (prev && { ...prev, additionalAction: undefined }));
-  }, []);
+  const [sidebarTab, setSidebarTab] = useStateWithUrlSearchParam<SidebarInfoTab>(
+    COMPONENT_INFO_TABS.Preview,
+    'st',
+    (value: string) => toSidebarInfoTab(value),
+    (value: SidebarInfoTab) => value.toString(),
+  );
+
+  const [sidebarAction, setSidebarAction] = useStateWithUrlSearchParam<SidebarActions>(
+    SidebarActions.None,
+    'sa',
+    (value: string) => Object.values(SidebarActions).find((enumValue) => value === enumValue),
+    (value: SidebarActions) => value.toString(),
+  );
+  const resetSidebarAction = useCallback(() => {
+    setSidebarAction(SidebarActions.None);
+  }, [setSidebarAction]);
 
   const closeLibrarySidebar = useCallback(() => {
     setSidebarComponentInfo(undefined);
@@ -99,25 +117,18 @@ export const SidebarProvider = ({
     setSidebarComponentInfo({ id: '', type: SidebarBodyComponentId.Info });
   }, []);
 
-  const openComponentInfoSidebar = useCallback((usageKey: string, additionalAction?: SidebarAdditionalActions) => {
-    setSidebarComponentInfo((prev) => ({
-      ...prev,
+  const openComponentInfoSidebar = useCallback((usageKey: string) => {
+    setSidebarComponentInfo({
       id: usageKey,
       type: SidebarBodyComponentId.ComponentInfo,
-      additionalAction,
-    }));
+    });
   }, []);
 
-  const openCollectionInfoSidebar = useCallback((
-    newCollectionId: string,
-    additionalAction?: SidebarAdditionalActions,
-  ) => {
-    setSidebarComponentInfo((prev) => ({
-      ...prev,
+  const openCollectionInfoSidebar = useCallback((newCollectionId: string) => {
+    setSidebarComponentInfo({
       id: newCollectionId,
       type: SidebarBodyComponentId.CollectionInfo,
-      additionalAction,
-    }));
+    });
   }, []);
 
   const openInfoSidebar = useCallback((componentId?: string, collectionId?: string) => {
@@ -130,10 +141,6 @@ export const SidebarProvider = ({
     }
   }, []);
 
-  const setSidebarCurrentTab = useCallback((tab: CollectionInfoTab | ComponentInfoTab) => {
-    setSidebarComponentInfo((prev) => (prev && { ...prev, currentTab: tab }));
-  }, []);
-
   const context = useMemo<SidebarContextData>(() => {
     const contextValue = {
       closeLibrarySidebar,
@@ -143,8 +150,11 @@ export const SidebarProvider = ({
       openComponentInfoSidebar,
       sidebarComponentInfo,
       openCollectionInfoSidebar,
-      resetSidebarAdditionalActions,
-      setSidebarCurrentTab,
+      sidebarAction,
+      setSidebarAction,
+      resetSidebarAction,
+      sidebarTab,
+      setSidebarTab,
     };
 
     return contextValue;
@@ -156,8 +166,11 @@ export const SidebarProvider = ({
     openComponentInfoSidebar,
     sidebarComponentInfo,
     openCollectionInfoSidebar,
-    resetSidebarAdditionalActions,
-    setSidebarCurrentTab,
+    sidebarAction,
+    setSidebarAction,
+    resetSidebarAction,
+    sidebarTab,
+    setSidebarTab,
   ]);
 
   return (
@@ -178,8 +191,11 @@ export function useSidebarContext(): SidebarContextData {
       openLibrarySidebar: () => {},
       openComponentInfoSidebar: () => {},
       openCollectionInfoSidebar: () => {},
-      resetSidebarAdditionalActions: () => {},
-      setSidebarCurrentTab: () => {},
+      sidebarAction: SidebarActions.None,
+      setSidebarAction: () => {},
+      resetSidebarAction: () => {},
+      sidebarTab: COMPONENT_INFO_TABS.Preview,
+      setSidebarTab: () => {},
       sidebarComponentInfo: undefined,
     };
   }

diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx
@@ -16,7 +16,7 @@ import { useLibraryContext } from '../common/context/LibraryContext';
 import {
   type ComponentInfoTab,
   COMPONENT_INFO_TABS,
-  SidebarAdditionalActions,
+  SidebarActions,
   isComponentInfoTab,
   useSidebarContext,
 } from '../common/context/SidebarContext';
@@ -101,27 +101,27 @@ const ComponentInfo = () => {
   const intl = useIntl();
 
   const { readOnly, openComponentEditor } = useLibraryContext();
-  const { setSidebarCurrentTab, sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext();
+  const {
+    sidebarTab,
+    setSidebarTab,
+    sidebarComponentInfo,
+    sidebarAction,
+  } = useSidebarContext();
 
-  const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections;
+  const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections;
 
   const tab: ComponentInfoTab = (
-    sidebarComponentInfo?.currentTab && isComponentInfoTab(sidebarComponentInfo.currentTab)
-  ) ? sidebarComponentInfo?.currentTab : COMPONENT_INFO_TABS.Preview;
+    isComponentInfoTab(sidebarTab)
+      ? sidebarTab
+      : COMPONENT_INFO_TABS.Preview
+  );
 
   useEffect(() => {
     // Show Manage tab if JumpToAddCollections action is set in sidebarComponentInfo
     if (jumpToCollections) {
-      setSidebarCurrentTab(COMPONENT_INFO_TABS.Manage);
-    }
-  }, [jumpToCollections]);
-
-  useEffect(() => {
-    // This is required to redo actions.
-    if (tab !== COMPONENT_INFO_TABS.Manage) {
-      resetSidebarAdditionalActions();
+      setSidebarTab(COMPONENT_INFO_TABS.Manage);
     }
-  }, [tab]);
+  }, [jumpToCollections, setSidebarTab]);
 
   const usageKey = sidebarComponentInfo?.id;
   // istanbul ignore if: this should never happen
@@ -169,7 +169,7 @@ const ComponentInfo = () => {
         className="my-3 d-flex justify-content-around"
         defaultActiveKey={COMPONENT_INFO_TABS.Preview}
         activeKey={tab}
-        onSelect={setSidebarCurrentTab}
+        onSelect={setSidebarTab}
       >
         <Tab eventKey={COMPONENT_INFO_TABS.Preview} title={intl.formatMessage(messages.previewTabTitle)}>
           <ComponentPreview />

diff --git a/src/library-authoring/component-info/ComponentManagement.tsx b/src/library-authoring/component-info/ComponentManagement.tsx
@@ -7,7 +7,7 @@ import {
 } from '@openedx/paragon/icons';
 
 import { useLibraryContext } from '../common/context/LibraryContext';
-import { SidebarAdditionalActions, useSidebarContext } from '../common/context/SidebarContext';
+import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext';
 import { useLibraryBlockMetadata } from '../data/apiHooks';
 import StatusWidget from '../generic/status-widget';
 import messages from './messages';
@@ -18,8 +18,8 @@ import ManageCollections from './ManageCollections';
 const ComponentManagement = () => {
   const intl = useIntl();
   const { readOnly, isLoadingLibraryData } = useLibraryContext();
-  const { sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext();
-  const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections;
+  const { sidebarComponentInfo, sidebarAction, resetSidebarAction } = useSidebarContext();
+  const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections;
   const [tagsCollapseIsOpen, setTagsCollapseOpen] = React.useState(!jumpToCollections);
   const [collectionsCollapseIsOpen, setCollectionsCollapseOpen] = React.useState(true);
 
@@ -33,7 +33,7 @@ const ComponentManagement = () => {
   useEffect(() => {
     // This is required to redo actions.
     if (tagsCollapseIsOpen || !collectionsCollapseIsOpen) {
-      resetSidebarAdditionalActions();
+      resetSidebarAction();
     }
   }, [tagsCollapseIsOpen, collectionsCollapseIsOpen]);