Skip to content

Commit cdf39f0

Browse files
pomegranitedrpenido
authored andcommitted
perf: use Library search results to populate container card preview [FC-0083] [TEAK] (openedx#1889)
* fix: several library unit page UX bugs (openedx#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 (openedx#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 e6febe6 commit cdf39f0

4 files changed

Lines changed: 169 additions & 249 deletions

File tree

src/library-authoring/LibraryAuthoringPage.test.tsx

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ const path = '/library/:libraryId/*';
5555
const libraryTitle = mockContentLibrary.libraryData.title;
5656

5757
describe('<LibraryAuthoringPage />', () => {
58-
beforeAll(() => {
59-
jest.useFakeTimers();
60-
});
61-
6258
beforeEach(async () => {
6359
const mocks = initializeMocks();
6460
axiosMock = mocks.axiosMock;
@@ -82,10 +78,6 @@ describe('<LibraryAuthoringPage />', () => {
8278
});
8379
});
8480

85-
afterAll(() => {
86-
jest.useRealTimers();
87-
});
88-
8981
const renderLibraryPage = async () => {
9082
render(<LibraryLayout />, { path, params: { libraryId: mockContentLibrary.libraryId } });
9183

@@ -370,7 +362,7 @@ describe('<LibraryAuthoringPage />', () => {
370362
fireEvent.change(searchBox, { target: { value: 'words to find' } });
371363

372364
// Default sort option changes to "Most Relevant"
373-
expect((await screen.findAllByText('Most Relevant')).length).toEqual(2);
365+
expect(screen.getAllByText('Most Relevant').length).toEqual(2);
374366
await waitFor(() => {
375367
expect(fetchMock).toHaveBeenLastCalledWith(searchEndpoint, {
376368
body: expect.stringContaining('"sort":[]'),
@@ -400,7 +392,7 @@ describe('<LibraryAuthoringPage />', () => {
400392
await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument());
401393
});
402394

403-
it('should open component sidebar, showing manage tab on clicking add to collection menu item - component', async () => {
395+
it('should open component sidebar, showing manage tab on clicking add to collection menu item (component)', async () => {
404396
const mockResult0 = { ...mockResult }.results[0].hits[0];
405397
const displayName = 'Introduction to Testing';
406398
expect(mockResult0.display_name).toStrictEqual(displayName);
@@ -415,18 +407,17 @@ describe('<LibraryAuthoringPage />', () => {
415407

416408
const sidebar = screen.getByTestId('library-sidebar');
417409

418-
const { getByRole, findByText } = within(sidebar);
410+
const { getByRole, queryByText } = within(sidebar);
419411

420-
expect(await findByText(displayName)).toBeInTheDocument();
421-
jest.advanceTimersByTime(300);
412+
await waitFor(() => expect(queryByText(displayName)).toBeInTheDocument());
422413
expect(getByRole('tab', { selected: true })).toHaveTextContent('Manage');
423414
const closeButton = getByRole('button', { name: /close/i });
424415
fireEvent.click(closeButton);
425416

426417
await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument());
427418
});
428419

429-
it('should open component sidebar, showing manage tab on clicking add to collection menu item - unit', async () => {
420+
it('should open component sidebar, showing manage tab on clicking add to collection menu item (unit)', async () => {
430421
const displayName = 'Test Unit';
431422
await renderLibraryPage();
432423

@@ -439,10 +430,9 @@ describe('<LibraryAuthoringPage />', () => {
439430

440431
const sidebar = screen.getByTestId('library-sidebar');
441432

442-
const { getByRole, findByText } = within(sidebar);
433+
const { getByRole, queryByText } = within(sidebar);
443434

444-
expect(await findByText(displayName)).toBeInTheDocument();
445-
jest.advanceTimersByTime(300);
435+
await waitFor(() => expect(queryByText(displayName)).toBeInTheDocument());
446436
expect(getByRole('tab', { selected: true })).toHaveTextContent('Manage');
447437
const closeButton = getByRole('button', { name: /close/i });
448438
fireEvent.click(closeButton);

src/library-authoring/components/ContainerCard.tsx

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,13 @@ import { ToastContext } from '../../generic/toast-context';
1717
import { type ContainerHit, PublishStatus } from '../../search-manager';
1818
import { useComponentPickerContext } from '../common/context/ComponentPickerContext';
1919
import { useLibraryContext } from '../common/context/LibraryContext';
20-
import { SidebarActions, SidebarBodyComponentId, useSidebarContext } from '../common/context/SidebarContext';
20+
import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext';
2121
import { useRemoveItemsFromCollection } from '../data/apiHooks';
2222
import { useLibraryRoutes } from '../routes';
2323
import AddComponentWidget from './AddComponentWidget';
2424
import BaseCard from './BaseCard';
2525
import messages from './messages';
2626
import ContainerDeleter from './ContainerDeleter';
27-
import { useRunOnNextRender } from '../../utils';
2827

2928
type ContainerMenuProps = {
3029
hit: ContainerHit,
@@ -46,7 +45,6 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => {
4645
} = useSidebarContext();
4746
const { showToast } = useContext(ToastContext);
4847
const [isConfirmingDelete, confirmDelete, cancelDelete] = useToggle(false);
49-
const { navigateTo } = useLibraryRoutes();
5048

5149
const removeComponentsMutation = useRemoveItemsFromCollection(libraryId, collectionId);
5250

@@ -62,17 +60,10 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => {
6260
});
6361
};
6462

65-
const scheduleJumpToCollection = useRunOnNextRender(() => {
66-
// TODO: Ugly hack to make sure sidebar shows add to collection section
67-
// This needs to run after all changes to url takes place to avoid conflicts.
68-
setTimeout(() => setSidebarAction(SidebarActions.JumpToManageCollections));
69-
});
70-
7163
const showManageCollections = useCallback(() => {
72-
navigateTo({ unitId: containerId });
64+
setSidebarAction(SidebarActions.JumpToAddCollections);
7365
openUnitInfoSidebar(containerId);
74-
scheduleJumpToCollection();
75-
}, [scheduleJumpToCollection, navigateTo, openUnitInfoSidebar, containerId]);
66+
}, [setSidebarAction, openUnitInfoSidebar, containerId]);
7667

7768
return (
7869
<>
@@ -174,7 +165,7 @@ type ContainerCardProps = {
174165
const ContainerCard = ({ hit } : ContainerCardProps) => {
175166
const { componentPickerMode } = useComponentPickerContext();
176167
const { setUnitId, showOnlyPublished } = useLibraryContext();
177-
const { openUnitInfoSidebar, sidebarComponentInfo } = useSidebarContext();
168+
const { openUnitInfoSidebar } = useSidebarContext();
178169

179170
const {
180171
blockType: itemType,
@@ -199,18 +190,13 @@ const ContainerCard = ({ hit } : ContainerCardProps) => {
199190
showOnlyPublished ? published?.content?.childUsageKeys : content?.childUsageKeys
200191
) ?? [];
201192

202-
const selected = sidebarComponentInfo?.type === SidebarBodyComponentId.UnitInfo
203-
&& sidebarComponentInfo.id === unitId;
204-
205193
const { navigateTo } = useLibraryRoutes();
206194

207-
const openContainer = useCallback((e?: React.MouseEvent) => {
195+
const openContainer = useCallback(() => {
208196
if (itemType === 'unit') {
209197
openUnitInfoSidebar(unitId);
210198
setUnitId(unitId);
211-
if (!componentPickerMode) {
212-
navigateTo({ unitId, doubleClicked: (e?.detail || 0) > 1 });
213-
}
199+
navigateTo({ unitId });
214200
}
215201
}, [unitId, itemType, openUnitInfoSidebar, navigateTo]);
216202

@@ -232,7 +218,6 @@ const ContainerCard = ({ hit } : ContainerCardProps) => {
232218
)}
233219
hasUnpublishedChanges={publishStatus !== PublishStatus.Published}
234220
onSelect={openContainer}
235-
selected={selected}
236221
/>
237222
);
238223
};

src/library-authoring/containers/UnitInfo.tsx

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@ import {
99
IconButton,
1010
useToggle,
1111
} from '@openedx/paragon';
12-
import React, { useCallback } from 'react';
12+
import React, { useEffect, useCallback } from 'react';
1313
import { Link } from 'react-router-dom';
1414
import { MoreVert } from '@openedx/paragon/icons';
1515

1616
import { useComponentPickerContext } from '../common/context/ComponentPickerContext';
1717
import { useLibraryContext } from '../common/context/LibraryContext';
1818
import {
1919
type UnitInfoTab,
20+
SidebarActions,
2021
UNIT_INFO_TABS,
2122
isUnitInfoTab,
2223
useSidebarContext,
@@ -80,8 +81,9 @@ const UnitInfo = () => {
8081
sidebarTab,
8182
setSidebarTab,
8283
sidebarComponentInfo,
83-
resetSidebarAction,
84+
sidebarAction,
8485
} = useSidebarContext();
86+
const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections;
8587
const { insideUnit } = useLibraryRoutes();
8688

8789
const tab: UnitInfoTab = (
@@ -94,12 +96,6 @@ const UnitInfo = () => {
9496

9597
const showOpenUnitButton = !insideUnit && !componentPickerMode;
9698

97-
/* istanbul ignore next */
98-
const handleTabChange = (newTab: UnitInfoTab) => {
99-
resetSidebarAction();
100-
setSidebarTab(newTab);
101-
};
102-
10399
const renderTab = useCallback((infoTab: UnitInfoTab, component: React.ReactNode, title: string) => {
104100
if (hiddenTabs.includes(infoTab)) {
105101
// For some reason, returning anything other than empty list breaks the tab style
@@ -121,6 +117,13 @@ const UnitInfo = () => {
121117
}
122118
}, [publishContainer]);
123119

120+
useEffect(() => {
121+
// Show Organize tab if JumpToAddCollections action is set in sidebarComponentInfo
122+
if (jumpToCollections) {
123+
setSidebarTab(UNIT_INFO_TABS.Manage);
124+
}
125+
}, [jumpToCollections, setSidebarTab]);
126+
124127
if (!container || !unitId) {
125128
return null;
126129
}
@@ -160,13 +163,9 @@ const UnitInfo = () => {
160163
className="my-3 d-flex justify-content-around"
161164
defaultActiveKey={defaultTab.unit}
162165
activeKey={tab}
163-
onSelect={handleTabChange}
166+
onSelect={setSidebarTab}
164167
>
165-
{renderTab(
166-
UNIT_INFO_TABS.Preview,
167-
<LibraryUnitBlocks readOnly />,
168-
intl.formatMessage(messages.previewTabTitle),
169-
)}
168+
{renderTab(UNIT_INFO_TABS.Preview, <LibraryUnitBlocks preview />, intl.formatMessage(messages.previewTabTitle))}
170169
{renderTab(UNIT_INFO_TABS.Manage, <ContainerOrganize />, intl.formatMessage(messages.manageTabTitle))}
171170
{renderTab(UNIT_INFO_TABS.Settings, 'Unit Settings', intl.formatMessage(messages.settingsTabTitle))}
172171
</Tabs>

0 commit comments

Comments
 (0)