refactor: improve library sub header (#1573)

Changes the Library (and Collection) subheader
2024-12-18 14:56:21 -03:00
parent 64906a1b9d
commit 230960b711
6 changed files with 94 additions and 105 deletions
--- a/src/hooks.ts
+++ b/src/hooks.ts
@@ -50,11 +50,11 @@ export const useLoadOnScroll = (
  useEffect(() => {
    if (enabled) {
      const canFetchNextPage = hasNextPage && !isFetchingNextPage;
+      // Used `loadLimit` to fetch next page before reach the end of the screen.
+      const loadLimit = 300;

      const onscroll = () => {
        // Verify the position of the scroll to implement an infinite scroll.
-        // Used `loadLimit` to fetch next page before reach the end of the screen.
-        const loadLimit = 300;
        const scrolledTo = window.scrollY + window.innerHeight;
        const scrollDiff = document.body.scrollHeight - scrolledTo;
        const isNearToBottom = scrollDiff <= loadLimit;
@@ -65,7 +65,7 @@ export const useLoadOnScroll = (
      window.addEventListener('scroll', onscroll);

      // If the content is less than the screen height, fetch the next page.
-      const hasNoScroll = document.body.scrollHeight <= window.innerHeight;
+      const hasNoScroll = (document.body.scrollHeight - loadLimit) <= window.innerHeight;
      if (hasNoScroll && canFetchNextPage) {
        fetchNextPage();
      }
--- a/src/library-authoring/LibraryAuthoringPage.tsx
+++ b/src/library-authoring/LibraryAuthoringPage.tsx
@@ -4,6 +4,7 @@ import classNames from 'classnames';
 import { StudioFooter } from '@edx/frontend-component-footer';
 import { useIntl } from '@edx/frontend-platform/i18n';
 import {
+  ActionRow,
  Alert,
  Badge,
  Breadcrumb,
@@ -57,12 +58,10 @@ const HeaderActions = () => {

  const { componentPickerMode } = useComponentPickerContext();

-  const infoSidebarIsOpen = () => (
-    sidebarComponentInfo?.type === SidebarBodyComponentId.Info
-  );
+  const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.Info;

  const handleOnClickInfoSidebar = () => {
-    if (infoSidebarIsOpen()) {
+    if (infoSidebarIsOpen) {
      closeLibrarySidebar();
    } else {
      openInfoSidebar();
@@ -73,8 +72,8 @@ const HeaderActions = () => {
    <div className="header-actions">
      <Button
        className={classNames('mr-1', {
-          'normal-border': !infoSidebarIsOpen(),
-          'open-border': infoSidebarIsOpen(),
+          'normal-border': !infoSidebarIsOpen,
+          'open-border': infoSidebarIsOpen,
        })}
        iconBefore={InfoOutline}
        variant="outline-primary rounded-0"
@@ -97,7 +96,7 @@ const HeaderActions = () => {
  );
 };

-const SubHeaderTitle = ({ title }: { title: string }) => {
+export const SubHeaderTitle = ({ title }: { title: string }) => {
  const intl = useIntl();

  const { readOnly } = useLibraryContext();
@@ -143,7 +142,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
  } = useLibraryContext();
  const { openInfoSidebar, sidebarComponentInfo } = useSidebarContext();

-  const [activeKey, setActiveKey] = useState<ContentType | undefined>(ContentType.home);
+  const [activeKey, setActiveKey] = useState<ContentType>(ContentType.home);

  useEffect(() => {
    const currentPath = location.pathname.split('/').pop();
@@ -151,7 +150,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
    if (componentPickerMode || currentPath === libraryId || currentPath === '') {
      setActiveKey(ContentType.home);
    } else if (currentPath && currentPath in ContentType) {
-      setActiveKey(ContentType[currentPath]);
+      setActiveKey(ContentType[currentPath] || ContentType.home);
    }
  }, []);

@@ -175,11 +174,6 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
    );
  }

-  // istanbul ignore if: this should never happen
-  if (activeKey === undefined) {
-    return <NotFoundAlert />;
-  }
-
  if (!libraryData) {
    return <NotFoundAlert />;
  }
@@ -249,15 +243,8 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
              subtitle={!componentPickerMode ? intl.formatMessage(messages.headingSubtitle) : undefined}
              breadcrumbs={breadcumbs}
              headerActions={<HeaderActions />}
+              hideBorder
            />
-            <SearchKeywordsField className="w-50" />
-            <div className="d-flex mt-3 align-items-center">
-              <FilterByTags />
-              <FilterByBlockType />
-              <ClearFiltersButton />
-              <div className="flex-grow-1" />
-              <SearchSortWidget />
-            </div>
            <Tabs
              variant="tabs"
              activeKey={activeKey}
@@ -268,6 +255,14 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
              <Tab eventKey={ContentType.components} title={intl.formatMessage(messages.componentsTab)} />
              <Tab eventKey={ContentType.collections} title={intl.formatMessage(messages.collectionsTab)} />
            </Tabs>
+            <ActionRow className="my-3">
+              <SearchKeywordsField className="mr-3" />
+              <FilterByTags />
+              <FilterByBlockType />
+              <ClearFiltersButton />
+              <ActionRow.Spacer />
+              <SearchSortWidget />
+            </ActionRow>
            <LibraryContent contentType={activeKey} />
          </SearchContextProvider>
        </Container>
--- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx
+++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx
@@ -150,7 +150,7 @@ describe('<LibraryCollectionPage />', () => {
    expect(screen.queryByText('Read Only')).not.toBeInTheDocument();
  });

-  it('shows an empty read-only library collection, without a new button', async () => {
+  it('shows an empty read-only library collection, with the new button disabled', async () => {
    // Use a library mock that is read-only:
    const libraryId = mockContentLibrary.libraryIdReadOnly;
    // Update search mock so it returns no results:
@@ -161,7 +161,8 @@ describe('<LibraryCollectionPage />', () => {
    // Show in the collection page and in the sidebar
    expect(screen.getAllByText('This collection is currently empty.').length).toEqual(2);

-    expect(screen.queryByRole('button', { name: /new/i })).not.toBeInTheDocument();
+    expect(screen.queryByRole('button', { name: /new/i })).toBeInTheDocument();
+    expect(screen.queryByRole('button', { name: /new/i })).toBeDisabled();
    expect(screen.getByText('Read Only')).toBeInTheDocument();
  });

@@ -230,14 +231,14 @@ describe('<LibraryCollectionPage />', () => {
    expect((await screen.findAllByText(title))[0]).toBeInTheDocument();
    expect((await screen.findAllByText(title))[1]).toBeInTheDocument();

-    // Open by default; close the library info sidebar
-    const closeButton = screen.getByRole('button', { name: /close/i });
-    fireEvent.click(closeButton);
+    const collectionInfoBtn = screen.getByRole('button', { name: /collection info/i });
+
+    // Open by default; click 'Collection info' button to close
+    fireEvent.click(collectionInfoBtn);
    expect(screen.queryByText('Draft')).not.toBeInTheDocument();
    expect(screen.queryByText('(Never Published)')).not.toBeInTheDocument();

-    // Open library info sidebar with 'Library info' button
-    const collectionInfoBtn = screen.getByRole('button', { name: /collection info/i });
+    // Open library info sidebar with 'Collection info' button
    fireEvent.click(collectionInfoBtn);
    expect(screen.getByText('Manage')).toBeInTheDocument();
    expect(screen.getByText('Details')).toBeInTheDocument();
--- a/src/library-authoring/collections/LibraryCollectionPage.tsx
+++ b/src/library-authoring/collections/LibraryCollectionPage.tsx
@@ -2,15 +2,15 @@ import { useEffect } from 'react';
 import { StudioFooter } from '@edx/frontend-component-footer';
 import { useIntl } from '@edx/frontend-platform/i18n';
 import {
-  Badge,
+  ActionRow,
  Button,
  Breadcrumb,
  Container,
  Icon,
-  IconButton,
-  Stack,
 } from '@openedx/paragon';
 import { Add, ArrowBack, InfoOutline } from '@openedx/paragon/icons';
+import classNames from 'classnames';
+import { Helmet } from 'react-helmet';
 import { Link } from 'react-router-dom';

 import Loading from '../../generic/Loading';
@@ -26,71 +26,68 @@ import {
  SearchKeywordsField,
  SearchSortWidget,
 } from '../../search-manager';
+import { SubHeaderTitle } from '../LibraryAuthoringPage';
 import { useCollection, useContentLibrary } from '../data/apiHooks';
 import { useComponentPickerContext } from '../common/context/ComponentPickerContext';
 import { useLibraryContext } from '../common/context/LibraryContext';
-import { useSidebarContext } from '../common/context/SidebarContext';
+import { SidebarBodyComponentId, useSidebarContext } from '../common/context/SidebarContext';
 import messages from './messages';
 import { LibrarySidebar } from '../library-sidebar';
 import LibraryCollectionComponents from './LibraryCollectionComponents';

 const HeaderActions = () => {
  const intl = useIntl();
-  const { readOnly } = useLibraryContext();
-  const { openAddContentSidebar } = useSidebarContext();

-  if (readOnly) {
-    return null;
+  const { componentPickerMode } = useComponentPickerContext();
+  const { collectionId, readOnly } = useLibraryContext();
+  const {
+    closeLibrarySidebar,
+    openAddContentSidebar,
+    openCollectionInfoSidebar,
+    sidebarComponentInfo,
+  } = useSidebarContext();
+
+  // istanbul ignore if: this should never happen
+  if (!collectionId) {
+    throw new Error('it should not be possible to render HeaderActions without a collectionId');
  }

+  const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.CollectionInfo
+    && sidebarComponentInfo?.id === collectionId;
+
+  const handleOnClickInfoSidebar = () => {
+    if (infoSidebarIsOpen) {
+      closeLibrarySidebar();
+    } else {
+      openCollectionInfoSidebar(collectionId);
+    }
+  };
+
  return (
    <div className="header-actions">
      <Button
-        className="ml-1"
-        iconBefore={Add}
-        variant="primary rounded-0"
-        onClick={openAddContentSidebar}
+        className={classNames('mr-1', {
+          'normal-border': !infoSidebarIsOpen,
+          'open-border': infoSidebarIsOpen,
+        })}
+        iconBefore={InfoOutline}
+        variant="outline-primary rounded-0"
+        onClick={handleOnClickInfoSidebar}
      >
-        {intl.formatMessage(messages.newContentButton)}
+        {intl.formatMessage(messages.collectionInfoButton)}
      </Button>
-    </div>
-  );
-};
-
-const SubHeaderTitle = ({
-  title,
-  infoClickHandler,
-}: {
-  title: string;
-  infoClickHandler: () => void;
-}) => {
-  const intl = useIntl();
-
-  const { componentPickerMode } = useComponentPickerContext();
-  const { readOnly } = useLibraryContext();
-
-  const showReadOnlyBadge = readOnly && !componentPickerMode;
-
-  return (
-    <Stack direction="vertical">
-      <Stack direction="horizontal" gap={2}>
-        {title}
-        <IconButton
-          src={InfoOutline}
-          iconAs={Icon}
-          alt={intl.formatMessage(messages.collectionInfoButton)}
-          onClick={infoClickHandler}
-          variant="primary"
-        />
-      </Stack>
-      {showReadOnlyBadge && (
-        <div>
-          <Badge variant="primary" style={{ fontSize: '50%' }}>
-            {intl.formatMessage(messages.readOnlyBadge)}
-          </Badge>
-        </div>
+      {!componentPickerMode && (
+        <Button
+          className="ml-1"
+          iconBefore={Add}
+          variant="primary rounded-0"
+          onClick={openAddContentSidebar}
+          disabled={readOnly}
+        >
+          {intl.formatMessage(messages.newContentButton)}
+        </Button>
      )}
-    </Stack>
+    </div>
  );
 };

@@ -181,6 +178,7 @@ const LibraryCollectionPage = () => {
  return (
    <div className="d-flex">
      <div className="flex-grow-1">
+        <Helmet><title>{libraryData.title} | {process.env.SITE_NAME}</title></Helmet>
        {!componentPickerMode && (
          <Header
            number={libraryData.slug}
@@ -188,34 +186,33 @@ const LibraryCollectionPage = () => {
            org={libraryData.org}
            contextId={libraryId}
            isLibrary
+            containerProps={{
+              size: undefined,
+            }}
          />
        )}
-        <Container size="xl" className="px-4 mt-4 mb-5 library-authoring-page">
+        <Container className="px-4 mt-4 mb-5 library-authoring-page">
          <SearchContextProvider
            extraFilter={extraFilter}
          >
            <SubHeader
-              title={(
-                <SubHeaderTitle
-                  title={collectionData.title}
-                  infoClickHandler={() => openCollectionInfoSidebar(collectionId)}
-                />
-              )}
+              title={<SubHeaderTitle title={collectionData.title} />}
              breadcrumbs={breadcumbs}
              headerActions={<HeaderActions />}
+              hideBorder
            />
-            <SearchKeywordsField className="w-50" placeholder={intl.formatMessage(messages.searchPlaceholder)} />
-            <div className="d-flex mt-3 mb-4 align-items-center">
+            <ActionRow className="my-3">
+              <SearchKeywordsField className="mr-3" />
              <FilterByTags />
              <FilterByBlockType />
              <ClearFiltersButton />
-              <div className="flex-grow-1" />
+              <ActionRow.Spacer />
              <SearchSortWidget />
-            </div>
+            </ActionRow>
            <LibraryCollectionComponents />
          </SearchContextProvider>
        </Container>
-        <StudioFooter />
+        {!componentPickerMode && <StudioFooter containerProps={{ size: undefined }} />}
      </div>
      {!!sidebarComponentInfo?.type && (
        <div className="library-authoring-sidebar box-shadow-left-1 bg-white" data-testid="library-sidebar">
--- a/src/library-authoring/collections/messages.ts
+++ b/src/library-authoring/collections/messages.ts
@@ -76,11 +76,6 @@ const messages = defineMessages({
    defaultMessage: 'Collection Info',
    description: 'Alt text for collection info button besides the collection title',
  },
-  readOnlyBadge: {
-    id: 'course-authoring.library-authoring.collections.badge.read-only',
-    defaultMessage: 'Read Only',
-    description: 'Text in badge when the user has read only access in collections page',
-  },
  allCollections: {
    id: 'course-authoring.library-authoring.all-collections.text',
    defaultMessage: 'All Collections',
@@ -91,11 +86,6 @@ const messages = defineMessages({
    defaultMessage: 'Navigation breadcrumbs',
    description: 'Aria label for navigation breadcrumbs',
  },
-  searchPlaceholder: {
-    id: 'course-authoring.library-authoring.search.placeholder.text',
-    defaultMessage: 'Search Collection',
-    description: 'Search placeholder text in collections page.',
-  },
  noSearchResultsCollections: {
    id: 'course-authoring.library-authoring.no-search-results-collections',
    defaultMessage: 'No matching collections found in this library.',
--- a/src/search-manager/FilterByBlockType.tsx
+++ b/src/search-manager/FilterByBlockType.tsx
@@ -262,6 +262,8 @@ const FilterByBlockType: React.FC<Record<never, never>> = () => {
    blockType => ({ label: <BlockTypeLabel blockType={blockType} /> }),
  );

+  const hiddenBlockTypes = blockTypesFilter.filter(blockType => !Object.keys(blockTypes).includes(blockType));
+
  return (
    <SearchFilterWidget
      appliedFilters={appliedFilters}
@@ -275,6 +277,10 @@ const FilterByBlockType: React.FC<Record<never, never>> = () => {
          value={blockTypesFilter}
        >
          <Menu className="block-type-refinement-menu" style={{ boxShadow: 'none' }}>
+            {
+              // Show applied filter items for block types that are not in the current search results
+              hiddenBlockTypes.map(blockType => <FilterItem key={blockType} blockType={blockType} count={0} />)
+            }
            {
              Object.entries(sortedBlockTypes).map(([blockType, count]) => (
                <FilterItem key={blockType} blockType={blockType} count={count} />
@@ -282,7 +288,7 @@ const FilterByBlockType: React.FC<Record<never, never>> = () => {
            }
            {
              // Show a message if there are no options at all to avoid the impression that the dropdown isn't working
-              Object.keys(sortedBlockTypes).length === 0 ? (
+              Object.keys(sortedBlockTypes).length === 0 && hiddenBlockTypes.length === 0 ? (
                <MenuItem disabled><FormattedMessage {...messages['blockTypeFilter.empty']} /></MenuItem>
              ) : null
            }