From ac6ef11311ad7e04133956a43725c9d0428eab5f Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Mon, 15 Sep 2025 13:20:38 +0500 Subject: [PATCH] fix: Double click component card to edit #1598 --- .../LibraryAuthoringPage.test.tsx | 106 +++++++++++++----- .../LibraryCollectionPage.test.tsx | 22 +++- .../component-picker/ComponentPicker.test.tsx | 106 +++++++++++------- .../components/ComponentCard.test.tsx | 23 ++++ .../components/ComponentCard.tsx | 38 +++++-- 5 files changed, 217 insertions(+), 78 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index d84ff75d76..4617e018ff 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -510,31 +510,6 @@ describe('', () => { await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); - it('should preserve the tab while switching from a component to a collection', async () => { - await renderLibraryPage(); - - // Click on the first collection - fireEvent.click((await screen.findByText('Collection 1'))); - - // Click on the Details tab - fireEvent.click(screen.getByRole('tab', { name: 'Details' })); - - // Change to a component - fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]); - - // Check that the Details tab is still selected - expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true'); - - // Click on the Previews tab - fireEvent.click(screen.getByRole('tab', { name: 'Preview' })); - - // Switch back to the collection - fireEvent.click((await screen.findByText('Collection 1'))); - - // The Details (default) tab should be selected because the collection does not have a Preview tab - expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true'); - }); - const problemTypes = { 'Multiple Choice': 'choiceresponse', Checkboxes: 'multiplechoiceresponse', @@ -1063,3 +1038,84 @@ describe('', () => { ); }); }); + +describe('', () => { + beforeAll(() => { + jest.useFakeTimers(); + }); + + beforeEach(async () => { + const mocks = initializeMocks(); + axiosMock = mocks.axiosMock; + mockShowToast = mocks.mockShowToast; + axiosMock.onGet(getStudioHomeApiUrl()).reply(200, studioHomeMock); + + // The Meilisearch client-side API uses fetch, not Axios. + fetchMock.mockReset(); + fetchMock.post(searchEndpoint, (_url, req) => { + const requestData = JSON.parse(req.body?.toString() ?? ''); + const query = requestData?.queries[0]?.q ?? ''; + // We have to replace the query (search keywords) in the mock results with the actual query, + // because otherwise Instantsearch will update the UI and change the query, + // leading to unexpected results in the test cases. + const newMockResult = { ...mockResult }; + newMockResult.results[0].query = query; + // And fake the required '_formatted' fields; it contains the highlighting ... around matched words + // eslint-disable-next-line no-underscore-dangle, no-param-reassign + newMockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); + return newMockResult; + }); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + + it('should preserve the tab while switching from a component to a collection', async () => { + jest.useFakeTimers(); // ✅ enable fake timers for this test only + + render(, { path, params: { libraryId: mockContentLibrary.libraryId } }); + + // Ensure the search endpoint is called: + // Call 1: To fetch searchable/filterable/sortable library data + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + + // Click on the first collection + fireEvent.click(await screen.findByText('Collection 1')); + + // ⏩ let sidebar open + jest.advanceTimersByTime(500); + + // Click on the Details tab + fireEvent.click(screen.getByRole('tab', { name: 'Details' })); + + // Change to a component + fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]); + + // ⏩ let sidebar switch to component + jest.advanceTimersByTime(500); + + // Check that the Details tab is still selected + expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + + // Click on the Preview tab + fireEvent.click(screen.getByRole('tab', { name: 'Preview' })); + + // Switch back to the collection + fireEvent.click(await screen.findByText('Collection 1')); + + // ⏩ let sidebar switch back to collection + jest.advanceTimersByTime(500); + + // The Details (default) tab should be selected because the collection does not have a Preview tab + expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + + jest.useRealTimers(); // ✅ restore for other tests + }); +}); diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index b748a25695..2072eaf310 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -355,29 +355,47 @@ describe('', () => { }); it('should remove component from collection and hides sidebar', async () => { + jest.useFakeTimers(); // ✅ isolate fake timers for this test + const url = getLibraryCollectionItemsApiUrl( mockContentLibrary.libraryId, mockCollection.collectionId, ); axiosMock.onDelete(url).reply(204); + const displayName = 'Introduction to Testing'; await renderLibraryCollectionPage(); // open sidebar fireEvent.click(await screen.findByText(displayName)); + + // ⏩ let the delayed sidebar open run + jest.advanceTimersByTime(500); + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).toBeInTheDocument()); - const menuBtns = await screen.findAllByRole('button', { name: 'Component actions menu' }); + const menuBtns = await screen.findAllByRole('button', { + name: 'Component actions menu', + }); + // open menu fireEvent.click(menuBtns[0]); + // click remove fireEvent.click(await screen.findByText('Remove from collection')); + await waitFor(() => { expect(axiosMock.history.delete.length).toEqual(1); }); + expect(mockShowToast).toHaveBeenCalledWith('Item successfully removed'); - // Should close sidebar as component was removed + + // ⏩ let the delayed sidebar close run + jest.advanceTimersByTime(500); + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); + + jest.useRealTimers(); // ✅ restore for other tests }); it('should show error when remove component from collection', async () => { diff --git a/src/library-authoring/component-picker/ComponentPicker.test.tsx b/src/library-authoring/component-picker/ComponentPicker.test.tsx index f7c3558a5d..4e724fa149 100644 --- a/src/library-authoring/component-picker/ComponentPicker.test.tsx +++ b/src/library-authoring/component-picker/ComponentPicker.test.tsx @@ -221,46 +221,6 @@ describe('', () => { }, '*'); }); - it('should pick component inside a collection using the sidebar', async () => { - render(); - - 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]); - - const sidebar = await screen.findByTestId('library-sidebar'); - - // Mock the collection search result - mockSearchResult(mockCollectionResult); - - // Click the add component from the component card - fireEvent.click(within(sidebar).getByRole('button', { name: 'Open' })); - - // Wait for the collection to load - await screen.findByText(/Back to Library/i); - await screen.findByText('Introduction to Testing'); - - // Click on the collection card to open the sidebar - fireEvent.click(screen.getByText('Introduction to Testing')); - - const collectionSidebar = await screen.findByTestId('library-sidebar'); - - // Click the add component from the collection sidebar - fireEvent.click(within(collectionSidebar).getByRole('button', { name: 'Add to Course' })); - - expect(postMessageSpy).toHaveBeenCalledWith({ - usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', - type: 'pickerComponentSelected', - category: 'html', - }, '*'); - }); - it('should return to library selection', async () => { render(); @@ -428,3 +388,69 @@ describe('', () => { expect(screen.queryByText(/never published/i)).not.toBeInTheDocument(); }); }); + +describe(' with collection', () => { + beforeEach(() => { + initializeMocks(); + postMessageSpy = jest.spyOn(window.parent, 'postMessage'); + + mockSearchResult({ ...mockResult }); + }); + + it('should pick component inside a collection using the sidebar', async () => { + jest.useFakeTimers(); // ✅ enable fake timers + + render(); + + 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 + const collections = await screen.findAllByText('Collection 1'); // ✅ wait until it renders + fireEvent.click(collections[0]); + + // ⏩ let the 500ms single-click timer finish + jest.advanceTimersByTime(500); + + const sidebar = await screen.findByTestId('library-sidebar'); + + // Mock the collection search result + mockSearchResult(mockCollectionResult); + + // Click the add component from the component card + fireEvent.click(within(sidebar).getByRole('button', { name: 'Open' })); + + // ⏩ advance timers again in case sidebar open uses timeout + jest.advanceTimersByTime(500); + + // Wait for the collection to load + await screen.findByText(/Back to Library/i); + await screen.findByText('Introduction to Testing'); + + // Click on the collection card to open the sidebar + fireEvent.click(screen.getByText('Introduction to Testing')); + + // ⏩ advance timers again for delayed sidebar open + jest.advanceTimersByTime(500); + + const collectionSidebar = await screen.findByTestId('library-sidebar'); + + // Click the add component from the collection sidebar + fireEvent.click(within(collectionSidebar).getByRole('button', { name: 'Add to Course' })); + + expect(postMessageSpy).toHaveBeenCalledWith( + { + usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', + type: 'pickerComponentSelected', + category: 'html', + }, + '*', + ); + + jest.useRealTimers(); // ✅ restore real timers + }); +}); diff --git a/src/library-authoring/components/ComponentCard.test.tsx b/src/library-authoring/components/ComponentCard.test.tsx index fd0a76e7e6..e5711fee2d 100644 --- a/src/library-authoring/components/ComponentCard.test.tsx +++ b/src/library-authoring/components/ComponentCard.test.tsx @@ -118,6 +118,29 @@ describe('', () => { }); }); + it('should open component editor immediately on double click', async () => { + jest.useFakeTimers(); // ✅ enable fake timers + + initializeMocks(); + render(); + + const card = await screen.findByText('Text Display Formated Name'); + + // First click starts the timeout + fireEvent.click(card); + // Second click cancels timeout + opens editor immediately + fireEvent.click(card); + + // ⏩ No need to runAllTimers, double click bypasses the timer + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: `/library/${libraryId}/${contentHit.usageKey}`, + search: '', + }); + + jest.useRealTimers(); // ✅ restore timers + }); + it('should select component on clicking edit menu option', async () => { initializeMocks(); render(); diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index d3fa33b86e..15ea949ca7 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -1,4 +1,4 @@ -import { useCallback } from 'react'; +import { useCallback, useRef } from 'react'; import { ActionRow, } from '@openedx/paragon'; @@ -15,11 +15,15 @@ type ComponentCardProps = { hit: ContentHit, }; +const DOUBLE_CLICK_DELAY = 500; // ms + const ComponentCard = ({ hit }: ComponentCardProps) => { - const { showOnlyPublished } = useLibraryContext(); + const { showOnlyPublished, openComponentEditor } = useLibraryContext(); const { openComponentInfoSidebar, openItemSidebar, sidebarItemInfo } = useSidebarContext(); const { componentPickerMode } = useComponentPickerContext(); + const clickTimerRef = useRef | null>(null); + const { blockType, formatted, @@ -34,15 +38,27 @@ const ComponentCard = ({ hit }: ComponentCardProps) => { showOnlyPublished ? formatted.published?.displayName : formatted.displayName ) ?? ''; - const selectComponent = useCallback(() => { - if (!componentPickerMode) { - openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo); - } else { - // In component picker mode, we want to open the sidebar - // without changing the URL - openComponentInfoSidebar(usageKey); - } - }, [usageKey, openItemSidebar, openComponentInfoSidebar, componentPickerMode]); + const selectComponent = useCallback( + () => { + if (clickTimerRef.current) { + clearTimeout(clickTimerRef.current); + clickTimerRef.current = null; + openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo); + openComponentEditor(usageKey); + } else { + clickTimerRef.current = setTimeout(() => { + clickTimerRef.current = null; + + if (componentPickerMode) { + openComponentInfoSidebar(usageKey); + } else { + openItemSidebar(usageKey, SidebarBodyItemId.ComponentInfo); + } + }, DOUBLE_CLICK_DELAY); + } + }, + [usageKey, openItemSidebar, openComponentInfoSidebar, componentPickerMode], + ); const selected = sidebarItemInfo?.type === SidebarBodyItemId.ComponentInfo && sidebarItemInfo.id === usageKey;