Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion src/course-unit/add-component/AddComponent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import ComponentModalView from './add-component-modals/ComponentModalView';
import AddComponentButton from './add-component-btn';
import messages from './messages';
import { ComponentPicker } from '../../library-authoring/component-picker';
import { ContentType } from '../../library-authoring/routes';
import { messageTypes } from '../constants';
import { useIframe } from '../../generic/hooks/context/hooks';
import { useEventListener } from '../../generic/hooks';
Expand Down Expand Up @@ -228,7 +229,8 @@ const AddComponent = ({
>
<ComponentPicker
showOnlyPublished
extraFilter={['NOT block_type = "unit"']}
extraFilter={['NOT block_type = "unit"', 'NOT block_type = "section"', 'NOT block_type = "subsection"']}
visibleTabs={[ContentType.home, ContentType.components, ContentType.collections]}
componentPickerMode={isAddLibraryContentModalOpen ? 'single' : 'multiple'}
onComponentSelected={handleLibraryV2Selection}
onChangeComponentSelection={setSelectedComponents}
Expand Down
9 changes: 5 additions & 4 deletions src/generic/block-type-utils/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@
}

.btn {
background-color: lighten(#0B8E77, 10%);
background-color: lighten(#EA3E3E, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#0B8E77, 20%);
background-color: lighten(#EA3E3E, 20%);
border: 1px solid $primary;
margin: -1px;
}
Expand All @@ -162,11 +162,12 @@
}

.btn {
background-color: lighten(#0B8E77, 10%);
background-color: lighten(#45009E, 10%);
border: 0;
color: white;

&:hover, &:active, &:focus {
background-color: lighten(#0B8E77, 20%);
background-color: lighten(#45009E, 20%);
border: 1px solid $primary;
margin: -1px;
}
Expand Down
13 changes: 11 additions & 2 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,20 @@ const App = () => {
<Route path="/library/:libraryId/*" element={<LibraryLayout />} />
<Route
path="/component-picker"
element={<ComponentPicker extraFilter={['NOT block_type = "unit"']} />}
element={(
<ComponentPicker
extraFilter={['NOT block_type = "unit", "NOT block_type = "section"', 'NOT block_type = "subsection"']}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, shouldn't visibleTabs be added like this? I'm not sure where this case is used to see the Component Picker

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, shouldn't visibleTabs be added like this? I'm not sure where this case is used to see the Component Picker

Sure! That makes sense!
Fixed: a4a6d99

Note: This current code is used on the legacy studio, where an iframe opens the route http://apps.local.edly.io:2001/authoring/component-picker to select components for courses.

/>
)}
/>
<Route
path="/component-picker/multiple"
element={<ComponentPicker componentPickerMode="multiple" extraFilter={['NOT block_type = "unit"']} />}
element={(
<ComponentPicker
componentPickerMode="multiple"
extraFilter={['NOT block_type = "unit", "NOT block_type = "section"', 'NOT block_type = "subsection"']}
/>
)}
/>
<Route path="/legacy/preview-changes/:usageKey" element={<PreviewChangesEmbed />} />
<Route path="/course/:courseId/*" element={<CourseAuthoringRoutes />} />
Expand Down
24 changes: 21 additions & 3 deletions src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,33 @@ describe('<LibraryAuthoringPage />', () => {
expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument();

// Navigate to the components tab
fireEvent.click(screen.getByRole('tab', { name: 'Components' }));
const componentsTab = screen.getByRole('tab', { name: 'Components' });
fireEvent.click(componentsTab);
expect(componentsTab).toHaveAttribute('aria-selected', 'true');
expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument();

// Navigate to the collections tab
fireEvent.click(screen.getByRole('tab', { name: 'Collections' }));
const collectionsTab = screen.getByRole('tab', { name: 'Collections' });
fireEvent.click(collectionsTab);
expect(collectionsTab).toHaveAttribute('aria-selected', 'true');
expect(await screen.findByText('No matching collections found in this library.')).toBeInTheDocument();

// Navigate to the units tab
fireEvent.click(screen.getByRole('tab', { name: 'Units' }));
const unitsTab = screen.getByRole('tab', { name: 'Units' });
fireEvent.click(unitsTab);
expect(unitsTab).toHaveAttribute('aria-selected', 'true');
expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument();

// Navigate to the subsections tab
const subsectionsTab = screen.getByRole('tab', { name: 'Subsections' });
fireEvent.click(subsectionsTab);
expect(subsectionsTab).toHaveAttribute('aria-selected', 'true');
expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument();

// Navigate to the subsections tab
const sectionsTab = screen.getByRole('tab', { name: 'Sections' });
fireEvent.click(sectionsTab);
expect(sectionsTab).toHaveAttribute('aria-selected', 'true');
expect(await screen.findByText('No matching components found in this library.')).toBeInTheDocument();

// Go back to Home tab
Expand Down
49 changes: 22 additions & 27 deletions src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
type ReactNode,
useCallback,
useEffect,
useState,
} from 'react';
import { Helmet } from 'react-helmet';
Expand Down Expand Up @@ -72,8 +71,8 @@ const HeaderActions = () => {
}

if (!componentPickerMode) {
// Reset URL to library home
navigateTo({ componentId: '', collectionId: '', unitId: '' });
// If not in component picker mode, reset selected item when opening the info sidebar
navigateTo({ selectedItemId: '' });
}
}, [navigateTo, sidebarComponentInfo, closeLibrarySidebar, openLibrarySidebar]);

Expand Down Expand Up @@ -151,16 +150,15 @@ const LibraryAuthoringPage = ({
isLoadingLibraryData,
showOnlyPublished,
extraFilter: contextExtraFilter,
componentId,
collectionId,
unitId,
} = useLibraryContext();
const { openInfoSidebar, sidebarComponentInfo } = useSidebarContext();
const { sidebarComponentInfo } = useSidebarContext();

const {
insideCollections,
insideComponents,
insideUnits,
insideSections,
insideSubsections,
navigateTo,
} = useLibraryRoutes();

Expand All @@ -178,16 +176,16 @@ const LibraryAuthoringPage = ({
if (insideUnits) {
return ContentType.units;
}
if (insideSubsections) {
return ContentType.subsections;
}
if (insideSections) {
return ContentType.sections;
}
return ContentType.home;
};
const [activeKey, setActiveKey] = useState<ContentType>(getActiveKey);

useEffect(() => {
if (!componentPickerMode) {
openInfoSidebar(componentId, collectionId, unitId);
}
}, []);

Comment on lines -185 to -190

@rpenido rpenido May 28, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved to SidebarContext

if (isLoadingLibraryData) {
return <Loading />;
}
Expand Down Expand Up @@ -241,31 +239,28 @@ const LibraryAuthoringPage = ({
components: 'type = "library_block"',
collections: 'type = "collection"',
units: 'block_type = "unit"',
subsections: 'block_type = "subsection"',
sections: 'block_type = "section"',
};
if (activeKey !== ContentType.home) {
extraFilter.push(activeTypeFilters[activeKey]);
}

/*

<FilterByPublished key={
// It is necessary to re-render `FilterByPublished` every time `FilterByBlockType`
// appears or disappears, this is because when the menu is opened it is rendered
// in a previous state, causing an inconsistency in its position.
// By changing the key we can re-render the component.
!(insideCollections || insideUnits) ? 'filter-published-1' : 'filter-published-2'
}
*/

Comment on lines -249 to -259

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This seems to be a temporary/not used code commented

// Disable filtering by block/problem type when viewing the Collections tab.
const overrideTypesFilter = (insideCollections || insideUnits) ? new TypesFilterData() : undefined;
// Disable filtering by block/problem type when viewing the Collections/Units/Sections/Subsections tab.
const onlyOneType = (insideCollections || insideUnits || insideSections || insideSubsections);
const overrideTypesFilter = onlyOneType
? new TypesFilterData()
: undefined;

const tabTitles = {
[ContentType.home]: intl.formatMessage(messages.homeTab),
[ContentType.collections]: intl.formatMessage(messages.collectionsTab),
[ContentType.components]: intl.formatMessage(messages.componentsTab),
[ContentType.units]: intl.formatMessage(messages.unitsTab),
[ContentType.subsections]: intl.formatMessage(messages.subsectionsTab),
[ContentType.sections]: intl.formatMessage(messages.sectionsTab),
};

const visibleTabsToRender = visibleTabs.map((contentType) => (
<Tab key={contentType} eventKey={contentType} title={tabTitles[contentType]} />
));
Expand Down Expand Up @@ -309,7 +304,7 @@ const LibraryAuthoringPage = ({
<ActionRow className="my-3">
<SearchKeywordsField className="mr-3" />
<FilterByTags />
{!(insideCollections || insideUnits) && <FilterByBlockType />}
{!(onlyOneType) && <FilterByBlockType />}
<LibraryFilterByPublished key={
// It is necessary to re-render `LibraryFilterByPublished` every time `FilterByBlockType`
// appears or disappears, this is because when the menu is opened it is rendered
Expand Down
60 changes: 26 additions & 34 deletions src/library-authoring/LibraryLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { useCallback } from 'react';
import React from 'react';
import {
Outlet,
Route,
Routes,
useMatch,
useParams,
type PathMatch,
} from 'react-router-dom';

import { BASE_ROUTE, ROUTES } from './routes';
import { ROUTES } from './routes';
import LibraryAuthoringPage from './LibraryAuthoringPage';
import { LibraryProvider } from './common/context/LibraryContext';
import { SidebarProvider } from './common/context/SidebarContext';
Expand All @@ -18,70 +17,63 @@ import { ComponentPicker } from './component-picker';
import { ComponentEditorModal } from './components/ComponentEditorModal';
import { LibraryUnitPage } from './units';

const LibraryLayout = () => {
const { libraryId } = useParams();
const LibraryLayoutWrapper: React.FC<React.PropsWithChildren> = ({ children }) => {
const { libraryId, collectionId, unitId } = useParams();

if (libraryId === undefined) {
// istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker.
throw new Error('Error: route is missing libraryId.');
}

// The top-level route is `${BASE_ROUTE}/*`, so match will always be non-null.
const matchCollection = useMatch(`${BASE_ROUTE}${ROUTES.COLLECTION}`) as PathMatch<'libraryId' | 'collectionId'> | null;
const collectionId = matchCollection?.params.collectionId;

// The top-level route is `${BASE_ROUTE}/*`, so match will always be non-null.
const matchUnit = useMatch(`${BASE_ROUTE}${ROUTES.UNIT}`) as PathMatch<'libraryId' | 'unitId'> | null;
const unitId = matchUnit?.params.unitId;

const context = useCallback((childPage) => (
return (
<LibraryProvider
/** We need to pass the collectionId or unitId as key to the LibraryProvider to force a re-render
* when we navigate to a collection or unit page. */
/** NOTE: We need to pass the collectionId or unitId as key to the LibraryProvider to force a re-render
* when we navigate to a collection or unit page. This is necessary to make the back/forward navigation
* work correctly, as the LibraryProvider needs to rebuild the state from the URL.
* */
key={collectionId || unitId}
libraryId={libraryId}
/** The component picker modal to use. We need to pass it as a reference instead of
/** NOTE: The component picker modal to use. We need to pass it as a reference instead of
* directly importing it to avoid the import cycle:
* ComponentPicker > LibraryAuthoringPage/LibraryCollectionPage >
* Sidebar > AddContent > ComponentPicker */
componentPicker={ComponentPicker}
>
<SidebarProvider>
<>
{childPage}
<CreateCollectionModal />
<CreateContainerModal />
<ComponentEditorModal />
</>
{children ?? <Outlet />}
<CreateCollectionModal />
<CreateContainerModal />
<ComponentEditorModal />
</SidebarProvider>
</LibraryProvider>
), [collectionId, unitId]);
);
};

return (
<Routes>
const LibraryLayout = () => (
<Routes>
<Route element={<LibraryLayoutWrapper />}>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the wrapper using proper route nesting instead of a function

{[
ROUTES.HOME,
ROUTES.COMPONENT,
ROUTES.COMPONENTS,
ROUTES.COLLECTIONS,
ROUTES.UNITS,
].map((route) => (
<Route
key={route}
path={route}
element={context(<LibraryAuthoringPage />)}
Component={LibraryAuthoringPage}
/>
))}
<Route
path={ROUTES.COLLECTION}
element={context(<LibraryCollectionPage />)}
Component={LibraryCollectionPage}
/>
<Route
path={ROUTES.UNIT}
element={context(<LibraryUnitPage />)}
Component={LibraryUnitPage}
/>
</Routes>
);
};
</Route>
</Routes>
);

export default LibraryLayout;
8 changes: 7 additions & 1 deletion src/library-authoring/__mocks__/library-search.json
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,13 @@
"context_key": "lib:Axim:TEST",
"org": "Axim",
"access_id": "15",
"num_children": "0"
"num_children": "0",
"published": {
"display_name": "Test Unit"
}
},
"published": {
"display_name": "Test Unit"
}
}
],
Expand Down
3 changes: 1 addition & 2 deletions src/library-authoring/collections/CollectionInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ const CollectionInfo = () => {
if (componentPickerMode) {
setCollectionId(collectionId);
} else {
/* istanbul ignore next */
navigateTo({ collectionId, doubleClicked: true });
navigateTo({ collectionId });
}
}, [componentPickerMode, navigateTo]);

Expand Down
9 changes: 2 additions & 7 deletions src/library-authoring/collections/LibraryCollectionPage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { useEffect } from 'react';
import { StudioFooterSlot } from '@edx/frontend-component-footer';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
Expand Down Expand Up @@ -110,9 +109,9 @@ const LibraryCollectionPage = () => {

const { componentPickerMode } = useComponentPickerContext();
const {
showOnlyPublished, extraFilter: contextExtraFilter, setCollectionId, componentId,
showOnlyPublished, extraFilter: contextExtraFilter, setCollectionId,
} = useLibraryContext();
const { sidebarComponentInfo, openInfoSidebar } = useSidebarContext();
const { sidebarComponentInfo } = useSidebarContext();

const {
data: collectionData,
Expand All @@ -121,10 +120,6 @@ const LibraryCollectionPage = () => {
error,
} = useCollection(libraryId, collectionId);

useEffect(() => {
openInfoSidebar(componentId, collectionId, '');
}, []);

const { data: libraryData, isLoading: isLibLoading } = useContentLibrary(libraryId);

// Only show loading if collection data is not fetched from index yet
Expand Down
Loading