From ac9faacc4d0f54724c9fa63cc446bc613e27c184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 30 Jun 2025 19:30:48 +0200 Subject: [PATCH] feat: add ParentBreadcrumbs component [FC-0090] (#2223) Adds the `ParentBreadcrumbs` component to show a list of parent containers on the Unit and Subsection breadcrumbs. --- .../__mocks__/unit-single.json | 60 +++++++++ src/library-authoring/generic/index.scss | 1 + .../ParentBreadcrumbs.test.tsx | 99 ++++++++++++++ .../generic/parent-breadcrumbs/index.scss | 13 ++ .../generic/parent-breadcrumbs/index.tsx | 121 ++++++++++++++++++ .../generic/parent-breadcrumbs/messages.ts | 21 +++ .../LibrarySubsectionPage.tsx | 86 ++----------- .../section-subsections/index.scss | 14 -- .../units/LibraryUnitPage.test.tsx | 37 ++++-- .../units/LibraryUnitPage.tsx | 49 +++---- 10 files changed, 373 insertions(+), 128 deletions(-) create mode 100644 src/library-authoring/__mocks__/unit-single.json create mode 100644 src/library-authoring/generic/parent-breadcrumbs/ParentBreadcrumbs.test.tsx create mode 100644 src/library-authoring/generic/parent-breadcrumbs/index.scss create mode 100644 src/library-authoring/generic/parent-breadcrumbs/index.tsx create mode 100644 src/library-authoring/generic/parent-breadcrumbs/messages.ts diff --git a/src/library-authoring/__mocks__/unit-single.json b/src/library-authoring/__mocks__/unit-single.json new file mode 100644 index 000000000..6531c67e2 --- /dev/null +++ b/src/library-authoring/__mocks__/unit-single.json @@ -0,0 +1,60 @@ +{ + "comment": "This mock is captured from a real search result and roughly edited to match the mocks in src/library-authoring/data/api.mocks.ts", + "note": "The _formatted fields have been removed from this result and should be re-added programatically when mocking.", + "results": [ + { + "indexUid": "studio_content", + "hits": [ + { + "display_name": "Test Unit", + "block_id": "test-unit-9284e2", + "id": "lctAximTESTunittest-unit-9284e2-a9a4386e", + "type": "library_container", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1742221203.895054, + "modified": 1742221203.895054, + "usage_key": "lct:org:lib:unit:test-unit-9a207", + "block_type": "unit", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "num_children": 0, + "_formatted": { + "display_name": "Test Unit", + "block_id": "test-unit-9284e2", + "id": "lctAximTESTunittest-unit-9284e2-a9a4386e", + "type": "library_container", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": "1742221203.895054", + "modified": "1742221203.895054", + "usage_key": "lct:org:lib:unit:test-unit-9a207", + "block_type": "unit", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": "15", + "num_children": "0", + "published": { + "display_name": "Published Test Unit" + } + }, + "published": { + "display_name": "Published Test Unit" + } + } + ], + "query": "", + "processingTimeMs": 1, + "limit": 20, + "offset": 0, + "estimatedTotalHits": 10 + } + ] +} diff --git a/src/library-authoring/generic/index.scss b/src/library-authoring/generic/index.scss index aa5bf5cc5..210a91092 100644 --- a/src/library-authoring/generic/index.scss +++ b/src/library-authoring/generic/index.scss @@ -1,2 +1,3 @@ @import "./history-widget/HistoryWidget"; @import "./status-widget/StatusWidget"; +@import "./parent-breadcrumbs"; diff --git a/src/library-authoring/generic/parent-breadcrumbs/ParentBreadcrumbs.test.tsx b/src/library-authoring/generic/parent-breadcrumbs/ParentBreadcrumbs.test.tsx new file mode 100644 index 000000000..a4b86e025 --- /dev/null +++ b/src/library-authoring/generic/parent-breadcrumbs/ParentBreadcrumbs.test.tsx @@ -0,0 +1,99 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { BrowserRouter } from 'react-router-dom'; + +import { ContainerType } from '@src/generic/key-utils'; + +import { type ContainerParents, ParentBreadcrumbs } from '.'; + +const mockNavigate = jest.fn(); + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useNavigate: () => mockNavigate, +})); + +const renderComponent = (containerType: ContainerType, parents: ContainerParents) => ( + render( + + + + + , + ) +); + +describe('', () => { + it('show breadcrumb without parent', async () => { + renderComponent(ContainerType.Unit, { displayName: [], key: [] }); + const links = screen.queryAllByRole('link'); + expect(links).toHaveLength(2); // Library link + Empty link + + expect(links[0]).toHaveTextContent('Library Title'); + expect(links[0]).toHaveProperty('href', 'http://localhost/library/library-id'); + + expect(links[1]).toHaveTextContent(''); // Empty link for no parent + expect(links[1]).toHaveProperty('href', 'http://localhost/'); + }); + + it('show breadcrumb to a unit without one parent', async () => { + renderComponent(ContainerType.Unit, { displayName: ['Parent Subsection'], key: ['subsection-key'] }); + const links = screen.queryAllByRole('link'); + expect(links).toHaveLength(2); // Library link + Parent Subsection link + + expect(links[0]).toHaveTextContent('Library Title'); + expect(links[0]).toHaveProperty('href', 'http://localhost/library/library-id'); + + expect(links[1]).toHaveTextContent('Parent Subsection'); + expect(links[1]).toHaveProperty('href', 'http://localhost/library/library-id/subsection/subsection-key'); + }); + + it('show breadcrumb to a subsection without one parent', async () => { + renderComponent(ContainerType.Subsection, { displayName: ['Parent Section'], key: ['section-key'] }); + const links = screen.queryAllByRole('link'); + expect(links).toHaveLength(2); // Library link + Parent Subsection link + + expect(links[0]).toHaveTextContent('Library Title'); + expect(links[0]).toHaveProperty('href', 'http://localhost/library/library-id'); + + expect(links[1]).toHaveTextContent('Parent Section'); + expect(links[1]).toHaveProperty('href', 'http://localhost/library/library-id/section/section-key'); + }); + + it('should throw an error if displayName and key arrays are not the same length', async () => { + expect(() => renderComponent(ContainerType.Unit, { + displayName: ['Parent 1'], + key: ['key1', 'key2'], + })).toThrow('Parents key and displayName arrays must have the same length.'); + }); + + it('show breadcrumb with multiple parents', async () => { + renderComponent(ContainerType.Unit, { + displayName: ['Parent Subsection 1', 'Parent Subsection 2'], + key: ['subsection-key-1', 'subsection-key-2'], + }); + const links = screen.queryAllByRole('link'); + expect(links).toHaveLength(1); // Library link only. Parents are displayed in a dropdown. + + expect(links[0]).toHaveTextContent('Library Title'); + expect(links[0]).toHaveProperty('href', 'http://localhost/library/library-id'); + + const dropdown = screen.getByRole('button', { name: '2 Subsections' }); + expect(dropdown).toBeInTheDocument(); + + fireEvent.click(dropdown); + + const subsectionLinks = screen.queryAllByRole('link'); + expect(subsectionLinks).toHaveLength(2); // Library link only. Parents are displayed in a dropdown. + + expect(subsectionLinks[0]).toHaveTextContent('Parent Subsection 1'); + expect(subsectionLinks[0]).toHaveProperty('href', 'http://localhost/library/library-id/subsection/subsection-key-1'); + + expect(subsectionLinks[1]).toHaveTextContent('Parent Subsection 2'); + expect(subsectionLinks[1]).toHaveProperty('href', 'http://localhost/library/library-id/subsection/subsection-key-2'); + }); +}); diff --git a/src/library-authoring/generic/parent-breadcrumbs/index.scss b/src/library-authoring/generic/parent-breadcrumbs/index.scss new file mode 100644 index 000000000..fdcee836e --- /dev/null +++ b/src/library-authoring/generic/parent-breadcrumbs/index.scss @@ -0,0 +1,13 @@ +.breadcrumb-menu { + button { + padding: 0; + } +} + +.parents-breadcrumb { + max-width: 700px; + display: block; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} diff --git a/src/library-authoring/generic/parent-breadcrumbs/index.tsx b/src/library-authoring/generic/parent-breadcrumbs/index.tsx new file mode 100644 index 000000000..5bc7a3c19 --- /dev/null +++ b/src/library-authoring/generic/parent-breadcrumbs/index.tsx @@ -0,0 +1,121 @@ +import type { ReactNode } from 'react'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { Link } from 'react-router-dom'; +import { + Breadcrumb, MenuItem, SelectMenu, +} from '@openedx/paragon'; +import { ContainerType } from '@src/generic/key-utils'; +import type { ContentLibrary } from '../../data/api'; +import messages from './messages'; + +interface OverflowLinksProps { + to: string | string[]; + children: ReactNode | ReactNode[]; + containerType: ContainerType; +} + +const OverflowLinks = ({ children, to, containerType }: OverflowLinksProps) => { + const intl = useIntl(); + + if (typeof to === 'string') { + return ( + + {children} + + ); + } + + // istanbul ignore if: this should never happen + if (!Array.isArray(to) || !Array.isArray(children) || to.length !== children.length) { + throw new Error('Both "to" and "children" should have the same length.'); + } + + // to is string[] that should be converted to overflow menu + const items = to.map((link, index) => ( + + {children[index]} + + )); + + const containerTypeName = containerType === ContainerType.Unit + ? intl.formatMessage(messages.breadcrumbsSubsectionsDropdown) + : intl.formatMessage(messages.breadcrumbsSectionsDropdown); + + return ( + + {items} + + ); +}; + +export interface ContainerParents { + displayName?: string[]; + key?: string[]; +} + +type ContentLibraryPartial = Pick & Partial; + +interface ParentBreadcrumbsProps { + libraryData: ContentLibraryPartial; + parents?: ContainerParents; + containerType: ContainerType; +} + +export const ParentBreadcrumbs = ({ libraryData, parents, containerType }: ParentBreadcrumbsProps) => { + const intl = useIntl(); + const { id: libraryId, title: libraryTitle } = libraryData; + + const links: Array<{ label: string | string[], to: string | string[], containerType: ContainerType }> = [ + { + label: libraryTitle, + to: `/library/${libraryId}`, + containerType, + }, + ]; + + const parentLength = parents?.key?.length || 0; + const parentNameLength = parents?.displayName?.length || 0; + + if (parentLength !== parentNameLength) { + throw new Error('Parents key and displayName arrays must have the same length.'); + } + + const parentType = containerType === ContainerType.Unit + ? 'subsection' + : 'section'; + + if (parentLength === 0 || !parents) { + // Adding empty breadcrumb to add the last `>` spacer. + links.push({ + label: '', + to: '', + containerType, + }); + } else if (parentLength === 1) { + links.push({ + label: parents.displayName?.[0] || '', + to: `/library/${libraryId}/${parentType}/${parents.key?.[0]}`, + containerType, + }); + } else { + // Add all parents as a single object containing list of links + // This is converted to overflow menu by OverflowLinks component + links.push({ + label: parents.displayName || [], + to: parents.key?.map((parentKey) => `/library/${libraryId}/${parentType}/${parentKey}`) || [], + containerType, + }); + } + + return ( + + ); +}; diff --git a/src/library-authoring/generic/parent-breadcrumbs/messages.ts b/src/library-authoring/generic/parent-breadcrumbs/messages.ts new file mode 100644 index 000000000..babb034fa --- /dev/null +++ b/src/library-authoring/generic/parent-breadcrumbs/messages.ts @@ -0,0 +1,21 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + breadcrumbsAriaLabel: { + id: 'course-authoring.library-authoring.parent-breadcrumbs.label.text', + defaultMessage: 'Navigation breadcrumbs', + description: 'Aria label for navigation breadcrumbs', + }, + breadcrumbsSectionsDropdown: { + id: 'course-authoring.library-authoring.parent-breadcrumbs.dropdown.sections', + defaultMessage: 'Sections', + description: 'Title for dropdown menu containing sections', + }, + breadcrumbsSubsectionsDropdown: { + id: 'course-authoring.library-authoring.parent-breadcrumbs.dropdown.subsections', + defaultMessage: 'Subsections', + description: 'Title for dropdown menu containing subsections', + }, +}); + +export default messages; diff --git a/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx b/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx index 661477afe..5839d6575 100644 --- a/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx +++ b/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx @@ -1,10 +1,8 @@ -import { ReactNode, useMemo } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Helmet } from 'react-helmet'; -import { - Breadcrumb, Container, MenuItem, SelectMenu, -} from '@openedx/paragon'; -import { Link } from 'react-router-dom'; +import { Container } from '@openedx/paragon'; + +import type { ContainerHit } from '@src/search-manager'; import { useLibraryContext } from '../common/context/LibraryContext'; import { useSidebarContext } from '../common/context/SidebarContext'; import { useContentFromSearchIndex, useContentLibrary } from '../data/apiHooks'; @@ -15,41 +13,11 @@ import { ContainerType } from '../../generic/key-utils'; import Header from '../../header'; import SubHeader from '../../generic/sub-header/SubHeader'; import { SubHeaderTitle } from '../LibraryAuthoringPage'; -import { messages, subsectionMessages } from './messages'; +import { subsectionMessages } from './messages'; import { LibrarySidebar } from '../library-sidebar'; +import { ParentBreadcrumbs } from '../generic/parent-breadcrumbs'; import { LibraryContainerChildren } from './LibraryContainerChildren'; import { ContainerEditableTitle, FooterActions, HeaderActions } from '../containers'; -import { ContainerHit } from '../../search-manager'; - -interface OverflowLinksProps { - to: string | string[]; - children: ReactNode | ReactNode[]; -} - -const OverflowLinks = ({ children, to }: OverflowLinksProps) => { - if (typeof to === 'string') { - return ( - - {children} - - ); - } - // to is string[] that should be converted to overflow menu - const items = to?.map((link, index) => ( - - {children?.[index]} - - )); - return ( - - {items} - - ); -}; /** Full library subsection page */ export const LibrarySubsectionPage = () => { @@ -64,42 +32,6 @@ export const LibrarySubsectionPage = () => { } = useContentFromSearchIndex(containerId ? [containerId] : []); const subsectionData = (hits as ContainerHit[])?.[0]; - const breadcrumbs = useMemo(() => { - const links: Array<{ label: string | string[], to: string | string[] }> = [ - { - label: libraryData?.title || '', - to: `/library/${libraryId}`, - }, - ]; - const sectionLength = subsectionData?.sections?.displayName?.length || 0; - if (sectionLength === 1) { - links.push({ - label: subsectionData.sections?.displayName?.[0] || '', - to: `/library/${libraryId}/section/${subsectionData?.sections?.key?.[0]}`, - }); - } else if (sectionLength > 1) { - // Add all sections as a single object containing list of links - // This is converted to overflow menu by OverflowLinks component - links.push({ - label: subsectionData?.sections?.displayName || '', - to: subsectionData?.sections?.key?.map((link) => `/library/${libraryId}/section/${link}`) || '', - }); - } else { - // Adding empty breadcrumb to add the last `>` spacer. - links.push({ - label: '', - to: '', - }); - } - return ( - - ); - }, [libraryData, subsectionData, libraryId]); - if (!containerId || !libraryId) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. throw new Error('Rendered without containerId or libraryId URL parameter'); @@ -141,7 +73,13 @@ export const LibrarySubsectionPage = () => {
} />} - breadcrumbs={breadcrumbs} + breadcrumbs={( + + )} headerActions={( { + const queryFilter = requestData?.queries[0]?.filter?.[1]; + const subsectionId = queryFilter?.split('usage_key IN ["')[1].split('"]')[0]; + switch (subsectionId) { + case mockGetContainerMetadata.unitIdLoading: + return new Promise(() => {}); + case mockGetContainerMetadata.unitIdError: + return Promise.reject(new Error('Not found')); + default: + return mockResult; + } +}; +mockSearchResult(mockResult, searchFilterfn); const verticalSortableListCollisionDetection = jest.fn(); jest.mock('../../generic/DraggableList/verticalSortableList', () => ({ @@ -107,13 +122,13 @@ describe('', () => { it('shows empty unit', async () => { renderLibraryUnitPage(mockGetContainerMetadata.unitIdEmpty); - expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar expect(await screen.findByText('This unit is empty')).toBeInTheDocument(); }); it('can rename unit', async () => { renderLibraryUnitPage(); - expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar const editUnitTitleButton = screen.getAllByRole( 'button', @@ -144,7 +159,7 @@ describe('', () => { it('show error if renaming unit fails', async () => { renderLibraryUnitPage(); - expect((await screen.findAllByText(libraryTitle))[0]).toBeInTheDocument(); + expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar const editUnitTitleButton = screen.getAllByRole( 'button', @@ -161,6 +176,8 @@ describe('', () => { const textBox = screen.getByRole('textbox', { name: /text input/i }); expect(textBox).toBeInTheDocument(); + expect(textBox).toHaveValue('Test Unit'); + screen.logTestingPlaygroundURL(); fireEvent.change(textBox, { target: { value: 'New Unit Title' } }); fireEvent.keyDown(textBox, { key: 'Enter', code: 'Enter', charCode: 13 }); @@ -195,7 +212,7 @@ describe('', () => { it('should open and close component sidebar on component selection', async () => { renderLibraryUnitPage(); - expect((await screen.findAllByText('Test Unit')).length).toBeGreaterThan(1); + expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar // No Preview tab shown in sidebar expect(screen.queryByText('Preview')).not.toBeInTheDocument(); @@ -220,6 +237,7 @@ describe('', () => { const url = getXBlockFieldsApiUrl('lb:org1:Demo_course_generated:html:text-0'); axiosMock.onPost(url).reply(200); renderLibraryUnitPage(); + expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar // Wait loading of the component await screen.findByText('text block 0'); @@ -254,6 +272,7 @@ describe('', () => { const url = getXBlockFieldsApiUrl('lb:org1:Demo_course_generated:html:text-0'); axiosMock.onPost(url).reply(400); renderLibraryUnitPage(); + expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar // Wait loading of the component await screen.findByText('text block 0'); diff --git a/src/library-authoring/units/LibraryUnitPage.tsx b/src/library-authoring/units/LibraryUnitPage.tsx index 1ab46f7b5..170c6cfc3 100644 --- a/src/library-authoring/units/LibraryUnitPage.tsx +++ b/src/library-authoring/units/LibraryUnitPage.tsx @@ -1,20 +1,20 @@ import { useIntl } from '@edx/frontend-platform/i18n'; -import { - Breadcrumb, - Container, -} from '@openedx/paragon'; +import { Container } from '@openedx/paragon'; import { Helmet } from 'react-helmet'; -import { Link } from 'react-router-dom'; +import ErrorAlert from '@src/generic/alert-error'; +import { ContainerType } from '@src/generic/key-utils'; +import type { ContainerHit } from '@src/search-manager'; import Loading from '../../generic/Loading'; import NotFoundAlert from '../../generic/NotFoundAlert'; import SubHeader from '../../generic/sub-header/SubHeader'; -import ErrorAlert from '../../generic/alert-error'; import Header from '../../header'; + import { useLibraryContext } from '../common/context/LibraryContext'; import { useSidebarContext } from '../common/context/SidebarContext'; -import { useContainer, useContentLibrary } from '../data/apiHooks'; +import { useContentFromSearchIndex, useContentLibrary } from '../data/apiHooks'; import { LibrarySidebar } from '../library-sidebar'; +import { ParentBreadcrumbs } from '../generic/parent-breadcrumbs'; import { SubHeaderTitle } from '../LibraryAuthoringPage'; import { LibraryUnitBlocks } from './LibraryUnitBlocks'; import messages from './messages'; @@ -36,12 +36,11 @@ export const LibraryUnitPage = () => { const { sidebarItemInfo } = useSidebarContext(); const { data: libraryData, isLoading: isLibLoading } = useContentLibrary(libraryId); + // fetch unitData from index as it includes its parent subsections as well. const { - data: unitData, - isLoading, - isError, - error, - } = useContainer(containerId); + hits, isLoading, isError, error, + } = useContentFromSearchIndex(containerId ? [containerId] : []); + const unitData = (hits as ContainerHit[])?.[0]; if (!containerId || !libraryId) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. @@ -62,24 +61,6 @@ export const LibraryUnitPage = () => { return ; } - const breadcrumbs = ( - ` spacer. - { - label: '', - to: '', - }, - ]} - linkAs={Link} - /> - ); - return (
@@ -105,7 +86,13 @@ export const LibraryUnitPage = () => { addContentBtnText={intl.formatMessage(messages.addContentButton)} /> )} - breadcrumbs={breadcrumbs} + breadcrumbs={( + + )} hideBorder />