Skip to content

Add subsection/unit to section/subsection full-page view within library [FC-0090] #2152

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

Merged
merged 14 commits into from
Jun 24, 2025
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
28 changes: 18 additions & 10 deletions src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ const LibraryAuthoringPage = ({
insideCollections,
insideComponents,
insideUnits,
insideSection,
insideSections,
insideSubsection,
insideSubsections,
navigateTo,
} = useLibraryRoutes();
Expand Down Expand Up @@ -251,8 +253,12 @@ const LibraryAuthoringPage = ({
extraFilter.push(activeTypeFilters[activeKey]);
}

// Disable filtering by block/problem type when viewing the Collections/Units/Sections/Subsections tab.
const onlyOneType = (insideCollections || insideUnits || insideSections || insideSubsections);
// Disable filtering by block/problem type when viewing the Collections/Units/Sections/Subsections tab,
// or when inside a specific Section or Subsection.
const onlyOneType = (
insideCollections || insideUnits || insideSections || insideSubsections
|| insideSection || insideSubsection
);
const overrideTypesFilter = onlyOneType
? new TypesFilterData()
: undefined;
Expand Down Expand Up @@ -298,14 +304,16 @@ const LibraryAuthoringPage = ({
headerActions={<HeaderActions />}
hideBorder
/>
<Tabs
variant="tabs"
activeKey={activeKey}
onSelect={handleTabChange}
className="my-3"
>
{visibleTabsToRender}
</Tabs>
{visibleTabs.length > 1 && (
<Tabs
variant="tabs"
activeKey={activeKey}
onSelect={handleTabChange}
className="my-3"
>
{visibleTabsToRender}
</Tabs>
)}
<ActionRow className="my-3">
<SearchKeywordsField className="mr-3" />
<FilterByTags />
Expand Down
17 changes: 13 additions & 4 deletions src/library-authoring/add-content/AddContent.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ describe('<AddContent />', () => {
expect(screen.queryByRole('button', { name: /video/i })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: /copy from clipboard/i })).not.toBeInTheDocument();
expect(await screen.findByRole('button', { name: /advanced \/ other/i })).toBeInTheDocument();
expect(await screen.queryByRole('button', { name: /existing library content/i })).not.toBeInTheDocument();
});

it('should render advanced content buttons', async () => {
Expand Down Expand Up @@ -331,6 +332,7 @@ describe('<AddContent />', () => {
const unitId = 'lct:orf1:lib1:unit:test-1';
renderWithContainer(unitId);

expect(await screen.findByRole('button', { name: /existing library content/i })).toBeInTheDocument();
expect(await screen.findByRole('button', { name: 'Text' })).toBeInTheDocument();

expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument();
Expand Down Expand Up @@ -396,25 +398,32 @@ describe('<AddContent />', () => {
expect(mockShowToast).toHaveBeenCalledWith('There was an error linking the content to this container.');
});

it('should only show subsection button when inside a section', async () => {
it('should only show subsection buttons when inside a section', async () => {
mockClipboardEmpty.applyMock();
const sectionId = 'lct:orf1:lib1:section:test-1';
renderWithContainer(sectionId, 'section');

expect(await screen.findByRole('button', { name: 'Subsection' })).toBeInTheDocument();
expect(await screen.findByRole('button', { name: /existing subsection/i })).toBeInTheDocument();

// Button is labeled "New Subsection" , not "Subsection"
expect(await screen.findByRole('button', { name: /new subsection/i })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Subsection' })).not.toBeInTheDocument();

expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Unit' })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Section' })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Text' })).not.toBeInTheDocument();
});

it('should only show unit button when inside a subsection', async () => {
it('should only show unit buttons when inside a subsection', async () => {
mockClipboardEmpty.applyMock();
const subsectionId = 'lct:orf1:lib1:subsection:test-1';
renderWithContainer(subsectionId, 'subsection');

expect(await screen.findByRole('button', { name: 'Unit' })).toBeInTheDocument();
expect(await screen.findByRole('button', { name: /existing unit/i })).toBeInTheDocument();
// Button is labeled "New Unit" in this context, not just "Unit"
expect(await screen.findByRole('button', { name: /new unit/i })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Unit' })).not.toBeInTheDocument();

expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Subsection' })).not.toBeInTheDocument();
Expand Down
179 changes: 107 additions & 72 deletions src/library-authoring/add-content/AddContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ import { blockTypes } from '../../editors/data/constants/app';

import { useLibraryRoutes } from '../routes';
import genericMessages from '../generic/messages';
import messages from './messages';
import { messages, getContentMessages } from './messages';
import type { BlockTypeMetadata } from '../data/api';
import { ContainerType } from '../../generic/key-utils';

type ContentType = {
name: string,
disabled: boolean,
disabled?: boolean,
icon?: React.ComponentType,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
icon?: React.ComponentType,

Can we delete this icon field now? Seems to be unused after these changes.

Copy link
Contributor Author

@pomegranited pomegranited Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I made a mistake here -- we do need that icon parameter so we can show the little sparkle next to "Advanced / Other":

image

Fixed with 2e50209.

blockType: string,
};
Expand All @@ -64,7 +64,7 @@ type AddAdvancedContentViewProps = {
const AddContentButton = ({ contentType, onCreateContent } : AddContentButtonProps) => {
const {
name,
disabled,
disabled = false,
icon,
blockType,
} = contentType;
Expand Down Expand Up @@ -96,97 +96,132 @@ const AddContentView = ({
insideSubsection,
} = useLibraryRoutes();

const collectionButtonData = {
name: intl.formatMessage(messages.collectionButton),
disabled: false,
blockType: 'collection',
};
const contentMessages = useMemo(() => (
getContentMessages(insideSection, insideSubsection, insideUnit)
), [insideSection, insideSubsection, insideUnit]);

const collectionButton = (
<AddContentButton
key="collection"
contentType={{
name: intl.formatMessage(contentMessages.collectionButton),
blockType: 'collection',
}}
onCreateContent={onCreateContent}
/>
);

const unitButtonData = {
name: intl.formatMessage(messages.unitButton),
disabled: false,
blockType: 'vertical',
};
const unitButton = (
<AddContentButton
key="unit"
contentType={{
name: intl.formatMessage(contentMessages.unitButton),
blockType: 'unit',
}}
onCreateContent={onCreateContent}
/>
);

const sectionButtonData = {
name: intl.formatMessage(messages.sectionButton),
disabled: false,
blockType: 'chapter',
};
const sectionButton = (
<AddContentButton
key="section"
contentType={{
name: intl.formatMessage(contentMessages.sectionButton),
blockType: 'section',
}}
onCreateContent={onCreateContent}
/>
);

const subsectionButtonData = {
name: intl.formatMessage(messages.subsectionButton),
disabled: false,
blockType: 'sequential',
};
const subsectionButton = (
<AddContentButton
key="subsection"
contentType={{
name: intl.formatMessage(contentMessages.subsectionButton),
blockType: 'subsection',
}}
onCreateContent={onCreateContent}
/>
);

const libraryContentButtonData = {
name: intl.formatMessage(messages.libraryContentButton),
disabled: false,
blockType: 'libraryContent',
};
const existingContentButton = (
<AddContentButton
key="libraryContent"
contentType={{
name: intl.formatMessage(contentMessages.libraryContentButton),
blockType: 'libraryContent',
}}
onCreateContent={onCreateContent}
/>
);

/** List container content types that should be displayed based on current path */
const visibleContentTypes = useMemo(() => {
/* Note: for MVP we are hiding the unsupported types, not just disabling them. */
const componentButtons = contentTypes.filter(ct => !ct.disabled).map((contentType) => (
<AddContentButton
key={`add-content-${contentType.blockType}`}
contentType={contentType}
onCreateContent={onCreateContent}
/>
));
const separator = (
<hr className="w-100 bg-gray-500" />
);

/** List buttons that should be displayed based on current path */
const visibleButtons = useMemo(() => {
if (insideCollection) {
// except for add collection button, show everthing.
// except for add collection button, show everything.
return [
libraryContentButtonData,
sectionButtonData,
subsectionButtonData,
unitButtonData,
existingContentButton,
sectionButton,
subsectionButton,
unitButton,
separator,
...componentButtons,
];
}
if (insideUnit) {
// Only show libraryContentButton
return [libraryContentButtonData];
// Only show existing content button + component buttons
return [
existingContentButton,
separator,
...componentButtons,
];
}
if (insideSection) {
// Should only allow adding subsections
return [subsectionButtonData];
// Only allow adding subsections
return [
existingContentButton,
subsectionButton,
];
}
if (insideSubsection) {
// Should only allow adding units
return [unitButtonData];
// Only allow adding units
return [
existingContentButton,
unitButton,
];
}
// except for libraryContentButton, show everthing.
// except for existing content, show everything.
return [
collectionButtonData,
sectionButtonData,
subsectionButtonData,
unitButtonData,
collectionButton,
sectionButton,
subsectionButton,
unitButton,
separator,
...componentButtons,
];
}, [insideCollection, insideUnit, insideSection, insideSubsection]);
}, [componentButtons, insideCollection, insideUnit, insideSection, insideSubsection]);

return (
<>
{visibleContentTypes.map((contentType) => (
<AddContentButton
key={contentType.blockType}
contentType={contentType}
onCreateContent={onCreateContent}
/>
))}
{componentPicker && visibleContentTypes.includes(libraryContentButtonData) && (
/// Show the "Add Library Content" button for units and collections
{visibleButtons}
{componentPicker && visibleButtons.includes(existingContentButton) && (
<PickLibraryContentModal
isOpen={isAddLibraryContentModalOpen}
onClose={closeAddLibraryContentModal}
/>
)}
{(!insideSection && !insideSubsection) && (
<>
<hr className="w-100 bg-gray-500" />
{/* Note: for MVP we are hiding the unuspported types, not just disabling them. */}
{contentTypes.filter(ct => !ct.disabled).map((contentType) => (
<AddContentButton
key={`add-content-${contentType.blockType}`}
contentType={contentType}
onCreateContent={onCreateContent}
/>
))}
</>
)}
</>
);
};
Expand Down Expand Up @@ -423,9 +458,9 @@ const AddContent = () => {
} else if (blockType === 'advancedXBlock') {
showAdvancedList();
} else if ([
ContainerType.Vertical,
ContainerType.Chapter,
ContainerType.Sequential,
ContainerType.Unit,
ContainerType.Subsection,
ContainerType.Section,
].includes(blockType as ContainerType)) {
setCreateContainerModalType(blockType as ContainerType);
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/add-content/AddContentHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { FormattedMessage } from '@edx/frontend-platform/i18n';
import messages from './messages';
import { messages } from './messages';

const AddContentHeader = () => (
<span className="font-weight-bold m-1.5">
Expand Down
Loading