refactor: remove parentLocator and next button from lib component picker (#1412)

This commit is contained in:
Navin Karkera
2024-10-21 22:35:04 +05:30
committed by GitHub
parent 56e025a4f0
commit d49fc85163
7 changed files with 9 additions and 79 deletions

View File

@@ -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,

View File

@@ -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),

View File

@@ -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('<ComponentPicker />', () => {
beforeEach(() => {
initializeMocks();
@@ -51,8 +39,6 @@ describe('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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('<ComponentPicker />', () => {
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));

View File

@@ -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<LibraryComponentPickerProps> = ({ 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}
>
<Stepper.Step eventKey="select-library" title="Select a library">
<SelectLibrary selectedLibrary={selectedLibrary} setSelectedLibrary={setSelectedLibrary} />
<SelectLibrary selectedLibrary={selectedLibrary} setSelectedLibrary={handleLibrarySelection} />
</Stepper.Step>
<Stepper.Step eventKey="pick-components" title="Pick some components">
<LibraryProvider libraryId={selectedLibrary} parentLocator={parentLocator} componentPickerMode>
<LibraryProvider libraryId={selectedLibrary} componentPickerMode>
<InnerComponentPicker returnToLibrarySelection={returnToLibrarySelection} />
</LibraryProvider>
</Stepper.Step>
<div className="p-5">
<Stepper.ActionRow eventKey="select-library">
<Stepper.ActionRow.Spacer />
<Button onClick={() => setCurrentStep('pick-components')} disabled={!selectedLibrary}>
{intl.formatMessage(messages.selectLibraryNextButton)}
</Button>
</Stepper.ActionRow>
</div>
</Stepper>
);
};

View File

@@ -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('<ComponentPicker />', () => {
beforeEach(() => {
initializeMocks();

View File

@@ -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);

View File

@@ -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,