From d49fc85163a4b84f3536069c47df87d5e33a3520 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 21 Oct 2024 22:35:04 +0530 Subject: [PATCH] refactor: remove parentLocator and next button from lib component picker (#1412) --- src/library-authoring/common/context.tsx | 5 --- .../component-info/ComponentInfo.tsx | 2 -- .../component-picker/ComponentPicker.test.tsx | 26 -------------- .../component-picker/ComponentPicker.tsx | 35 +++++-------------- .../component-picker/SelectLibrary.test.tsx | 12 ------- .../component-picker/SelectLibrary.tsx | 6 +--- .../components/ComponentCard.tsx | 2 -- 7 files changed, 9 insertions(+), 79 deletions(-) diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index 924d4d4b3..cc085ef77 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -26,7 +26,6 @@ export interface LibraryContextData { setCollectionId: (collectionId?: string) => void; // Whether we're in "component picker" mode componentPickerMode: boolean; - parentLocator?: string; // Sidebar stuff - only one sidebar is active at any given time: sidebarBodyComponent: SidebarBodyComponentId | null; closeLibrarySidebar: () => void; @@ -70,8 +69,6 @@ interface LibraryProviderProps { /** The component picker mode is a special mode where the user is selecting a component to add to a Unit (or another * XBlock) */ componentPickerMode?: boolean; - /** The parent component locator, if we're in component picker mode */ - parentLocator?: string; /** Only used for testing */ initialSidebarComponentUsageKey?: string; /** Only used for testing */ @@ -86,7 +83,6 @@ export const LibraryProvider = ({ libraryId, collectionId: collectionIdProp, componentPickerMode = false, - parentLocator, initialSidebarComponentUsageKey, initialSidebarCollectionId, }: LibraryProviderProps) => { @@ -144,7 +140,6 @@ export const LibraryProvider = ({ readOnly, isLoadingLibraryData, componentPickerMode, - parentLocator, sidebarBodyComponent, closeLibrarySidebar, openAddContentSidebar, diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx index e6ea4d74c..4cbb35598 100644 --- a/src/library-authoring/component-info/ComponentInfo.tsx +++ b/src/library-authoring/component-info/ComponentInfo.tsx @@ -23,7 +23,6 @@ const ComponentInfo = () => { readOnly, openComponentEditor, componentPickerMode, - parentLocator, } = useLibraryContext(); // istanbul ignore if: this should never happen @@ -35,7 +34,6 @@ const ComponentInfo = () => { const handleAddComponentToCourse = () => { window.parent.postMessage({ - parentLocator, usageKey, type: 'pickerComponentSelected', category: getBlockType(usageKey), diff --git a/src/library-authoring/component-picker/ComponentPicker.test.tsx b/src/library-authoring/component-picker/ComponentPicker.test.tsx index 030dd4662..7ec097cb0 100644 --- a/src/library-authoring/component-picker/ComponentPicker.test.tsx +++ b/src/library-authoring/component-picker/ComponentPicker.test.tsx @@ -25,18 +25,6 @@ mockLibraryBlockMetadata.applyMock(); let postMessageSpy: jest.SpyInstance; -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useSearchParams: () => { - const [params] = [new URLSearchParams({ - parentLocator: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical1', - })]; - return [ - params, - ]; - }, -})); - describe('', () => { beforeEach(() => { initializeMocks(); @@ -51,8 +39,6 @@ describe('', () => { expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i)); - fireEvent.click(screen.getByText('Next')); - // Wait for the content library to load await screen.findByText(/Change Library/i); expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); @@ -61,7 +47,6 @@ describe('', () => { fireEvent.click(screen.queryAllByRole('button', { name: 'Add' })[0]); expect(postMessageSpy).toHaveBeenCalledWith({ - parentLocator: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical1', usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', type: 'pickerComponentSelected', category: 'html', @@ -74,8 +59,6 @@ describe('', () => { expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i)); - fireEvent.click(screen.getByText('Next')); - // Wait for the content library to load await screen.findByText(/Change Library/i); expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); @@ -89,7 +72,6 @@ describe('', () => { fireEvent.click(within(sidebar).getByRole('button', { name: 'Add to Course' })); expect(postMessageSpy).toHaveBeenCalledWith({ - parentLocator: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical1', usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', type: 'pickerComponentSelected', category: 'html', @@ -102,8 +84,6 @@ describe('', () => { expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i)); - fireEvent.click(screen.getByText('Next')); - // Wait for the content library to load await screen.findByText(/Change Library/i); expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); @@ -127,7 +107,6 @@ describe('', () => { fireEvent.click(screen.queryAllByRole('button', { name: 'Add' })[0]); expect(postMessageSpy).toHaveBeenCalledWith({ - parentLocator: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical1', usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', type: 'pickerComponentSelected', category: 'html', @@ -140,8 +119,6 @@ describe('', () => { expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i)); - fireEvent.click(screen.getByText('Next')); - // Wait for the content library to load await screen.findByText(/Change Library/i); expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); @@ -170,7 +147,6 @@ describe('', () => { fireEvent.click(within(collectionSidebar).getByRole('button', { name: 'Add to Course' })); expect(postMessageSpy).toHaveBeenCalledWith({ - parentLocator: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical1', usageKey: 'lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd', type: 'pickerComponentSelected', category: 'html', @@ -183,8 +159,6 @@ describe('', () => { expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i)); - fireEvent.click(screen.getByText('Next')); - // Wait for the content library to load await screen.findByText(/Change Library/i); fireEvent.click(screen.getByText(/Change Library/i)); diff --git a/src/library-authoring/component-picker/ComponentPicker.tsx b/src/library-authoring/component-picker/ComponentPicker.tsx index 933691a61..372506d4c 100644 --- a/src/library-authoring/component-picker/ComponentPicker.tsx +++ b/src/library-authoring/component-picker/ComponentPicker.tsx @@ -1,13 +1,10 @@ import React, { useState } from 'react'; -import { useIntl } from '@edx/frontend-platform/i18n'; -import { Button, Stepper } from '@openedx/paragon'; -import { useSearchParams } from 'react-router-dom'; +import { Stepper } from '@openedx/paragon'; import { LibraryProvider, useLibraryContext } from '../common/context'; import LibraryAuthoringPage from '../LibraryAuthoringPage'; import LibraryCollectionPage from '../collections/LibraryCollectionPage'; import SelectLibrary from './SelectLibrary'; -import messages from './messages'; interface LibraryComponentPickerProps { returnToLibrarySelection: () => void; @@ -24,21 +21,14 @@ const InnerComponentPicker: React.FC = ({ returnToL // eslint-disable-next-line import/prefer-default-export export const ComponentPicker = () => { - const intl = useIntl(); - const [searchParams] = useSearchParams(); - let parentLocator = searchParams.get('parentLocator'); - - // istanbul ignore if: this should never happen - if (!parentLocator) { - throw new Error('parentLocator is required'); - } - - // URLSearchParams decodes '+' to ' ', so we need to convert it back - parentLocator = parentLocator.replaceAll(' ', '+'); - const [currentStep, setCurrentStep] = useState('select-library'); const [selectedLibrary, setSelectedLibrary] = useState(''); + const handleLibrarySelection = (library: string) => { + setCurrentStep('pick-components'); + setSelectedLibrary(library); + }; + const returnToLibrarySelection = () => { setCurrentStep('select-library'); setSelectedLibrary(''); @@ -49,23 +39,14 @@ export const ComponentPicker = () => { activeKey={currentStep} > - + - + - -
- - - - -
); }; diff --git a/src/library-authoring/component-picker/SelectLibrary.test.tsx b/src/library-authoring/component-picker/SelectLibrary.test.tsx index 90681b9fb..cc06fa3af 100644 --- a/src/library-authoring/component-picker/SelectLibrary.test.tsx +++ b/src/library-authoring/component-picker/SelectLibrary.test.tsx @@ -8,18 +8,6 @@ import { } from '../data/api.mocks'; import { ComponentPicker } from './ComponentPicker'; -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useSearchParams: () => { - const [params] = [new URLSearchParams({ - parentLocator: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical1', - })]; - return [ - params, - ]; - }, -})); - describe('', () => { beforeEach(() => { initializeMocks(); diff --git a/src/library-authoring/component-picker/SelectLibrary.tsx b/src/library-authoring/component-picker/SelectLibrary.tsx index 4c4f1754c..e6d97c559 100644 --- a/src/library-authoring/component-picker/SelectLibrary.tsx +++ b/src/library-authoring/component-picker/SelectLibrary.tsx @@ -7,7 +7,7 @@ import { SearchField, Stack, } from '@openedx/paragon'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useState } from 'react'; import Loading from '../../generic/Loading'; import AlertError from '../../generic/alert-error'; @@ -36,10 +36,6 @@ const SelectLibrary = ({ selectedLibrary, setSelectedLibrary }: SelectLibraryPro const [searchQuery, setSearchQuery] = useState(''); const [currentPage, setCurrentPage] = useState(1); - useEffect(() => { - setSelectedLibrary(''); - }, [currentPage, searchQuery]); - const handleSearch = useCallback((search: string) => { setSearchQuery(search); setCurrentPage(1); diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index c8df6c202..5dd56381a 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -93,7 +93,6 @@ const ComponentCard = ({ contentHit }: ComponentCardProps) => { const { openComponentInfoSidebar, componentPickerMode, - parentLocator, } = useLibraryContext(); const { @@ -111,7 +110,6 @@ const ComponentCard = ({ contentHit }: ComponentCardProps) => { const handleAddComponentToCourse = () => { window.parent.postMessage({ - parentLocator, usageKey, type: 'pickerComponentSelected', category: blockType,