[Teak] fix: Inconsistent publish status filter menu placement & fix: Remove never published filter from component picker (#2021)

* fix: Inconsistent publish status filter menu placement (#1966)

* fix: Remove never published filter from component picker (#1947)

Removes the never-published filter option from the component picker and unit picker.

Author: ChrisChV

src/library-authoring/LibraryAuthoringPage.tsx

@@ -32,7 +32,6 @@ import {
   ClearFiltersButton,
   FilterByBlockType,
   FilterByTags,
-  FilterByPublished,
   SearchContextProvider,
   SearchKeywordsField,
   SearchSortWidget,
@@ -46,6 +45,7 @@ import { SidebarBodyComponentId, useSidebarContext } from './common/context/Side
 import { allLibraryPageTabs, ContentType, useLibraryRoutes } from './routes';
 
 import messages from './messages';
+import LibraryFilterByPublished from './generic/filter-by-published';
 
 const HeaderActions = () => {
   const intl = useIntl();
@@ -246,6 +246,17 @@ const LibraryAuthoringPage = ({
     extraFilter.push(activeTypeFilters[activeKey]);
   }
 
+  /*
+
+  <FilterByPublished key={
+                // It is necessary to re-render `FilterByPublished` every time `FilterByBlockType`
+                // appears or disappears, this is because when the menu is opened it is rendered
+                // in a previous state, causing an inconsistency in its position.
+                // By changing the key we can re-render the component.
+                !(insideCollections || insideUnits) ? 'filter-published-1' : 'filter-published-2'
+              }
+  */
+
   // Disable filtering by block/problem type when viewing the Collections tab.
   const overrideTypesFilter = (insideCollections || insideUnits) ? new TypesFilterData() : undefined;
 
@@ -299,7 +310,14 @@ const LibraryAuthoringPage = ({
               <SearchKeywordsField className="mr-3" />
               <FilterByTags />
               {!(insideCollections || insideUnits) && <FilterByBlockType />}
-              <FilterByPublished />
+              <LibraryFilterByPublished key={
+                // It is necessary to re-render `LibraryFilterByPublished` every time `FilterByBlockType`
+                // appears or disappears, this is because when the menu is opened it is rendered
+                // in a previous state, causing an inconsistency in its position.
+                // By changing the key we can re-render the component.
+                !(insideCollections || insideUnits) ? 'filter-published-1' : 'filter-published-2'
+              }
+              />
               <ClearFiltersButton />
               <ActionRow.Spacer />
               <SearchSortWidget />

src/library-authoring/collections/LibraryCollectionPage.tsx

@@ -22,7 +22,6 @@ import NotFoundAlert from '../../generic/NotFoundAlert';
 import {
   ClearFiltersButton,
   FilterByBlockType,
-  FilterByPublished,
   FilterByTags,
   SearchContextProvider,
   SearchKeywordsField,
@@ -36,6 +35,7 @@ import { SidebarBodyComponentId, useSidebarContext } from '../common/context/Sid
 import messages from './messages';
 import { LibrarySidebar } from '../library-sidebar';
 import LibraryCollectionComponents from './LibraryCollectionComponents';
+import LibraryFilterByPublished from '../generic/filter-by-published';
 
 const HeaderActions = () => {
   const intl = useIntl();
@@ -218,7 +218,7 @@ const LibraryCollectionPage = () => {
               <SearchKeywordsField className="mr-3" />
               <FilterByTags />
               <FilterByBlockType />
-              <FilterByPublished />
+              <LibraryFilterByPublished />
               <ClearFiltersButton />
               <ActionRow.Spacer />
               <SearchSortWidget />

src/library-authoring/component-picker/ComponentPicker.test.tsx

@@ -302,4 +302,43 @@ describe('<ComponentPicker />', () => {
     expect(screen.queryByRole('tab', { name: /collections/i })).not.toBeInTheDocument();
     expect(screen.queryByRole('tab', { name: /components/i })).not.toBeInTheDocument();
   });
+
+  it('should not display never published filter', async () => {
+    render(<ComponentPicker />);
+
+    expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
+    fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i));
+
+    // Wait for the content library to load
+    const filterButton = await screen.findByRole('button', { name: /publish status/i });
+    fireEvent.click(filterButton);
+
+    // Verify the filters. Note: It's hard to verify the `published` filter,
+    // because there are many components with that text on the screen, but that's not the important thing.
+    expect(screen.getByText(/modified since publish/i)).toBeInTheDocument();
+    expect(screen.queryByText(/never published/i)).not.toBeInTheDocument();
+  });
+
+  it('should not display never published filter in collection page', async () => {
+    render(<ComponentPicker />);
+
+    expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
+    fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i));
+
+    // Wait for the content library to load
+    await screen.findByText(/Change Library/i);
+    expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
+
+    // Click on the collection card to open the sidebar
+    fireEvent.click(screen.queryAllByText('Collection 1')[0]);
+
+    // Wait for the content library to load
+    const filterButton = await screen.findByRole('button', { name: /publish status/i });
+    fireEvent.click(filterButton);
+
+    // Verify the filters. Note: It's hard to verify the `published` filter,
+    // because there are many components with that text on the screen, but that's not the important thing.
+    expect(screen.getByText(/modified since publish/i)).toBeInTheDocument();
+    expect(screen.queryByText(/never published/i)).not.toBeInTheDocument();
+  });
 });

src/library-authoring/generic/filter-by-published/index.tsx

@@ -0,0 +1,26 @@
+import React from 'react';
+import { useLibraryContext } from '../../common/context/LibraryContext';
+import { FilterByPublished, PublishStatus } from '../../../search-manager';
+
+/**
+ * When browsing library content for insertion into a course, we only show published
+ * content. In that case, there is no need for a 'Never Published' filter, which will
+ * never show results. This component removes that option from FilterByPublished
+ * when not relevant.
+ */
+const LibraryFilterByPublished : React.FC<Record<never, never>> = () => {
+  const { showOnlyPublished } = useLibraryContext();
+
+  if (showOnlyPublished) {
+    return (
+      <FilterByPublished visibleFilters={
+        [PublishStatus.Published, PublishStatus.Modified]
+      }
+      />
+    );
+  }
+
+  return <FilterByPublished />;
+};
+
+export default LibraryFilterByPublished;

src/search-manager/FilterByPublished.tsx

@@ -10,12 +10,18 @@ import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
 import messages from './messages';
 import SearchFilterWidget from './SearchFilterWidget';
 import { useSearchContext } from './SearchManager';
-import { PublishStatus } from './data/api';
+import { allPublishFilters, PublishStatus } from './data/api';
+
+interface FilterByPublishedProps {
+  visibleFilters?: PublishStatus[],
+}
 
 /**
  * A button with a dropdown that allows filtering the current search by publish status
  */
-const FilterByPublished: React.FC<Record<never, never>> = () => {
+const FilterByPublished = ({
+  visibleFilters = allPublishFilters,
+}: FilterByPublishedProps) => {
   const intl = useIntl();
   const {
     publishStatus,
@@ -42,6 +48,26 @@ const FilterByPublished: React.FC<Record<never, never>> = () => {
   };
   const appliedFilters = publishStatusFilter.map(mode => ({ label: modeToLabel[mode] }));
 
+  const filterLabels = {
+    [PublishStatus.Published]: intl.formatMessage(messages.publishStatusPublished),
+    [PublishStatus.Modified]: intl.formatMessage(messages.publishStatusModified),
+    [PublishStatus.NeverPublished]: intl.formatMessage(messages.publishStatusNeverPublished),
+  };
+
+  const visibleFiltersToRender = visibleFilters.map((filter) => (
+    <MenuItem
+      key={filter}
+      as={Form.Checkbox}
+      value={filter}
+      onChange={() => { toggleFilterMode(filter); }}
+    >
+      <div>
+        {filterLabels[filter]}
+        <Badge variant="light" pill>{publishStatus[filter] ?? 0}</Badge>
+      </div>
+    </MenuItem>
+  ));
+
   return (
     <SearchFilterWidget
       appliedFilters={appliedFilters}
@@ -55,36 +81,7 @@ const FilterByPublished: React.FC<Record<never, never>> = () => {
           value={publishStatusFilter}
         >
           <Menu className="block-type-refinement-menu" style={{ boxShadow: 'none' }}>
-            <MenuItem
-              as={Form.Checkbox}
-              value={PublishStatus.Published}
-              onChange={() => { toggleFilterMode(PublishStatus.Published); }}
-            >
-              <div>
-                {intl.formatMessage(messages.publishStatusPublished)}
-                <Badge variant="light" pill>{publishStatus[PublishStatus.Published] ?? 0}</Badge>
-              </div>
-            </MenuItem>
-            <MenuItem
-              as={Form.Checkbox}
-              value={PublishStatus.Modified}
-              onChange={() => { toggleFilterMode(PublishStatus.Modified); }}
-            >
-              <div>
-                {intl.formatMessage(messages.publishStatusModified)}
-                <Badge variant="light" pill>{publishStatus[PublishStatus.Modified] ?? 0}</Badge>
-              </div>
-            </MenuItem>
-            <MenuItem
-              as={Form.Checkbox}
-              value={PublishStatus.NeverPublished}
-              onChange={() => { toggleFilterMode(PublishStatus.NeverPublished); }}
-            >
-              <div>
-                {intl.formatMessage(messages.publishStatusNeverPublished)}
-                <Badge variant="light" pill>{publishStatus[PublishStatus.NeverPublished] ?? 0}</Badge>
-              </div>
-            </MenuItem>
+            {visibleFiltersToRender}
           </Menu>
         </Form.CheckboxSet>
       </Form.Group>

src/search-manager/data/api.ts

@@ -31,6 +31,8 @@ export enum PublishStatus {
   NeverPublished = 'never',
 }
 
+export const allPublishFilters: PublishStatus[] = Object.values(PublishStatus);
+
 /**
  * Get the content search configuration from the CMS.
  */