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

feat: collections page [FC-0062] #1281

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions src/editors/EditorContainer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ jest.mock('react-router', () => ({
blockId: 'company-id1',
blockType: 'html',
}),
useLocation: () => {},
}));

const props = { learningContextId: 'cOuRsEId' };
Expand Down
12 changes: 7 additions & 5 deletions src/editors/EditorContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { useParams } from 'react-router-dom';
import { useLocation, useParams } from 'react-router-dom';
import { getConfig } from '@edx/frontend-platform';

import EditorPage from './EditorPage';
Expand All @@ -8,7 +8,7 @@ interface Props {
/** Course ID or Library ID */
learningContextId: string;
/** Event handler sometimes called when user cancels out of the editor page */
onClose?: () => void;
onClose?: (prevPath?: string) => void;
/**
* Event handler called after when user saves their changes using an editor
* and sometimes called when user cancels the editor, instead of onClose.
Expand All @@ -17,7 +17,7 @@ interface Props {
* TODO: clean this up so there are separate onCancel and onSave callbacks,
* and they are used consistently instead of this mess.
*/
returnFunction?: () => (newData: Record<string, any> | undefined) => void;
returnFunction?: (prevPath?: string) => (newData: Record<string, any> | undefined) => void;
}

const EditorContainer: React.FC<Props> = ({
Expand All @@ -26,6 +26,8 @@ const EditorContainer: React.FC<Props> = ({
returnFunction,
}) => {
const { blockType, blockId } = useParams();
const location = useLocation();

if (blockType === undefined || blockId === undefined) {
// istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker.
return <div>Error: missing URL parameters</div>;
Expand All @@ -38,8 +40,8 @@ const EditorContainer: React.FC<Props> = ({
blockId={blockId}
studioEndpointUrl={getConfig().STUDIO_BASE_URL}
lmsEndpointUrl={getConfig().LMS_BASE_URL}
onClose={onClose}
returnFunction={returnFunction}
onClose={onClose ? () => onClose(location.state?.from) : null}
returnFunction={returnFunction ? () => returnFunction(location.state?.from) : null}
Copy link
Contributor

Choose a reason for hiding this comment

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

See my other comment - I don't think it's necessary to pass location.state?.from here.

Though it may actually solve @rpenido's concern about not going back to a different website.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bradenmacdonald Yes, my first solution was to use navigate(-1) but it goes back to other website if we directly use the URL and has no way of checking history before moving back, so I used this option of passing state where we can be sure that it goes back to libraries even if the user directly jumps to the url.

/>
</div>
);
Expand Down
36 changes: 36 additions & 0 deletions src/generic/block-type-utils/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
.pgn__icon {
color: white;
}

.btn-icon {
&:hover, &:active, &:focus {
background-color: darken(#005C9E, 15%);
}
}
}

.component-style-html {
Expand All @@ -12,6 +18,12 @@
.pgn__icon {
color: white;
}

.btn-icon {
&:hover, &:active, &:focus {
background-color: darken(#9747FF, 15%);
}
}
}

.component-style-collection {
Expand All @@ -20,6 +32,12 @@
.pgn__icon {
color: black;
}

.btn-icon {
&:hover, &:active, &:focus {
background-color: darken(#FFCD29, 15%);
}
}
navinkarkera marked this conversation as resolved.
Show resolved Hide resolved
}

.component-style-video {
Expand All @@ -28,6 +46,12 @@
.pgn__icon {
color: white;
}

.btn-icon {
&:hover, &:active, &:focus {
background-color: darken(#358F0A, 15%);
}
}
}

.component-style-vertical {
Expand All @@ -36,6 +60,12 @@
.pgn__icon {
color: white;
}

.btn-icon {
&:hover, &:active, &:focus {
background-color: darken(#0B8E77, 15%);
}
}
}

.component-style-other {
Expand All @@ -44,4 +74,10 @@
.pgn__icon {
color: white;
}

.btn-icon {
&:hover, &:active, &:focus {
background-color: darken(#646464, 15%);
}
}
}
46 changes: 19 additions & 27 deletions src/library-authoring/EmptyStates.tsx
Original file line number Diff line number Diff line change
@@ -1,54 +1,46 @@
import React, { useContext, useCallback } from 'react';
import { useParams } from 'react-router';
import { FormattedMessage } from '@edx/frontend-platform/i18n';
import type { MessageDescriptor } from 'react-intl';
import {
Button, Stack,
} from '@openedx/paragon';
import { Add } from '@openedx/paragon/icons';
import { ClearFiltersButton } from '../search-manager';
import messages from './messages';
import { LibraryContext } from './common/context';
import { useContentLibrary } from './data/apiHooks';

type NoSearchResultsProps = {
searchType?: 'collection' | 'component',
};

export const NoComponents = ({ searchType = 'component' }: NoSearchResultsProps) => {
const { openAddContentSidebar, openCreateCollectionModal } = useContext(LibraryContext);
export const NoComponents = ({
infoText = messages.noComponents,
addBtnText = messages.addComponent,
handleBtnClick,
}: {
infoText?: MessageDescriptor;
addBtnText?: MessageDescriptor;
handleBtnClick: () => void;
}) => {
const { libraryId } = useParams();
const { data: libraryData } = useContentLibrary(libraryId);
const canEditLibrary = libraryData?.canEditLibrary ?? false;

const handleOnClickButton = useCallback(() => {
if (searchType === 'collection') {
openCreateCollectionModal();
} else {
openAddContentSidebar();
}
}, [searchType]);

return (
<Stack direction="horizontal" gap={3} className="mt-6 justify-content-center">
{searchType === 'collection'
? <FormattedMessage {...messages.noCollections} />
: <FormattedMessage {...messages.noComponents} />}
<FormattedMessage {...infoText} />
{canEditLibrary && (
<Button iconBefore={Add} onClick={handleOnClickButton}>
{searchType === 'collection'
? <FormattedMessage {...messages.addCollection} />
: <FormattedMessage {...messages.addComponent} />}
<Button iconBefore={Add} onClick={handleBtnClick}>
<FormattedMessage {...addBtnText} />
</Button>
)}
</Stack>
);
};

export const NoSearchResults = ({ searchType = 'component' }: NoSearchResultsProps) => (
export const NoSearchResults = ({
infoText = messages.noSearchResults,
}: {
infoText?: MessageDescriptor;
}) => (
<Stack direction="horizontal" gap={3} className="my-6 justify-content-center">
{searchType === 'collection'
? <FormattedMessage {...messages.noSearchResultsCollections} />
: <FormattedMessage {...messages.noSearchResults} />}
<FormattedMessage {...infoText} />
<ClearFiltersButton variant="primary" size="md" />
</Stack>
);
5 changes: 5 additions & 0 deletions src/library-authoring/LibraryAuthoringPage.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@
height: 100vh;
overflow-y: auto;
}

// Reduce breadcrumb bottom margin
ol.list-inline {
margin-bottom: 0;
}
2 changes: 1 addition & 1 deletion src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
SearchSortWidget,
} from '../search-manager';
import LibraryComponents from './components/LibraryComponents';
import LibraryCollections from './LibraryCollections';
import LibraryCollections from './collections/LibraryCollections';
import LibraryHome from './LibraryHome';
import { useContentLibrary } from './data/apiHooks';
import { LibrarySidebar } from './library-sidebar';
Expand Down
8 changes: 5 additions & 3 deletions src/library-authoring/LibraryHome.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import React from 'react';
import React, { useContext } from 'react';
import { Stack } from '@openedx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';

import { useSearchContext } from '../search-manager';
import { NoComponents, NoSearchResults } from './EmptyStates';
import LibraryCollections from './LibraryCollections';
import LibraryCollections from './collections/LibraryCollections';
import { LibraryComponents } from './components';
import LibrarySection from './components/LibrarySection';
import LibraryRecentlyModified from './LibraryRecentlyModified';
import messages from './messages';
import { LibraryContext } from './common/context';

type LibraryHomeProps = {
libraryId: string,
Expand All @@ -23,10 +24,11 @@ const LibraryHome = ({ libraryId, tabList, handleTabChange } : LibraryHomeProps)
totalCollectionHits: collectionCount,
isFiltered,
} = useSearchContext();
const { openAddContentSidebar } = useContext(LibraryContext);

const renderEmptyState = () => {
if (componentCount === 0 && collectionCount === 0) {
return isFiltered ? <NoSearchResults /> : <NoComponents />;
return isFiltered ? <NoSearchResults /> : <NoComponents handleBtnClick={openAddContentSidebar} />;
}
return null;
};
Expand Down
21 changes: 16 additions & 5 deletions src/library-authoring/LibraryLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import { LibraryProvider } from './common/context';
import { CreateCollectionModal } from './create-collection';
import { invalidateComponentData } from './data/apiHooks';
import LibraryCollectionPage from './collections/LibraryCollectionPage';

const LibraryLayout = () => {
const { libraryId } = useParams();
Expand All @@ -24,14 +25,20 @@
}

const navigate = useNavigate();
const goBack = React.useCallback(() => {
// Go back to the library
navigate(`/library/${libraryId}`);
const goBack = React.useCallback((prevPath?: string) => {
if (prevPath) {
// Redirects back to the previous route like collection page or library page
navigate(prevPath);
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
navigate(prevPath);
navigate(-1);

If you want to go back, it's not necessary to pass prevPath through all these callbacks like you've done here. You can just call navigate(-1). I actually had this in an earlier PR, but @rpenido suggested changing it.

For now I don't really care either way because what we really need to do is refactor this to display the editor as a modal, instead of navigating to a new URL for the editor and then going back. Obviously that will have to come in a dedicated PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my other comment.

} else {

Check warning on line 32 in src/library-authoring/LibraryLayout.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/LibraryLayout.tsx#L32

Added line #L32 was not covered by tests
// Go back to the library
navigate(`/library/${libraryId}`);

Check warning on line 34 in src/library-authoring/LibraryLayout.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/LibraryLayout.tsx#L34

Added line #L34 was not covered by tests
}
}, []);
const returnFunction = React.useCallback(() => {

const returnFunction = React.useCallback((prevPath?: string) => {
// When changes are cancelled, either onClose (goBack) or this returnFunction will be called.
// When changes are saved, this returnFunction is called.
goBack();
goBack(prevPath);
return (args) => {
if (args === undefined) {
return; // Do nothing - the user cancelled the changes
Expand All @@ -58,6 +65,10 @@
</PageWrap>
)}
/>
<Route
path="collection/:collectionId"
element={<LibraryCollectionPage />}
/>
<Route
path="*"
element={<LibraryAuthoringPage />}
Expand Down
Loading