Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove parentLocator and next button from component picker [FC-0062] #1412

Merged
merged 2 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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