From c1d874f94fc3678994bd4e2d5c0f2d9ade0b7c41 Mon Sep 17 00:00:00 2001 From: Jillian Date: Tue, 10 Jun 2025 21:05:11 -0400 Subject: [PATCH] fix: show Preview tab in sidebar when container child selected [FC-0090] (#2128) Shows the sidebar Preview tab when a container child is selected, while hiding the Preview tab when sidebar shows the container itself, since it's "preview" is already in the main page body. Adds tests to ensure this behavior is maintained. --- .../common/context/SidebarContext.tsx | 17 +++-- .../LibrarySectionSubsectionPage.test.tsx | 68 ++++++++++++++----- .../units/LibraryUnitPage.test.tsx | 11 ++- 3 files changed, 72 insertions(+), 24 deletions(-) diff --git a/src/library-authoring/common/context/SidebarContext.tsx b/src/library-authoring/common/context/SidebarContext.tsx index 62d61ebba..e05db180b 100644 --- a/src/library-authoring/common/context/SidebarContext.tsx +++ b/src/library-authoring/common/context/SidebarContext.tsx @@ -173,7 +173,7 @@ export const SidebarProvider = ({ }, []); // Set the initial sidebar state based on the URL parameters and context. - const { selectedItemId, containerId: selectedContainerId } = useParams(); + const { selectedItemId } = useParams(); const { collectionId, containerId } = useLibraryContext(); const { componentPickerMode } = useComponentPickerContext(); @@ -203,9 +203,15 @@ export const SidebarProvider = ({ } else { openLibrarySidebar(); } + }, [selectedItemId, collectionId, containerId]); - // Hide the Preview tab if we're inside a collection - if (selectedContainerId) { + useEffect(() => { + // Hide the Preview tab (and replace Preview as the default tab) on the container page when: + // * the sidebar is showing the current container, OR + // * the sidebar is showing a selected component. + // We do this to avoid duplicating content between the + // Section/Subsection main pages and the container sidebar. + if (containerId && (!selectedItemId || selectedItemId.startsWith('lb:'))) { setDefaultTab({ collection: COLLECTION_INFO_TABS.Details, component: COMPONENT_INFO_TABS.Manage, @@ -215,8 +221,11 @@ export const SidebarProvider = ({ COMPONENT_INFO_TABS.Preview, CONTAINER_INFO_TABS.Preview, ]); + } else { + setDefaultTab(DEFAULT_TAB); + setHiddenTabs([]); } - }, [selectedItemId, selectedContainerId, collectionId, containerId]); + }, [selectedItemId, containerId]); const context = useMemo(() => { const contextValue = { diff --git a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx index 9527a5cc2..727773c91 100644 --- a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx @@ -79,6 +79,7 @@ describe('', () => { containerId?: string, libraryId?: string, cType: ContainerType = ContainerType.Section, + childId?: string, ) => { const libId = libraryId || mockContentLibrary.libraryId; const defaultId = cType === ContainerType.Section @@ -88,7 +89,10 @@ describe('', () => { render(, { path, routerProps: { - initialEntries: [`/library/${libId}/${cType}/${cId}`], + initialEntries: [childId + ? `/library/${libId}/${cType}/${cId}/${childId}` + : `/library/${libId}/${cType}/${cId}`, + ], }, }); }; @@ -126,15 +130,18 @@ describe('', () => { : mockGetContainerMetadata.subsectionId; renderLibrarySectionPage(cId, undefined, cType); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - // Unit title + // Container title -- on main page + sidebar expect((await screen.findAllByText(`Test ${cType}`))[0]).toBeInTheDocument(); - // unit info button + // Container info button shown expect(await screen.findByRole('button', { name: new RegExp(`${cType} Info`, 'i') })).toBeInTheDocument(); + // Reorder children buttons shown expect((await screen.findAllByRole('button', { name: 'Drag to reorder' })).length).toEqual(3); - // check all children components are rendered. - expect((await screen.findAllByText(`${childType} block 0`))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(`${childType} block 1`))[0]).toBeInTheDocument(); - expect((await screen.findAllByText(`${childType} block 2`))[0]).toBeInTheDocument(); + // Check all children components are rendered only once. + expect(await screen.findByText(`${childType} block 0`)).toBeInTheDocument(); + expect(await screen.findByText(`${childType} block 1`)).toBeInTheDocument(); + expect(await screen.findByText(`${childType} block 2`)).toBeInTheDocument(); + // Check no Preview tab is shown + expect(screen.queryByText('Preview')).not.toBeInTheDocument(); }); it(`shows ${cType} data with no children`, async () => { @@ -143,12 +150,14 @@ describe('', () => { : mockGetContainerMetadata.subsectionIdEmpty; renderLibrarySectionPage(cId, undefined, cType); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - // Unit title + // Container title -- rendered on main page + sidebar expect((await screen.findAllByText(`Test ${cType}`))[0]).toBeInTheDocument(); - // unit info button + // Container info button shown expect(await screen.findByRole('button', { name: new RegExp(`${cType} Info`, 'i') })).toBeInTheDocument(); - // check all children components are rendered. - expect((await screen.findAllByText(`This ${cType} is empty`))[0]).toBeInTheDocument(); + // Check "no children" text is rendered. + expect(await screen.findByText(`This ${cType} is empty`)).toBeInTheDocument(); + // Check no Preview tab is shown + expect(screen.queryByText('Preview')).not.toBeInTheDocument(); }); it(`can rename ${cType}`, async () => { @@ -219,20 +228,43 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalledWith('Failed to update container.'); }); + it(`should preview child in sidebar by clicking ${childType} on ${cType} page`, async () => { + const childId = `lct:org1:Demo_course:${childType}:${childType}-0`; + const url = getLibraryContainerApiUrl(childId); + axiosMock.onPatch(url).reply(200); + renderLibrarySectionPage(undefined, undefined, cType); + + // Wait loading of the children + const child = await screen.findByText(`${childType} block 0`); + // No Preview tab is shown yet + expect(screen.queryByText('Preview')).not.toBeInTheDocument(); + + // Select the child + fireEvent.click(child); + expect((await screen.findAllByText(`${childType} block 0`)).length === 2); + + // Because the Preview show/hide is dependent on the selected item + // being in the URL, and because our test router doesn't change + // paths, we have to explicitly navigate to the child page to check + // the Preview tab is shown. Boo. + renderLibrarySectionPage(undefined, undefined, cType, childId); + expect((await screen.findAllByText(`${childType} block 0`)).length === 2); + expect(await screen.findByText('Preview')).toBeInTheDocument(); + }); + it(`should rename child by clicking edit icon besides name in ${cType} page`, async () => { const url = getLibraryContainerApiUrl(`lb:org1:Demo_course:${childType}:${childType}-0`); axiosMock.onPatch(url).reply(200); renderLibrarySectionPage(undefined, undefined, cType); - // Wait loading of the component (on page and in sidebar) - await screen.findAllByText(`${childType} block 0`); + // Wait loading of the children + await screen.findByText(`${childType} block 0`); const editButton = (await screen.findAllByRole( 'button', { name: /edit/i }, ))[1]; // 0 is the Section Title, 1 is the first subsection on the list fireEvent.click(editButton); - screen.debug(editButton); expect(await screen.findByRole('textbox', { name: /text input/i })).toBeInTheDocument(); @@ -257,8 +289,8 @@ describe('', () => { axiosMock.onPatch(url).reply(400); renderLibrarySectionPage(undefined, undefined, cType); - // Wait loading of the component (on page and in sidebar) - await screen.findAllByText(`${childType} block 0`); + // Wait loading of the children + await screen.findByText(`${childType} block 0`); const editButton = screen.getAllByRole( 'button', @@ -339,9 +371,9 @@ describe('', () => { it(`should open ${childType} page on double click`, async () => { renderLibrarySectionPage(undefined, undefined, cType); - const subsection = (await screen.findAllByText(`${childType} block 0`))[0]; + const child = await screen.findByText(`${childType} block 0`); // trigger double click - userEvent.click(subsection.parentElement!, undefined, { clickCount: 2 }); + userEvent.click(child.parentElement!, undefined, { clickCount: 2 }); expect((await screen.findAllByText(new RegExp(`Test ${childType}`, 'i')))[0]).toBeInTheDocument(); expect(await screen.findByRole('button', { name: new RegExp(`${childType} Info`, 'i') })).toBeInTheDocument(); }); diff --git a/src/library-authoring/units/LibraryUnitPage.test.tsx b/src/library-authoring/units/LibraryUnitPage.test.tsx index b627f62bd..e9584a849 100644 --- a/src/library-authoring/units/LibraryUnitPage.test.tsx +++ b/src/library-authoring/units/LibraryUnitPage.test.tsx @@ -90,7 +90,7 @@ describe('', () => { it('shows unit data', async () => { renderLibraryUnitPage(); expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); - // Unit title + // Unit title -- on main page + sidebar expect((await screen.findAllByText(mockGetContainerMetadata.containerData.displayName))[0]).toBeInTheDocument(); // unit info button expect(await screen.findByRole('button', { name: 'Unit Info' })).toBeInTheDocument(); @@ -99,8 +99,10 @@ describe('', () => { expect(await screen.findByText('text block 0')).toBeInTheDocument(); expect(await screen.findByText('text block 1')).toBeInTheDocument(); expect(await screen.findByText('text block 2')).toBeInTheDocument(); - // 3 preview iframes + // 3 preview iframes on main page expect((await screen.findAllByTestId('block-preview')).length).toEqual(3); + // No Preview tab in sidebar + expect(screen.queryByText('Preview')).not.toBeInTheDocument(); }); it('can rename unit', async () => { @@ -188,6 +190,9 @@ describe('', () => { it('should open and close component sidebar on component selection', async () => { renderLibraryUnitPage(); expect((await screen.findAllByText('Test Unit')).length).toBeGreaterThan(1); + // No Preview tab shown in sidebar + expect(screen.queryByText('Preview')).not.toBeInTheDocument(); + const component = await screen.findByText('text block 0'); // Card is 3 levels up the component name div userEvent.click(component.parentElement!.parentElement!.parentElement!); @@ -197,6 +202,8 @@ describe('', () => { // The mock data for the sidebar has a title of "text block 0" expect(await findByText('text block 0')).toBeInTheDocument(); + // Preview tab still not shown in sidebar + expect(screen.queryByText('Preview')).not.toBeInTheDocument(); const closeButton = await findByRole('button', { name: /close/i }); userEvent.click(closeButton);