Skip to content

Commit b375806

Browse files
perf: use Library search results to populate container card preview [FC-0083] [TEAK] (#1889)
* fix: several library unit page UX bugs (#1868) * fix: rename "Organize" tab to "Manage" * fix: duplicate key warnings * fix: uniform messages while adding to collection * fix: do not allow units be added to a unit (cherry picked from commit 0fdc460) * perf: use Library search results to populate container card preview (#1820) * fix: use Library search results to populate container card preview * feat: show published children when showing only published Unit content * fix: nits (cherry picked from commit 24e4695) --------- Co-authored-by: Rômulo Penido <romulo.penido@gmail.com>
1 parent ab0e0d7 commit b375806

19 files changed

Lines changed: 144 additions & 88 deletions

src/library-authoring/LibraryAuthoringPage.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ describe('<LibraryAuthoringPage />', () => {
433433
const { getByRole, queryByText } = within(sidebar);
434434

435435
await waitFor(() => expect(queryByText(displayName)).toBeInTheDocument());
436-
expect(getByRole('tab', { selected: true })).toHaveTextContent('Organize');
436+
expect(getByRole('tab', { selected: true })).toHaveTextContent('Manage');
437437
const closeButton = getByRole('button', { name: /close/i });
438438
fireEvent.click(closeButton);
439439

src/library-authoring/add-content/AddContent.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ describe('<AddContent />', () => {
272272
await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(pasteUrl));
273273
await waitFor(() => expect(axiosMock.history.patch.length).toEqual(1));
274274
await waitFor(() => expect(axiosMock.history.patch[0].url).toEqual(collectionComponentUrl));
275-
expect(mockShowToast).toHaveBeenCalledWith('There was an error linking the content to this collection.');
275+
expect(mockShowToast).toHaveBeenCalledWith('Failed to add content to collection.');
276276
});
277277

278278
it('should stop user from pasting unsupported blocks and show toast', async () => {

src/library-authoring/add-content/AddContent.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import { useLibraryContext } from '../common/context/LibraryContext';
2929
import { PickLibraryContentModal } from './PickLibraryContentModal';
3030
import { blockTypes } from '../../editors/data/constants/app';
3131

32+
import { ContentType as LibraryContentTypes } from '../routes';
33+
import genericMessages from '../generic/messages';
3234
import messages from './messages';
3335
import type { BlockTypeMetadata } from '../data/api';
3436
import { getContainerTypeFromId, ContainerType } from '../../generic/key-utils';
@@ -114,6 +116,9 @@ const AddContentView = ({
114116
blockType: 'libraryContent',
115117
};
116118

119+
const extraFilter = unitId ? ['NOT block_type = "unit"', 'NOT type = "collections"'] : undefined;
120+
const visibleTabs = unitId ? [LibraryContentTypes.components] : undefined;
121+
117122
return (
118123
<>
119124
{(collectionId || unitId) && componentPicker && (
@@ -123,6 +128,8 @@ const AddContentView = ({
123128
<PickLibraryContentModal
124129
isOpen={isAddLibraryContentModalOpen}
125130
onClose={closeAddLibraryContentModal}
131+
extraFilter={extraFilter}
132+
visibleTabs={visibleTabs}
126133
/>
127134
</>
128135
)}
@@ -301,7 +308,7 @@ const AddContent = () => {
301308
const linkComponent = (opaqueKey: string) => {
302309
if (collectionId) {
303310
addComponentsToCollectionMutation.mutateAsync([opaqueKey]).catch(() => {
304-
showToast(intl.formatMessage(messages.errorAssociateComponentToCollectionMessage));
311+
showToast(intl.formatMessage(genericMessages.manageCollectionsFailed));
305312
});
306313
}
307314
if (unitId) {

src/library-authoring/add-content/PickLibraryContentModal.test.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ describe('<PickLibraryContentModal />', () => {
9292
}
9393
});
9494
expect(onClose).toHaveBeenCalled();
95-
expect(mockShowToast).toHaveBeenCalledWith('Content linked successfully.');
95+
const text = context === 'collection'
96+
? 'Content added to collection.'
97+
: 'Content linked successfully.';
98+
expect(mockShowToast).toHaveBeenCalledWith(text);
9699
});
97100

98101
it(`show error when api call fails (${context})`, async () => {
@@ -130,8 +133,10 @@ describe('<PickLibraryContentModal />', () => {
130133
}
131134
});
132135
expect(onClose).toHaveBeenCalled();
133-
const name = context === 'collection' ? 'collection' : 'container';
134-
expect(mockShowToast).toHaveBeenCalledWith(`There was an error linking the content to this ${name}.`);
136+
const text = context === 'collection'
137+
? 'Failed to add content to collection.'
138+
: 'There was an error linking the content to this container.';
139+
expect(mockShowToast).toHaveBeenCalledWith(text);
135140
});
136141
});
137142
});

src/library-authoring/add-content/PickLibraryContentModal.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { ToastContext } from '../../generic/toast-context';
66
import { useLibraryContext } from '../common/context/LibraryContext';
77
import type { SelectedComponent } from '../common/context/ComponentPickerContext';
88
import { useAddItemsToCollection, useAddComponentsToContainer } from '../data/apiHooks';
9+
import genericMessages from '../generic/messages';
10+
import type { ContentType } from '../routes';
911
import messages from './messages';
1012

1113
interface PickLibraryContentModalFooterProps {
@@ -32,12 +34,14 @@ interface PickLibraryContentModalProps {
3234
isOpen: boolean;
3335
onClose: () => void;
3436
extraFilter?: string[];
37+
visibleTabs?: ContentType[],
3538
}
3639

3740
export const PickLibraryContentModal: React.FC<PickLibraryContentModalProps> = ({
3841
isOpen,
3942
onClose,
4043
extraFilter,
44+
visibleTabs,
4145
}) => {
4246
const intl = useIntl();
4347

@@ -69,16 +73,16 @@ export const PickLibraryContentModal: React.FC<PickLibraryContentModalProps> = (
6973
if (collectionId) {
7074
updateCollectionItemsMutation.mutateAsync(usageKeys)
7175
.then(() => {
72-
showToast(intl.formatMessage(messages.successAssociateComponentMessage));
76+
showToast(intl.formatMessage(genericMessages.manageCollectionsSuccess));
7377
})
7478
.catch(() => {
75-
showToast(intl.formatMessage(messages.errorAssociateComponentToCollectionMessage));
79+
showToast(intl.formatMessage(genericMessages.manageCollectionsFailed));
7680
});
7781
}
7882
if (unitId) {
7983
updateUnitComponentsMutation.mutateAsync(usageKeys)
8084
.then(() => {
81-
showToast(intl.formatMessage(messages.successAssociateComponentMessage));
85+
showToast(intl.formatMessage(messages.successAssociateComponentToContainerMessage));
8286
})
8387
.catch(() => {
8488
showToast(intl.formatMessage(messages.errorAssociateComponentToContainerMessage));
@@ -109,6 +113,7 @@ export const PickLibraryContentModal: React.FC<PickLibraryContentModalProps> = (
109113
componentPickerMode="multiple"
110114
onChangeComponentSelection={setSelectedComponents}
111115
extraFilter={extraFilter}
116+
visibleTabs={visibleTabs}
112117
/>
113118
</StandardModal>
114119
);

src/library-authoring/add-content/messages.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,10 @@ const messages = defineMessages({
8484
+ ' The {detail} text provides more information about the error.'
8585
),
8686
},
87-
successAssociateComponentMessage: {
88-
id: 'course-authoring.library-authoring.associate-collection-content.success.text',
87+
successAssociateComponentToContainerMessage: {
88+
id: 'course-authoring.library-authoring.associate-container-content.success.text',
8989
defaultMessage: 'Content linked successfully.',
90-
description: 'Message when linking of content to a collection in library is success',
91-
},
92-
errorAssociateComponentToCollectionMessage: {
93-
id: 'course-authoring.library-authoring.associate-collection-content.error.text',
94-
defaultMessage: 'There was an error linking the content to this collection.',
95-
description: 'Message when linking of content to a collection in library fails',
90+
description: 'Message when linking of content to a container in library is success',
9691
},
9792
errorAssociateComponentToContainerMessage: {
9893
id: 'course-authoring.library-authoring.associate-container-content.error.text',

src/library-authoring/common/context/SidebarContext.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export const isComponentInfoTab = (tab: string): tab is ComponentInfoTab => (
3636

3737
export const UNIT_INFO_TABS = {
3838
Preview: 'preview',
39-
Organize: 'organize',
39+
Manage: 'manage',
4040
Usage: 'usage',
4141
Settings: 'settings',
4242
} as const;

src/library-authoring/components/ContainerCard.test.tsx

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
fireEvent,
77
} from '../../testUtils';
88
import { LibraryProvider } from '../common/context/LibraryContext';
9-
import { mockContentLibrary, mockGetContainerChildren } from '../data/api.mocks';
9+
import { mockContentLibrary } from '../data/api.mocks';
1010
import { type ContainerHit, PublishStatus } from '../../search-manager';
1111
import ContainerCard from './ContainerCard';
1212
import { getLibraryContainerApiUrl, getLibraryContainerRestoreApiUrl } from '../data/api';
@@ -40,7 +40,6 @@ let axiosMock: MockAdapter;
4040
let mockShowToast;
4141

4242
mockContentLibrary.applyMock();
43-
mockGetContainerChildren.applyMock();
4443

4544
const render = (ui: React.ReactElement, showOnlyPublished: boolean = false) => baseRender(ui, {
4645
extraWrapper: ({ children }) => (
@@ -155,29 +154,54 @@ describe('<ContainerCard />', () => {
155154
it('should render no child blocks in card preview', async () => {
156155
render(<ContainerCard hit={containerHitSample} />);
157156

158-
expect(screen.queryByTitle('text block')).not.toBeInTheDocument();
157+
expect(screen.queryByTitle('lb:org1:Demo_course:html:text-0')).not.toBeInTheDocument();
159158
expect(screen.queryByText('+0')).not.toBeInTheDocument();
160159
});
161160

162161
it('should render <=5 child blocks in card preview', async () => {
163162
const containerWith5Children = {
164163
...containerHitSample,
165-
usageKey: mockGetContainerChildren.fiveChildren,
166-
};
164+
content: {
165+
childUsageKeys: Array(5).fill('').map((_child, idx) => `lb:org1:Demo_course:html:text-${idx}`),
166+
},
167+
} satisfies ContainerHit;
167168
render(<ContainerCard hit={containerWith5Children} />);
168169

169-
expect((await screen.findAllByTitle(/text block */)).length).toBe(5);
170+
expect((await screen.findAllByTitle(/lb:org1:Demo_course:html:text-*/)).length).toBe(5);
170171
expect(screen.queryByText('+0')).not.toBeInTheDocument();
171172
});
172173

173174
it('should render >5 child blocks with +N in card preview', async () => {
174175
const containerWith6Children = {
175176
...containerHitSample,
176-
usageKey: mockGetContainerChildren.sixChildren,
177-
};
177+
content: {
178+
childUsageKeys: Array(6).fill('').map((_child, idx) => `lb:org1:Demo_course:html:text-${idx}`),
179+
},
180+
} satisfies ContainerHit;
178181
render(<ContainerCard hit={containerWith6Children} />);
179182

180-
expect((await screen.findAllByTitle(/text block */)).length).toBe(4);
183+
expect((await screen.findAllByTitle(/lb:org1:Demo_course:html:text-*/)).length).toBe(4);
181184
expect(screen.queryByText('+2')).toBeInTheDocument();
182185
});
186+
187+
it('should render published child blocks when rendering a published card preview', async () => {
188+
const containerWithPublishedChildren = {
189+
...containerHitSample,
190+
content: {
191+
childUsageKeys: Array(6).fill('').map((_child, idx) => `lb:org1:Demo_course:html:text-${idx}`),
192+
},
193+
published: {
194+
content: {
195+
childUsageKeys: Array(2).fill('').map((_child, idx) => `lb:org1:Demo_course:html:text-${idx}`),
196+
},
197+
},
198+
} satisfies ContainerHit;
199+
render(
200+
<ContainerCard hit={containerWithPublishedChildren} />,
201+
true,
202+
);
203+
204+
expect((await screen.findAllByTitle(/lb:org1:Demo_course:html:text-*/)).length).toBe(2);
205+
expect(screen.queryByText('+2')).not.toBeInTheDocument();
206+
});
183207
});

src/library-authoring/components/ContainerCard.tsx

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@ import { MoreVert } from '@openedx/paragon/icons';
1212
import { Link } from 'react-router-dom';
1313

1414
import { getItemIcon, getComponentStyleColor } from '../../generic/block-type-utils';
15+
import { getBlockType } from '../../generic/key-utils';
1516
import { ToastContext } from '../../generic/toast-context';
1617
import { type ContainerHit, PublishStatus } from '../../search-manager';
1718
import { useComponentPickerContext } from '../common/context/ComponentPickerContext';
1819
import { useLibraryContext } from '../common/context/LibraryContext';
1920
import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext';
20-
import { useContainerChildren, useRemoveItemsFromCollection } from '../data/apiHooks';
21+
import { useRemoveItemsFromCollection } from '../data/apiHooks';
2122
import { useLibraryRoutes } from '../routes';
2223
import AddComponentWidget from './AddComponentWidget';
2324
import BaseCard from './BaseCard';
@@ -107,21 +108,17 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => {
107108
};
108109

109110
type ContainerCardPreviewProps = {
110-
containerId: string;
111+
childUsageKeys: Array<string>;
111112
showMaxChildren?: number;
112113
};
113114

114-
const ContainerCardPreview = ({ containerId, showMaxChildren = 5 }: ContainerCardPreviewProps) => {
115-
const { data, isLoading, isError } = useContainerChildren(containerId);
116-
if (isLoading || isError) {
117-
return null;
118-
}
119-
120-
const hiddenChildren = data.length - showMaxChildren;
115+
const ContainerCardPreview = ({ childUsageKeys, showMaxChildren = 5 }: ContainerCardPreviewProps) => {
116+
const hiddenChildren = childUsageKeys.length - showMaxChildren;
121117
return (
122118
<Stack direction="horizontal" gap={2}>
123119
{
124-
data.slice(0, showMaxChildren).map(({ id, blockType, displayName }, idx) => {
120+
childUsageKeys.slice(0, showMaxChildren).map((usageKey, idx) => {
121+
const blockType = getBlockType(usageKey);
125122
let blockPreview: ReactNode;
126123
let classNames;
127124

@@ -133,7 +130,7 @@ const ContainerCardPreview = ({ containerId, showMaxChildren = 5 }: ContainerCar
133130
<Icon
134131
src={getItemIcon(blockType)}
135132
screenReaderText={blockType}
136-
title={displayName}
133+
title={usageKey}
137134
/>
138135
);
139136
} else {
@@ -147,7 +144,9 @@ const ContainerCardPreview = ({ containerId, showMaxChildren = 5 }: ContainerCar
147144
}
148145
return (
149146
<div
150-
key={`container-card-preview-block-${id}`}
147+
// A container can have multiple instances of the same block
148+
// eslint-disable-next-line react/no-array-index-key
149+
key={`${usageKey}-${idx}`}
151150
className={classNames}
152151
>
153152
{blockPreview}
@@ -176,6 +175,7 @@ const ContainerCard = ({ hit } : ContainerCardProps) => {
176175
published,
177176
publishStatus,
178177
usageKey: unitId,
178+
content,
179179
} = hit;
180180

181181
const numChildrenCount = showOnlyPublished ? (
@@ -186,6 +186,10 @@ const ContainerCard = ({ hit } : ContainerCardProps) => {
186186
showOnlyPublished ? formatted.published?.displayName : formatted.displayName
187187
) ?? '';
188188

189+
const childUsageKeys: Array<string> = (
190+
showOnlyPublished ? published?.content?.childUsageKeys : content?.childUsageKeys
191+
) ?? [];
192+
189193
const { navigateTo } = useLibraryRoutes();
190194

191195
const openContainer = useCallback(() => {
@@ -200,7 +204,7 @@ const ContainerCard = ({ hit } : ContainerCardProps) => {
200204
<BaseCard
201205
itemType={itemType}
202206
displayName={displayName}
203-
preview={<ContainerCardPreview containerId={unitId} />}
207+
preview={<ContainerCardPreview childUsageKeys={childUsageKeys} />}
204208
tags={tags}
205209
numChildren={numChildrenCount}
206210
actions={(

src/library-authoring/containers/ContainerOrganize.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ const ContainerOrganize = () => {
8585
>
8686
<Stack gap={1} direction="horizontal">
8787
<Icon src={Tag} />
88-
{intl.formatMessage(messages.organizeTabTagsTitle, { count: tagsCount })}
88+
{intl.formatMessage(messages.manageTabTagsTitle, { count: tagsCount })}
8989
</Stack>
9090
<Collapsible.Visible whenClosed>
9191
<Icon src={ExpandMore} />
@@ -113,7 +113,7 @@ const ContainerOrganize = () => {
113113
>
114114
<Stack gap={1} direction="horizontal">
115115
<Icon src={BookOpen} />
116-
{intl.formatMessage(messages.organizeTabCollectionsTitle, { count: collectionsCount })}
116+
{intl.formatMessage(messages.manageTabCollectionsTitle, { count: collectionsCount })}
117117
</Stack>
118118
<Collapsible.Visible whenClosed>
119119
<Icon src={ExpandMore} />

0 commit comments

Comments
 (0)