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.

Sign up for GitHub

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

feat: improve library sub header #1573

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: improve library sub header #1573

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db409cd
feat: improve library sub header
rpenido Dec 16, 2024
3275c7c
fix: spacing between search and filters
rpenido Dec 17, 2024
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
6 changes: 3 additions & 3 deletions src/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ export const useLoadOnScroll = (
useEffect(() => {
if (enabled) {
const canFetchNextPage = hasNextPage && !isFetchingNextPage;
// Used `loadLimit` to fetch next page before reach the end of the screen.
const loadLimit = 300;

const onscroll = () => {
// Verify the position of the scroll to implement an infinite scroll.
// Used `loadLimit` to fetch next page before reach the end of the screen.
const loadLimit = 300;
const scrolledTo = window.scrollY + window.innerHeight;
const scrollDiff = document.body.scrollHeight - scrolledTo;
const isNearToBottom = scrollDiff <= loadLimit;
Expand All @@ -65,7 +65,7 @@ export const useLoadOnScroll = (
window.addEventListener('scroll', onscroll);

// If the content is less than the screen height, fetch the next page.
const hasNoScroll = document.body.scrollHeight <= window.innerHeight;
const hasNoScroll = (document.body.scrollHeight - loadLimit) <= window.innerHeight;
Copy link
Contributor Author

@rpenido rpenido Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This was a small bug that if the last line was half-filled, it was not calling a new fetch

if (hasNoScroll && canFetchNextPage) {
fetchNextPage();
}
Expand Down
39 changes: 17 additions & 22 deletions src/library-authoring/LibraryAuthoringPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import classNames from 'classnames';
import { StudioFooter } from '@edx/frontend-component-footer';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
ActionRow,
Alert,
Badge,
Breadcrumb,
Expand Down Expand Up @@ -57,12 +58,10 @@ const HeaderActions = () => {

const { componentPickerMode } = useComponentPickerContext();

const infoSidebarIsOpen = () => (
sidebarComponentInfo?.type === SidebarBodyComponentId.Info
);
const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.Info;

const handleOnClickInfoSidebar = () => {
if (infoSidebarIsOpen()) {
if (infoSidebarIsOpen) {
closeLibrarySidebar();
} else {
openInfoSidebar();
Expand All @@ -73,8 +72,8 @@ const HeaderActions = () => {
<div className="header-actions">
<Button
className={classNames('mr-1', {
'normal-border': !infoSidebarIsOpen(),
'open-border': infoSidebarIsOpen(),
'normal-border': !infoSidebarIsOpen,
'open-border': infoSidebarIsOpen,
})}
iconBefore={InfoOutline}
variant="outline-primary rounded-0"
Expand All @@ -97,7 +96,7 @@ const HeaderActions = () => {
);
};

const SubHeaderTitle = ({ title }: { title: string }) => {
export const SubHeaderTitle = ({ title }: { title: string }) => {
const intl = useIntl();

const { readOnly } = useLibraryContext();
Expand Down Expand Up @@ -143,15 +142,15 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
} = useLibraryContext();
const { openInfoSidebar, sidebarComponentInfo } = useSidebarContext();

const [activeKey, setActiveKey] = useState<ContentType | undefined>(ContentType.home);
const [activeKey, setActiveKey] = useState<ContentType>(ContentType.home);

useEffect(() => {
const currentPath = location.pathname.split('/').pop();

if (componentPickerMode || currentPath === libraryId || currentPath === '') {
setActiveKey(ContentType.home);
} else if (currentPath && currentPath in ContentType) {
setActiveKey(ContentType[currentPath]);
setActiveKey(ContentType[currentPath] || ContentType.home);
}
}, []);

Expand All @@ -175,11 +174,6 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
);
}

// istanbul ignore if: this should never happen
if (activeKey === undefined) {
return <NotFoundAlert />;
}

if (!libraryData) {
return <NotFoundAlert />;
}
Expand Down Expand Up @@ -249,15 +243,8 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
subtitle={!componentPickerMode ? intl.formatMessage(messages.headingSubtitle) : undefined}
breadcrumbs={breadcumbs}
headerActions={<HeaderActions />}
hideBorder
/>
<SearchKeywordsField className="w-50" />
<div className="d-flex mt-3 align-items-center">
<FilterByTags />
<FilterByBlockType />
<ClearFiltersButton />
<div className="flex-grow-1" />
<SearchSortWidget />
</div>
<Tabs
variant="tabs"
activeKey={activeKey}
Expand All @@ -268,6 +255,14 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
<Tab eventKey={ContentType.components} title={intl.formatMessage(messages.componentsTab)} />
<Tab eventKey={ContentType.collections} title={intl.formatMessage(messages.collectionsTab)} />
</Tabs>
<ActionRow className="my-3">
<SearchKeywordsField />
rpenido marked this conversation as resolved.
Show resolved Hide resolved
<FilterByTags />
<FilterByBlockType />
<ClearFiltersButton />
<ActionRow.Spacer />
<SearchSortWidget />
</ActionRow>
<LibraryContent contentType={activeKey} />
</SearchContextProvider>
</Container>
Expand Down
15 changes: 8 additions & 7 deletions src/library-authoring/collections/LibraryCollectionPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('<LibraryCollectionPage />', () => {
expect(screen.queryByText('Read Only')).not.toBeInTheDocument();
});

it('shows an empty read-only library collection, without a new button', async () => {
it('shows an empty read-only library collection, with the new button disabled', async () => {
// Use a library mock that is read-only:
const libraryId = mockContentLibrary.libraryIdReadOnly;
// Update search mock so it returns no results:
Expand All @@ -161,7 +161,8 @@ describe('<LibraryCollectionPage />', () => {
// Show in the collection page and in the sidebar
expect(screen.getAllByText('This collection is currently empty.').length).toEqual(2);

expect(screen.queryByRole('button', { name: /new/i })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: /new/i })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: /new/i })).toBeDisabled();
expect(screen.getByText('Read Only')).toBeInTheDocument();
});

Expand Down Expand Up @@ -230,14 +231,14 @@ describe('<LibraryCollectionPage />', () => {
expect((await screen.findAllByText(title))[0]).toBeInTheDocument();
expect((await screen.findAllByText(title))[1]).toBeInTheDocument();

// Open by default; close the library info sidebar
const closeButton = screen.getByRole('button', { name: /close/i });
fireEvent.click(closeButton);
const collectionInfoBtn = screen.getByRole('button', { name: /collection info/i });

// Open by default; click 'Collection info' button to close
fireEvent.click(collectionInfoBtn);
expect(screen.queryByText('Draft')).not.toBeInTheDocument();
expect(screen.queryByText('(Never Published)')).not.toBeInTheDocument();

// Open library info sidebar with 'Library info' button
const collectionInfoBtn = screen.getByRole('button', { name: /collection info/i });
// Open library info sidebar with 'Collection info' button
fireEvent.click(collectionInfoBtn);
expect(screen.getByText('Manage')).toBeInTheDocument();
expect(screen.getByText('Details')).toBeInTheDocument();
Expand Down
121 changes: 59 additions & 62 deletions src/library-authoring/collections/LibraryCollectionPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import { useEffect } from 'react';
import { StudioFooter } from '@edx/frontend-component-footer';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
Badge,
ActionRow,
Button,
Breadcrumb,
Container,
Icon,
IconButton,
Stack,
} from '@openedx/paragon';
import { Add, ArrowBack, InfoOutline } from '@openedx/paragon/icons';
import classNames from 'classnames';
import { Helmet } from 'react-helmet';
import { Link } from 'react-router-dom';

import Loading from '../../generic/Loading';
Expand All @@ -26,71 +26,68 @@ import {
SearchKeywordsField,
SearchSortWidget,
} from '../../search-manager';
import { SubHeaderTitle } from '../LibraryAuthoringPage';
import { useCollection, useContentLibrary } from '../data/apiHooks';
import { useComponentPickerContext } from '../common/context/ComponentPickerContext';
import { useLibraryContext } from '../common/context/LibraryContext';
import { useSidebarContext } from '../common/context/SidebarContext';
import { SidebarBodyComponentId, useSidebarContext } from '../common/context/SidebarContext';
import messages from './messages';
import { LibrarySidebar } from '../library-sidebar';
import LibraryCollectionComponents from './LibraryCollectionComponents';

const HeaderActions = () => {
const intl = useIntl();
const { readOnly } = useLibraryContext();
const { openAddContentSidebar } = useSidebarContext();

if (readOnly) {
return null;
const { componentPickerMode } = useComponentPickerContext();
const { collectionId, readOnly } = useLibraryContext();
const {
closeLibrarySidebar,
openAddContentSidebar,
openCollectionInfoSidebar,
sidebarComponentInfo,
} = useSidebarContext();

// istanbul ignore if: this should never happen
if (!collectionId) {
throw new Error('it should not be possible to render HeaderActions without a collectionId');
}

const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.CollectionInfo
&& sidebarComponentInfo?.id === collectionId;

const handleOnClickInfoSidebar = () => {
if (infoSidebarIsOpen) {
closeLibrarySidebar();
} else {
openCollectionInfoSidebar(collectionId);
}
};

return (
<div className="header-actions">
<Button
className="ml-1"
iconBefore={Add}
variant="primary rounded-0"
onClick={openAddContentSidebar}
className={classNames('mr-1', {
'normal-border': !infoSidebarIsOpen,
'open-border': infoSidebarIsOpen,
})}
iconBefore={InfoOutline}
variant="outline-primary rounded-0"
onClick={handleOnClickInfoSidebar}
>
{intl.formatMessage(messages.newContentButton)}
{intl.formatMessage(messages.collectionInfoButton)}
</Button>
</div>
);
};

const SubHeaderTitle = ({
title,
infoClickHandler,
}: {
title: string;
infoClickHandler: () => void;
}) => {
const intl = useIntl();

const { componentPickerMode } = useComponentPickerContext();
const { readOnly } = useLibraryContext();

const showReadOnlyBadge = readOnly && !componentPickerMode;

return (
<Stack direction="vertical">
<Stack direction="horizontal" gap={2}>
{title}
<IconButton
src={InfoOutline}
iconAs={Icon}
alt={intl.formatMessage(messages.collectionInfoButton)}
onClick={infoClickHandler}
variant="primary"
/>
</Stack>
{showReadOnlyBadge && (
<div>
<Badge variant="primary" style={{ fontSize: '50%' }}>
{intl.formatMessage(messages.readOnlyBadge)}
</Badge>
</div>
{!componentPickerMode && (
<Button
className="ml-1"
iconBefore={Add}
variant="primary rounded-0"
onClick={openAddContentSidebar}
disabled={readOnly}
>
{intl.formatMessage(messages.newContentButton)}
</Button>
)}
</Stack>
</div>
);
};

Expand Down Expand Up @@ -181,41 +178,41 @@ const LibraryCollectionPage = () => {
return (
<div className="d-flex">
<div className="flex-grow-1">
<Helmet><title>{libraryData.title} | {process.env.SITE_NAME}</title></Helmet>
{!componentPickerMode && (
<Header
number={libraryData.slug}
title={libraryData.title}
org={libraryData.org}
contextId={libraryId}
isLibrary
containerProps={{
size: undefined,
}}
/>
)}
<Container size="xl" className="px-4 mt-4 mb-5 library-authoring-page">
<Container className="px-4 mt-4 mb-5 library-authoring-page">
<SearchContextProvider
extraFilter={extraFilter}
>
<SubHeader
title={(
<SubHeaderTitle
title={collectionData.title}
infoClickHandler={() => openCollectionInfoSidebar(collectionId)}
/>
)}
title={<SubHeaderTitle title={collectionData.title} />}
breadcrumbs={breadcumbs}
headerActions={<HeaderActions />}
hideBorder
/>
<SearchKeywordsField className="w-50" placeholder={intl.formatMessage(messages.searchPlaceholder)} />
<div className="d-flex mt-3 mb-4 align-items-center">
<ActionRow className="my-3">
<SearchKeywordsField />
rpenido marked this conversation as resolved.
Show resolved Hide resolved
<FilterByTags />
<FilterByBlockType />
<ClearFiltersButton />
<div className="flex-grow-1" />
<ActionRow.Spacer />
<SearchSortWidget />
</div>
</ActionRow>
<LibraryCollectionComponents />
</SearchContextProvider>
</Container>
<StudioFooter />
{!componentPickerMode && <StudioFooter containerProps={{ size: undefined }} />}
</div>
{!!sidebarComponentInfo?.type && (
<div className="library-authoring-sidebar box-shadow-left-1 bg-white" data-testid="library-sidebar">
Expand Down
10 changes: 0 additions & 10 deletions src/library-authoring/collections/messages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,6 @@ const messages = defineMessages({
defaultMessage: 'Collection Info',
description: 'Alt text for collection info button besides the collection title',
},
readOnlyBadge: {
id: 'course-authoring.library-authoring.collections.badge.read-only',
defaultMessage: 'Read Only',
description: 'Text in badge when the user has read only access in collections page',
},
allCollections: {
id: 'course-authoring.library-authoring.all-collections.text',
defaultMessage: 'All Collections',
Expand All @@ -91,11 +86,6 @@ const messages = defineMessages({
defaultMessage: 'Navigation breadcrumbs',
description: 'Aria label for navigation breadcrumbs',
},
searchPlaceholder: {
id: 'course-authoring.library-authoring.search.placeholder.text',
defaultMessage: 'Search Collection',
description: 'Search placeholder text in collections page.',
},
noSearchResultsCollections: {
id: 'course-authoring.library-authoring.no-search-results-collections',
defaultMessage: 'No matching collections found in this library.',
Expand Down
Loading
Loading