fix: clean up invalid breadcrumb usages in library updates (#2811)

Author: bradenmacdonald

src/container-comparison/CompareContainersWidget.test.tsx

@@ -68,8 +68,8 @@ describe('CompareContainersWidget', () => {
     let block = await screen.findByText('subsection block 00');
     await user.click(block);
     // Breadcrumbs - shows old and new name
-    expect(await screen.findByRole('button', { name: 'subsection block 00' })).toBeInTheDocument();
-    expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument();
+    expect(await screen.findByRole('heading', { name: 'subsection block 00' })).toBeInTheDocument();
+    expect(await screen.findByRole('heading', { name: 'subsection block 0' })).toBeInTheDocument();
 
     // Back breadcrumb
     const backbtns = await screen.findAllByRole('button', { name: 'Back' });
@@ -83,8 +83,8 @@ describe('CompareContainersWidget', () => {
 
     // After side click also works
     await user.click(block);
-    expect(await screen.findByRole('button', { name: 'subsection block 00' })).toBeInTheDocument();
-    expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument();
+    expect(await screen.findByRole('heading', { name: 'subsection block 00' })).toBeInTheDocument();
+    expect(await screen.findByRole('heading', { name: 'subsection block 0' })).toBeInTheDocument();
   });
 
   test('should show removed container diff state', async () => {

src/container-comparison/CompareContainersWidget.tsx

@@ -1,10 +1,17 @@
 import { useCallback, useMemo, useState } from 'react';
-
 import {
   Alert,
-  Breadcrumb, Button, Card, Icon, Stack,
+  Button,
+  Card,
+  Icon,
+  Stack,
 } from '@openedx/paragon';
-import { ArrowBack, Add, Delete } from '@openedx/paragon/icons';
+import {
+  Add,
+  ArrowBack,
+  ChevronRight,
+  Delete,
+} from '@openedx/paragon/icons';
 import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
 
 import { ContainerType, getBlockType } from '@src/generic/key-utils';
@@ -148,25 +155,15 @@ const CompareContainersWidgetInner = ({
       return title;
     }
     return (
-      <Breadcrumb
-        ariaLabel={intl.formatMessage(messages.breadcrumbAriaLabel)}
-        links={[
-          {
-            // This raises failed prop-type error as label expects a string but it works without any issues
-            label: <Stack direction="horizontal" gap={1}><Icon size="xs" src={ArrowBack} />Back</Stack>,
-            onClick: onBackBtnClick,
-            variant: 'link',
-            className: 'px-0 text-gray-900',
-          },
-          {
-            label: title,
-            variant: 'link',
-            className: 'px-0 text-gray-900',
-            disabled: true,
-          },
-        ]}
-        linkAs={Button}
-      />
+      <Stack direction="horizontal" gap={1}>
+        <Button variant="link" className="px-0 text-gray-900" onClick={onBackBtnClick}>
+          {/* We could also use iconBefore={ArrowBack} on the <Button> above but it's a bit too big that way. */}
+          <Icon size="xs" src={ArrowBack} className="mr-1" />
+          {intl.formatMessage(messages.breadcrumbBackLabel)}
+        </Button>
+        <Icon size="md" src={ChevronRight} />
+        <span role="heading" aria-level={3}>{title}</span>
+      </Stack>
     );
   }, [parent]);
 

src/container-comparison/messages.ts

@@ -68,8 +68,13 @@ const messages = defineMessages({
   },
   breadcrumbAriaLabel: {
     id: 'course-authoring.container-comparison.diff.breadcrumb.ariaLabel',
-    defaultMessage: 'Title breadcrumb',
-    description: 'Aria label text for breadcrumb in diff preview',
+    defaultMessage: 'Location',
+    description: 'Accessible label for the breadcrumbs which display the location of the unit, e.g. Section 1 > Unit 4',
+  },
+  breadcrumbBackLabel: {
+    id: 'course-authoring.container-comparison.diff.breadcrumb.backLabel',
+    defaultMessage: 'Back',
+    description: 'Link to go back to the parent section/subsection',
   },
   diffBeforeTitle: {
     id: 'course-authoring.container-comparison.diff.before.title',

src/course-unit/move-modal/index.tsx

@@ -50,7 +50,7 @@ const MoveModal: FC<IUseMoveModalParams> = ({
         { label: breadcrumb, 'data-parent-index': index }
       ))}
       activeLabel={breadcrumbs[breadcrumbs.length - 1]}
-      clickHandler={({ target }) => handleBreadcrumbsClick(target.dataset.parentIndex)}
+      clickHandler={({ currentTarget }) => handleBreadcrumbsClick(currentTarget.dataset.parentIndex!)}
     />
   ), [isExtraSmall, breadcrumbs, handleBreadcrumbsClick]);
 

src/library-authoring/generic/parent-breadcrumbs/index.tsx

@@ -105,7 +105,7 @@ export const ParentBreadcrumbs = ({ libraryData, parents, containerType }: Paren
     // 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 || [],
+      label: parents.displayName || '',
       to: parents.key?.map((parentKey) => `/library/${libraryId}/${parentType}/${parentKey}`) || [],
       containerType,
     });
@@ -114,6 +114,8 @@ export const ParentBreadcrumbs = ({ libraryData, parents, containerType }: Paren
   return (
     <Breadcrumb
       ariaLabel={intl.formatMessage(messages.breadcrumbsAriaLabel)}
+      // @ts-ignore FIXME: <Breadcrumb> uses `label` as a key, so it shouldn't be an array (string[]),
+      // even if our custom <OverflowLinks> is handling arrays properly.
       links={links}
       linkAs={OverflowLinks}
     />