Skip to content

Commit 218b65e

Browse files
author
Ahtesham Quraish
committed
fix: Double click component card to edit #1598
1 parent ab645ad commit 218b65e

File tree

4 files changed

+194
-78
lines changed

4 files changed

+194
-78
lines changed

src/library-authoring/LibraryAuthoringPage.test.tsx

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -510,31 +510,6 @@ describe('<LibraryAuthoringPage />', () => {
510510
await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument());
511511
});
512512

513-
it('should preserve the tab while switching from a component to a collection', async () => {
514-
await renderLibraryPage();
515-
516-
// Click on the first collection
517-
fireEvent.click((await screen.findByText('Collection 1')));
518-
519-
// Click on the Details tab
520-
fireEvent.click(screen.getByRole('tab', { name: 'Details' }));
521-
522-
// Change to a component
523-
fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]);
524-
525-
// Check that the Details tab is still selected
526-
expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true');
527-
528-
// Click on the Previews tab
529-
fireEvent.click(screen.getByRole('tab', { name: 'Preview' }));
530-
531-
// Switch back to the collection
532-
fireEvent.click((await screen.findByText('Collection 1')));
533-
534-
// The Details (default) tab should be selected because the collection does not have a Preview tab
535-
expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true');
536-
});
537-
538513
const problemTypes = {
539514
'Multiple Choice': 'choiceresponse',
540515
Checkboxes: 'multiplechoiceresponse',
@@ -1063,3 +1038,84 @@ describe('<LibraryAuthoringPage />', () => {
10631038
);
10641039
});
10651040
});
1041+
1042+
describe('<LibraryAuthoringPage />', () => {
1043+
beforeAll(() => {
1044+
jest.useFakeTimers();
1045+
});
1046+
1047+
beforeEach(async () => {
1048+
const mocks = initializeMocks();
1049+
axiosMock = mocks.axiosMock;
1050+
mockShowToast = mocks.mockShowToast;
1051+
axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock);
1052+
1053+
// The Meilisearch client-side API uses fetch, not Axios.
1054+
fetchMock.mockReset();
1055+
fetchMock.post(searchEndpoint, (_url, req) => {
1056+
const requestData = JSON.parse(req.body?.toString() ?? '');
1057+
const query = requestData?.queries[0]?.q ?? '';
1058+
// We have to replace the query (search keywords) in the mock results with the actual query,
1059+
// because otherwise Instantsearch will update the UI and change the query,
1060+
// leading to unexpected results in the test cases.
1061+
const newMockResult = { ...mockResult };
1062+
newMockResult.results[0].query = query;
1063+
// And fake the required '_formatted' fields; it contains the highlighting <mark>...</mark> around matched words
1064+
// eslint-disable-next-line no-underscore-dangle, no-param-reassign
1065+
newMockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; });
1066+
return newMockResult;
1067+
});
1068+
});
1069+
1070+
afterAll(() => {
1071+
jest.useRealTimers();
1072+
});
1073+
1074+
it('should preserve the tab while switching from a component to a collection', async () => {
1075+
jest.useFakeTimers(); // ✅ enable fake timers for this test only
1076+
1077+
render(<LibraryLayout />, { path, params: { libraryId: mockContentLibrary.libraryId } });
1078+
1079+
// Ensure the search endpoint is called:
1080+
// Call 1: To fetch searchable/filterable/sortable library data
1081+
await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); });
1082+
1083+
// Click on the first collection
1084+
fireEvent.click(await screen.findByText('Collection 1'));
1085+
1086+
// ⏩ let sidebar open
1087+
jest.advanceTimersByTime(500);
1088+
1089+
// Click on the Details tab
1090+
fireEvent.click(screen.getByRole('tab', { name: 'Details' }));
1091+
1092+
// Change to a component
1093+
fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]);
1094+
1095+
// ⏩ let sidebar switch to component
1096+
jest.advanceTimersByTime(500);
1097+
1098+
// Check that the Details tab is still selected
1099+
expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute(
1100+
'aria-selected',
1101+
'true',
1102+
);
1103+
1104+
// Click on the Preview tab
1105+
fireEvent.click(screen.getByRole('tab', { name: 'Preview' }));
1106+
1107+
// Switch back to the collection
1108+
fireEvent.click(await screen.findByText('Collection 1'));
1109+
1110+
// ⏩ let sidebar switch back to collection
1111+
jest.advanceTimersByTime(500);
1112+
1113+
// The Details (default) tab should be selected because the collection does not have a Preview tab
1114+
expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute(
1115+
'aria-selected',
1116+
'true',
1117+
);
1118+
1119+
jest.useRealTimers(); // ✅ restore for other tests
1120+
});
1121+
});

src/library-authoring/collections/LibraryCollectionPage.test.tsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,29 +355,47 @@ describe('<LibraryCollectionPage />', () => {
355355
});
356356

357357
it('should remove component from collection and hides sidebar', async () => {
358+
jest.useFakeTimers(); // ✅ isolate fake timers for this test
359+
358360
const url = getLibraryCollectionItemsApiUrl(
359361
mockContentLibrary.libraryId,
360362
mockCollection.collectionId,
361363
);
362364
axiosMock.onDelete(url).reply(204);
365+
363366
const displayName = 'Introduction to Testing';
364367
await renderLibraryCollectionPage();
365368

366369
// open sidebar
367370
fireEvent.click(await screen.findByText(displayName));
371+
372+
// ⏩ let the delayed sidebar open run
373+
jest.advanceTimersByTime(500);
374+
368375
await waitFor(() => expect(screen.queryByTestId('library-sidebar')).toBeInTheDocument());
369376

370-
const menuBtns = await screen.findAllByRole('button', { name: 'Component actions menu' });
377+
const menuBtns = await screen.findAllByRole('button', {
378+
name: 'Component actions menu',
379+
});
380+
371381
// open menu
372382
fireEvent.click(menuBtns[0]);
373383

384+
// click remove
374385
fireEvent.click(await screen.findByText('Remove from collection'));
386+
375387
await waitFor(() => {
376388
expect(axiosMock.history.delete.length).toEqual(1);
377389
});
390+
378391
expect(mockShowToast).toHaveBeenCalledWith('Item successfully removed');
379-
// Should close sidebar as component was removed
392+
393+
// ⏩ let the delayed sidebar close run
394+
jest.advanceTimersByTime(500);
395+
380396
await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument());
397+
398+
jest.useRealTimers(); // ✅ restore for other tests
381399
});
382400

383401
it('should show error when remove component from collection', async () => {

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

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -221,46 +221,6 @@ describe('<ComponentPicker />', () => {
221221
}, '*');
222222
});
223223

224-
it('should pick component inside a collection using the sidebar', async () => {
225-
render(<ComponentPicker />);
226-
227-
expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
228-
fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i));
229-
230-
// Wait for the content library to load
231-
await screen.findByText(/Change Library/i);
232-
expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
233-
234-
// Click on the collection card to open the sidebar
235-
fireEvent.click(screen.queryAllByText('Collection 1')[0]);
236-
237-
const sidebar = await screen.findByTestId('library-sidebar');
238-
239-
// Mock the collection search result
240-
mockSearchResult(mockCollectionResult);
241-
242-
// Click the add component from the component card
243-
fireEvent.click(within(sidebar).getByRole('button', { name: 'Open' }));
244-
245-
// Wait for the collection to load
246-
await screen.findByText(/Back to Library/i);
247-
await screen.findByText('Introduction to Testing');
248-
249-
// Click on the collection card to open the sidebar
250-
fireEvent.click(screen.getByText('Introduction to Testing'));
251-
252-
const collectionSidebar = await screen.findByTestId('library-sidebar');
253-
254-
// Click the add component from the collection sidebar
255-
fireEvent.click(within(collectionSidebar).getByRole('button', { name: 'Add to Course' }));
256-
257-
expect(postMessageSpy).toHaveBeenCalledWith({
258-
usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd',
259-
type: 'pickerComponentSelected',
260-
category: 'html',
261-
}, '*');
262-
});
263-
264224
it('should return to library selection', async () => {
265225
render(<ComponentPicker />);
266226

@@ -428,3 +388,69 @@ describe('<ComponentPicker />', () => {
428388
expect(screen.queryByText(/never published/i)).not.toBeInTheDocument();
429389
});
430390
});
391+
392+
describe('<ComponentPicker /> with collection', () => {
393+
beforeEach(() => {
394+
initializeMocks();
395+
postMessageSpy = jest.spyOn(window.parent, 'postMessage');
396+
397+
mockSearchResult({ ...mockResult });
398+
});
399+
400+
it('should pick component inside a collection using the sidebar', async () => {
401+
jest.useFakeTimers(); // ✅ enable fake timers
402+
403+
render(<ComponentPicker />);
404+
405+
expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
406+
fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i));
407+
408+
// Wait for the content library to load
409+
await screen.findByText(/Change Library/i);
410+
expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
411+
412+
// Click on the collection card to open the sidebar
413+
const collections = await screen.findAllByText('Collection 1'); // ✅ wait until it renders
414+
fireEvent.click(collections[0]);
415+
416+
// ⏩ let the 500ms single-click timer finish
417+
jest.advanceTimersByTime(500);
418+
419+
const sidebar = await screen.findByTestId('library-sidebar');
420+
421+
// Mock the collection search result
422+
mockSearchResult(mockCollectionResult);
423+
424+
// Click the add component from the component card
425+
fireEvent.click(within(sidebar).getByRole('button', { name: 'Open' }));
426+
427+
// ⏩ advance timers again in case sidebar open uses timeout
428+
jest.advanceTimersByTime(500);
429+
430+
// Wait for the collection to load
431+
await screen.findByText(/Back to Library/i);
432+
await screen.findByText('Introduction to Testing');
433+
434+
// Click on the collection card to open the sidebar
435+
fireEvent.click(screen.getByText('Introduction to Testing'));
436+
437+
// ⏩ advance timers again for delayed sidebar open
438+
jest.advanceTimersByTime(500);
439+
440+
const collectionSidebar = await screen.findByTestId('library-sidebar');
441+
442+
// Click the add component from the collection sidebar
443+
fireEvent.click(within(collectionSidebar).getByRole('button', { name: 'Add to Course' }));
444+
445+
expect(postMessageSpy).toHaveBeenCalledWith(
446+
{
447+
usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd',
448+
type: 'pickerComponentSelected',
449+
category: 'html',
450+
},
451+
'*',
452+
);
453+
454+
jest.useRealTimers(); // ✅ restore real timers
455+
});
456+
});

src/library-authoring/components/ComponentCard.tsx

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback } from 'react';
1+
import { useCallback, useRef } from 'react';
22
import {
33
ActionRow,
44
} from '@openedx/paragon';
@@ -15,11 +15,15 @@ type ComponentCardProps = {
1515
hit: ContentHit,
1616
};
1717

18+
const DOUBLE_CLICK_DELAY = 500; // ms
19+
1820
const ComponentCard = ({ hit }: ComponentCardProps) => {
19-
const { showOnlyPublished } = useLibraryContext();
21+
const { showOnlyPublished, openComponentEditor } = useLibraryContext();
2022
const { openComponentInfoSidebar, openItemSidebar, sidebarItemInfo } = useSidebarContext();
2123
const { componentPickerMode } = useComponentPickerContext();
2224

25+
const clickTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
26+
2327
const {
2428
blockType,
2529
formatted,
@@ -34,15 +38,27 @@ const ComponentCard = ({ hit }: ComponentCardProps) => {
3438
showOnlyPublished ? formatted.published?.displayName : formatted.displayName
3539
) ?? '';
3640

37-
const selectComponent = useCallback(() => {
38-
if (!componentPickerMode) {
39-
openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo);
40-
} else {
41-
// In component picker mode, we want to open the sidebar
42-
// without changing the URL
43-
openComponentInfoSidebar(usageKey);
44-
}
45-
}, [usageKey, openItemSidebar, openComponentInfoSidebar, componentPickerMode]);
41+
const selectComponent = useCallback(
42+
() => {
43+
if (clickTimerRef.current) {
44+
clearTimeout(clickTimerRef.current);
45+
clickTimerRef.current = null;
46+
openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo);
47+
openComponentEditor(usageKey);
48+
} else {
49+
clickTimerRef.current = setTimeout(() => {
50+
clickTimerRef.current = null;
51+
52+
if (componentPickerMode) {
53+
openComponentInfoSidebar(usageKey);
54+
} else {
55+
openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo);
56+
}
57+
}, DOUBLE_CLICK_DELAY);
58+
}
59+
},
60+
[usageKey, openItemSidebar, openComponentInfoSidebar, componentPickerMode],
61+
);
4662

4763
const selected = sidebarItemInfo?.type === SidebarBodyItemId.ComponentInfo
4864
&& sidebarItemInfo.id === usageKey;

0 commit comments

Comments
 (0)