Skip to content

Commit

Permalink
refactor: remove parentLocator and next button from lib component pic…
Browse files Browse the repository at this point in the history
…ker (#1412)
  • Loading branch information
navinkarkera authored Oct 21, 2024
1 parent 56e025a commit d49fc85
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 79 deletions.
5 changes: 0 additions & 5 deletions src/library-authoring/common/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand All @@ -86,7 +83,6 @@ export const LibraryProvider = ({
libraryId,
collectionId: collectionIdProp,
componentPickerMode = false,
parentLocator,
initialSidebarComponentUsageKey,
initialSidebarCollectionId,
}: LibraryProviderProps) => {
Expand Down Expand Up @@ -144,7 +140,6 @@ export const LibraryProvider = ({
readOnly,
isLoadingLibraryData,
componentPickerMode,
parentLocator,
sidebarBodyComponent,
closeLibrarySidebar,
openAddContentSidebar,
Expand Down
2 changes: 0 additions & 2 deletions src/library-authoring/component-info/ComponentInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const ComponentInfo = () => {
readOnly,
openComponentEditor,
componentPickerMode,
parentLocator,
} = useLibraryContext();

// istanbul ignore if: this should never happen
Expand All @@ -35,7 +34,6 @@ const ComponentInfo = () => {

const handleAddComponentToCourse = () => {
window.parent.postMessage({
parentLocator,
usageKey,
type: 'pickerComponentSelected',
category: getBlockType(usageKey),
Expand Down
26 changes: 0 additions & 26 deletions src/library-authoring/component-picker/ComponentPicker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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',
Expand All @@ -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();
Expand All @@ -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',
Expand All @@ -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();
Expand All @@ -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',
Expand All @@ -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();
Expand Down Expand Up @@ -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',
Expand All @@ -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));
Expand Down
35 changes: 8 additions & 27 deletions src/library-authoring/component-picker/ComponentPicker.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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('');
Expand All @@ -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>
);
};
12 changes: 0 additions & 12 deletions src/library-authoring/component-picker/SelectLibrary.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 1 addition & 5 deletions src/library-authoring/component-picker/SelectLibrary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions src/library-authoring/components/ComponentCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ const ComponentCard = ({ contentHit }: ComponentCardProps) => {
const {
openComponentInfoSidebar,
componentPickerMode,
parentLocator,
} = useLibraryContext();

const {
Expand All @@ -111,7 +110,6 @@ const ComponentCard = ({ contentHit }: ComponentCardProps) => {

const handleAddComponentToCourse = () => {
window.parent.postMessage({
parentLocator,
usageKey,
type: 'pickerComponentSelected',
category: blockType,
Expand Down

0 comments on commit d49fc85

Please sign in to comment.