Skip to content
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
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
16 changes: 14 additions & 2 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { TaxonomyLayout, TaxonomyDetailPage, TaxonomyListPage } from './taxonomy
import { ContentTagsDrawer } from './content-tags-drawer';
import AccessibilityPage from './accessibility-page';
import { ToastProvider } from './generic/toast-context';
import { ContentType } from './library-authoring/routes';

import 'react-datepicker/dist/react-datepicker.css';
import './index.scss';
Expand Down Expand Up @@ -68,11 +69,22 @@ 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"']}
visibleTabs={[ContentType.home, ContentType.components, ContentType.collections]}
/>
)}
/>
<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"']}
visibleTabs={[ContentType.home, ContentType.components, ContentType.collections]}
/>
)}
/>
<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
66 changes: 36 additions & 30 deletions src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
Tabs,
} from '@openedx/paragon';
import { Add, ArrowBack, InfoOutline } from '@openedx/paragon/icons';
import { Link } from 'react-router-dom';
import { Link, useLocation } from 'react-router-dom';

import Loading from '../generic/Loading';
import SubHeader from '../generic/sub-header/SubHeader';
Expand Down Expand Up @@ -72,8 +72,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 @@ -137,6 +137,7 @@ const LibraryAuthoringPage = ({
visibleTabs = allLibraryPageTabs,
}: LibraryAuthoringPageProps) => {
const intl = useIntl();
const location = useLocation();

const {
isLoadingPage: isLoadingStudioHome,
Expand All @@ -151,16 +152,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,15 +178,31 @@ 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);
// Update the active key whenever the route changes. This ensures that the correct tab is selected
// when navigating using the browser's back/forward buttons because it does not trigger a re-render.
setActiveKey(getActiveKey());
}
}, []);
}, [location.key, getActiveKey]);

const handleTabChange = useCallback((key: ContentType) => {
setActiveKey(key);
if (!componentPickerMode) {
navigateTo({ contentType: key });
}
}, [navigateTo]);

if (isLoadingLibraryData) {
return <Loading />;
Expand All @@ -204,13 +220,6 @@ const LibraryAuthoringPage = ({
return <NotFoundAlert />;
}

const handleTabChange = (key: ContentType) => {
setActiveKey(key);
if (!componentPickerMode) {
navigateTo({ contentType: key });
}
};

const breadcumbs = componentPickerMode && !restrictToLibrary ? (
<Breadcrumb
links={[
Expand Down Expand Up @@ -241,31 +250,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 +315,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
62 changes: 28 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,65 @@ 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,
ROUTES.SECTIONS,
ROUTES.SUBSECTIONS,
].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
Loading