From ae82486d7232408971440dac7d409cab0235424d Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Fri, 4 Jul 2025 05:38:37 -0400 Subject: [PATCH] fix: cannot open item sidebar after closing with 'X' button (#2241) When closing an item (component, unit, section, container, etc) sidebar, and trying to reopen it inmediately by clicking the card, it was not opening because navigateTo was being used, but the URL already was the same you were being navigated to. So we also have to update the sidebar item info in the sidebar context in order for it to reopen properly. --- .../LibraryAuthoringPage.test.tsx | 6 +++++- .../common/context/SidebarContext.tsx | 11 +++++++++++ .../components/CollectionCard.tsx | 6 +++--- .../components/ComponentCard.test.tsx | 5 ++++- .../components/ComponentCard.tsx | 8 +++----- .../components/ComponentMenu.tsx | 15 +++++++-------- .../containers/ContainerCard.tsx | 8 ++++---- .../LibraryContainerChildren.tsx | 17 ++++++++--------- .../units/LibraryUnitBlocks.tsx | 15 ++++++--------- 9 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index cbc38bf12..dd3ed50a9 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -398,7 +398,7 @@ describe('', () => { }); }, 10000); - it('should open and close the component sidebar', async () => { + it('should open, close and re-open the component sidebar', async () => { const mockResult0 = { ...mockResult }.results[0].hits[0]; const displayName = 'Introduction to Testing'; expect(mockResult0.display_name).toStrictEqual(displayName); @@ -416,6 +416,10 @@ describe('', () => { fireEvent.click(closeButton); await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); + + fireEvent.click((await screen.findAllByText(displayName))[0]); + + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).toBeInTheDocument()); }); it('should open component sidebar, showing manage tab on clicking add to collection menu item - component', async () => { diff --git a/src/library-authoring/common/context/SidebarContext.tsx b/src/library-authoring/common/context/SidebarContext.tsx index e05db180b..85db6f28e 100644 --- a/src/library-authoring/common/context/SidebarContext.tsx +++ b/src/library-authoring/common/context/SidebarContext.tsx @@ -10,6 +10,7 @@ import { useParams } from 'react-router-dom'; import { useStateWithUrlSearchParam } from '../../../hooks'; import { useComponentPickerContext } from './ComponentPickerContext'; import { useLibraryContext } from './LibraryContext'; +import { useLibraryRoutes } from '../../routes'; export enum SidebarBodyItemId { AddContent = 'add-content', @@ -86,6 +87,7 @@ export type SidebarContextData = { openCollectionInfoSidebar: (collectionId: string) => void; openComponentInfoSidebar: (usageKey: string) => void; openContainerInfoSidebar: (usageKey: string) => void; + openItemSidebar: (selectedItemId: string, type: SidebarBodyItemId) => void; sidebarItemInfo?: SidebarItemInfo; sidebarAction: SidebarActions; setSidebarAction: (action: SidebarActions) => void; @@ -172,6 +174,12 @@ export const SidebarProvider = ({ }); }, []); + const { navigateTo } = useLibraryRoutes(); + const openItemSidebar = useCallback((selectedItemId: string, type: SidebarBodyItemId) => { + navigateTo({ selectedItemId }); + setSidebarItemInfo({ id: selectedItemId, type }); + }, [navigateTo, setSidebarItemInfo]); + // Set the initial sidebar state based on the URL parameters and context. const { selectedItemId } = useParams(); const { collectionId, containerId } = useLibraryContext(); @@ -236,6 +244,7 @@ export const SidebarProvider = ({ sidebarItemInfo, openCollectionInfoSidebar, openContainerInfoSidebar, + openItemSidebar, sidebarAction, setSidebarAction, resetSidebarAction, @@ -254,6 +263,7 @@ export const SidebarProvider = ({ sidebarItemInfo, openCollectionInfoSidebar, openContainerInfoSidebar, + openItemSidebar, sidebarAction, setSidebarAction, resetSidebarAction, @@ -281,6 +291,7 @@ export function useSidebarContext(): SidebarContextData { openComponentInfoSidebar: () => {}, openCollectionInfoSidebar: () => {}, openContainerInfoSidebar: () => {}, + openItemSidebar: () => {}, sidebarAction: SidebarActions.None, setSidebarAction: () => {}, resetSidebarAction: () => {}, diff --git a/src/library-authoring/components/CollectionCard.tsx b/src/library-authoring/components/CollectionCard.tsx index e90b8ae67..4b1f46081 100644 --- a/src/library-authoring/components/CollectionCard.tsx +++ b/src/library-authoring/components/CollectionCard.tsx @@ -116,7 +116,7 @@ type CollectionCardProps = { const CollectionCard = ({ hit } : CollectionCardProps) => { const { componentPickerMode } = useComponentPickerContext(); const { setCollectionId, showOnlyPublished } = useLibraryContext(); - const { openCollectionInfoSidebar, sidebarItemInfo } = useSidebarContext(); + const { openCollectionInfoSidebar, openItemSidebar, sidebarItemInfo } = useSidebarContext(); const { type: itemType, @@ -144,7 +144,7 @@ const CollectionCard = ({ hit } : CollectionCardProps) => { if (doubleClicked) { navigateTo({ collectionId }); } else { - navigateTo({ selectedItemId: collectionId }); + openItemSidebar(collectionId, SidebarBodyItemId.CollectionInfo); } // In component picker mode, we want to open the sidebar or the collection @@ -154,7 +154,7 @@ const CollectionCard = ({ hit } : CollectionCardProps) => { } else { openCollectionInfoSidebar(collectionId); } - }, [collectionId, navigateTo, openCollectionInfoSidebar, setCollectionId, componentPickerMode]); + }, [collectionId, navigateTo, openItemSidebar, openCollectionInfoSidebar, setCollectionId, componentPickerMode]); return ( baseRender(, { params: { libraryId }, extraWrapper: ({ children }) => ( - { children } + + { children } + ), }); diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index 68e1f6cdb..d3fa33b86 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -7,7 +7,6 @@ import { type ContentHit, PublishStatus } from '../../search-manager'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; import { SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext'; -import { useLibraryRoutes } from '../routes'; import AddComponentWidget from './AddComponentWidget'; import BaseCard from './BaseCard'; import { ComponentMenu } from './ComponentMenu'; @@ -18,7 +17,7 @@ type ComponentCardProps = { const ComponentCard = ({ hit }: ComponentCardProps) => { const { showOnlyPublished } = useLibraryContext(); - const { openComponentInfoSidebar, sidebarItemInfo } = useSidebarContext(); + const { openComponentInfoSidebar, openItemSidebar, sidebarItemInfo } = useSidebarContext(); const { componentPickerMode } = useComponentPickerContext(); const { @@ -35,16 +34,15 @@ const ComponentCard = ({ hit }: ComponentCardProps) => { showOnlyPublished ? formatted.published?.displayName : formatted.displayName ) ?? ''; - const { navigateTo } = useLibraryRoutes(); const selectComponent = useCallback(() => { if (!componentPickerMode) { - navigateTo({ selectedItemId: usageKey }); + openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo); } else { // In component picker mode, we want to open the sidebar // without changing the URL openComponentInfoSidebar(usageKey); } - }, [usageKey, navigateTo, openComponentInfoSidebar]); + }, [usageKey, openItemSidebar, openComponentInfoSidebar, componentPickerMode]); const selected = sidebarItemInfo?.type === SidebarBodyItemId.ComponentInfo && sidebarItemInfo.id === usageKey; diff --git a/src/library-authoring/components/ComponentMenu.tsx b/src/library-authoring/components/ComponentMenu.tsx index 52de18ee1..6b9b3e4a0 100644 --- a/src/library-authoring/components/ComponentMenu.tsx +++ b/src/library-authoring/components/ComponentMenu.tsx @@ -10,7 +10,7 @@ import { MoreVert } from '@openedx/paragon/icons'; import { getBlockType } from '@src/generic/key-utils'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext'; import { useClipboard } from '../../generic/clipboard'; import { ToastContext } from '../../generic/toast-context'; import { @@ -36,11 +36,11 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { const { sidebarItemInfo, - openComponentInfoSidebar, closeLibrarySidebar, setSidebarAction, + openItemSidebar, } = useSidebarContext(); - const { navigateTo, insideCollection } = useLibraryRoutes(); + const { insideCollection } = useLibraryRoutes(); const canEdit = usageKey && canEditComponent(usageKey); const { showToast } = useContext(ToastContext); @@ -93,9 +93,9 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { }; const handleEdit = useCallback(() => { - navigateTo({ selectedItemId: usageKey }); + openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo); openComponentEditor(usageKey); - }, [usageKey, navigateTo]); + }, [usageKey, openItemSidebar, openComponentEditor]); const scheduleJumpToCollection = useRunOnNextRender(() => { // TODO: Ugly hack to make sure sidebar shows add to collection section @@ -104,13 +104,12 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { }); const showManageCollections = useCallback(() => { - navigateTo({ selectedItemId: usageKey }); + openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo); scheduleJumpToCollection(); }, [ scheduleJumpToCollection, - openComponentInfoSidebar, usageKey, - navigateTo, + openItemSidebar, ]); const containerType = containerId ? getBlockType(containerId) : 'collection'; diff --git a/src/library-authoring/containers/ContainerCard.tsx b/src/library-authoring/containers/ContainerCard.tsx index f96b2a041..e3e946c3c 100644 --- a/src/library-authoring/containers/ContainerCard.tsx +++ b/src/library-authoring/containers/ContainerCard.tsx @@ -16,7 +16,7 @@ import { ToastContext } from '@src/generic/toast-context'; import { type ContainerHit, Highlight, PublishStatus } from '../../search-manager'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext'; import { useRemoveItemsFromCollection } from '../data/apiHooks'; import { useLibraryRoutes } from '../routes'; import messages from './messages'; @@ -245,7 +245,7 @@ type ContainerCardProps = { const ContainerCard = ({ hit } : ContainerCardProps) => { const { componentPickerMode } = useComponentPickerContext(); const { showOnlyPublished } = useLibraryContext(); - const { openContainerInfoSidebar, sidebarItemInfo } = useSidebarContext(); + const { openContainerInfoSidebar, openItemSidebar, sidebarItemInfo } = useSidebarContext(); const { blockType: itemType, @@ -276,11 +276,11 @@ const ContainerCard = ({ hit } : ContainerCardProps) => { // without changing the URL openContainerInfoSidebar(containerKey); } else if (!doubleClicked) { - navigateTo({ selectedItemId: containerKey }); + openItemSidebar(containerKey, SidebarBodyItemId.ContainerInfo); } else { navigateTo({ containerId: containerKey }); } - }, [containerKey, openContainerInfoSidebar, navigateTo]); + }, [containerKey, openContainerInfoSidebar, openItemSidebar, navigateTo]); return ( { try { @@ -67,10 +66,10 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps) setTimeout(() => setSidebarAction(SidebarActions.JumpToManageTags), 250); }); - const jumpToManageTags = () => { - navigateTo({ selectedItemId: container.originalId }); + const jumpToManageTags = useCallback(() => { + openItemSidebar(container.originalId, SidebarBodyItemId.ContainerInfo); scheduleJumpToTags(); - }; + }, [openItemSidebar, scheduleJumpToTags, container.originalId]); return ( <> @@ -127,7 +126,7 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont const [orderedChildren, setOrderedChildren] = useState([]); const { showOnlyPublished, readOnly: libReadOnly } = useLibraryContext(); const { navigateTo } = useLibraryRoutes(); - const { sidebarItemInfo } = useSidebarContext(); + const { sidebarItemInfo, openItemSidebar } = useSidebarContext(); const [activeDraggingId, setActiveDraggingId] = useState(null); const orderMutator = useUpdateContainerChildren(containerKey); const { showToast } = useContext(ToastContext); @@ -169,11 +168,11 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont const handleChildClick = useCallback((child: LibraryContainerMetadataWithUniqueId, numberOfClicks: number) => { const doubleClicked = numberOfClicks > 1; if (!doubleClicked) { - navigateTo({ selectedItemId: child.originalId }); + openItemSidebar(child.originalId, SidebarBodyItemId.ContainerInfo); } else { navigateTo({ containerId: child.originalId }); } - }, [navigateTo]); + }, [openItemSidebar, navigateTo]); const getComponentStyle = useCallback((childId: string) => { const style: { marginBottom: string, borderRadius: string, outline?: string } = { diff --git a/src/library-authoring/units/LibraryUnitBlocks.tsx b/src/library-authoring/units/LibraryUnitBlocks.tsx index 0dd1072b7..7c1ff0e7b 100644 --- a/src/library-authoring/units/LibraryUnitBlocks.tsx +++ b/src/library-authoring/units/LibraryUnitBlocks.tsx @@ -26,9 +26,8 @@ import { useUpdateXBlockFields, } from '../data/apiHooks'; import { LibraryBlock } from '../LibraryBlock'; -import { useLibraryRoutes } from '../routes'; import messages from './messages'; -import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext'; import { ToastContext } from '../../generic/toast-context'; import { canEditComponent } from '../components/ComponentEditorModal'; import { useRunOnNextRender } from '../../utils'; @@ -57,8 +56,7 @@ const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => { const intl = useIntl(); const { showOnlyPublished } = useLibraryContext(); const { showToast } = useContext(ToastContext); - const { navigateTo } = useLibraryRoutes(); - const { setSidebarAction } = useSidebarContext(); + const { setSidebarAction, openItemSidebar } = useSidebarContext(); const updateMutation = useUpdateXBlockFields(block.originalId); @@ -84,7 +82,7 @@ const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => { /* istanbul ignore next */ const jumpToManageTags = () => { - navigateTo({ selectedItemId: block.originalId }); + openItemSidebar(block.originalId, SidebarBodyItemId.ComponentInfo); scheduleJumpToTags(); }; @@ -132,19 +130,18 @@ const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => { /** ComponentBlock to render preview of given component under Unit */ const ComponentBlock = ({ block, readOnly, isDragging }: ComponentBlockProps) => { const { showOnlyPublished } = useLibraryContext(); - const { navigateTo } = useLibraryRoutes(); const { openComponentEditor } = useLibraryContext(); - const { sidebarItemInfo } = useSidebarContext(); + const { sidebarItemInfo, openItemSidebar } = useSidebarContext(); const handleComponentSelection = useCallback((numberOfClicks: number) => { - navigateTo({ selectedItemId: block.originalId }); + openItemSidebar(block.originalId, SidebarBodyItemId.ComponentInfo); const canEdit = canEditComponent(block.originalId); if (numberOfClicks > 1 && canEdit) { // Open editor on double click. openComponentEditor(block.originalId); } - }, [block, navigateTo, canEditComponent, openComponentEditor]); + }, [block, openItemSidebar, canEditComponent, openComponentEditor]); useEffect(() => { if (block.isNew) {