fix: Multiple UI/UX improvements (#2529)

This includes multiple improvements described in https://github.com/openedx/frontend-app-authoring/issues/2528
This commit is contained in:
Chris Chávez
2025-10-13 09:21:00 -05:00
committed by GitHub
parent 6c829b9421
commit 46d2465064
12 changed files with 227 additions and 115 deletions

View File

@@ -4,7 +4,7 @@ import {
import {
ActionRow, Button, Icon, ModalDialog, useToggle,
} from '@openedx/paragon';
import { Info, Warning } from '@openedx/paragon/icons';
import { Info } from '@openedx/paragon/icons';
import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n';
import { ToastContext } from '@src/generic/toast-context';
@@ -150,7 +150,7 @@ export const PreviewLibraryXBlockChanges = ({
return (
<CompareChangesWidget
usageKey={blockData.upstreamBlockId}
oldUsageKey={isTextWithLocalChanges ? blockData.downstreamBlockId : undefined}
oldUsageKey={blockData.downstreamBlockId}
oldTitle={isTextWithLocalChanges ? blockData.displayName : undefined}
oldVersion={blockData.upstreamBlockVersionSynced || 'published'}
newVersion="published"
@@ -235,21 +235,14 @@ export const PreviewLibraryXBlockChanges = ({
</ModalDialog.Title>
</ModalDialog.Header>
<ModalDialog.Body>
{isTextWithLocalChanges ? (
{isTextWithLocalChanges && (
<AlertMessage
show
variant="info"
icon={Info}
title={intl.formatMessage(messages.localEditsAlert)}
/>
) : (!blockData.isContainer && (
<AlertMessage
show
variant="warning"
icon={Warning}
title={intl.formatMessage(messages.olderVersionPreviewAlert)}
/>
))}
)}
{getBody()}
</ModalDialog.Body>
<ModalDialog.Footer>

View File

@@ -51,11 +51,6 @@ const messages = defineMessages({
defaultMessage: 'Ignore',
description: 'Preview changes confirmation dialog confirm button text when user clicks on ignore changes.',
},
olderVersionPreviewAlert: {
id: 'course-authoring.review-tab.preview.old-version-alert',
defaultMessage: 'The old version preview is the previous library version',
description: 'Alert message stating that older version in preview is of library block',
},
localEditsAlert: {
id: 'course-authoring.review-tab.preview.loal-edits-alert',
defaultMessage: 'This library content has local edits.',
@@ -73,7 +68,7 @@ const messages = defineMessages({
},
updateToPublishedLibraryContentBody: {
id: 'course-authoring.review-tab.preview.update-to-published.modal.body',
defaultMessage: 'Updating this block will discard local changes. Any eidts made within this course will be discarted, and cannot be recovered',
defaultMessage: 'Updating this block will discard local changes. Any edits made within this course will be discarded, and cannot be recovered',
description: 'Body of the modal to update a content to the published library content',
},
updateToPublishedLibraryContentConfirm: {
@@ -93,7 +88,7 @@ const messages = defineMessages({
},
keepCourseContentBody: {
id: 'course-authoring.review-tab.preview.keep-course-content.modal.body',
defaultMessage: 'This will keep the locally edited course content. if the component is published again in its library, you can choose to update to published library content',
defaultMessage: 'This will keep the locally edited course content. If the component is published again in its library, you can choose to update to published library content',
description: 'Body of the modal to keep the content of a course component',
},
});

View File

@@ -82,6 +82,7 @@ describe('<LegacyLibMigrationPage />', () => {
});
it('should select legacy libraries', async () => {
const user = userEvent.setup();
renderPage();
expect(await screen.findByText('Migrate Legacy Libraries')).toBeInTheDocument();
@@ -89,6 +90,15 @@ describe('<LegacyLibMigrationPage />', () => {
// The next button is disabled
expect(nextButton).toBeDisabled();
// The filter is Unmigrated by default
const filterButton = await screen.findByRole('button', { name: /unmigrated/i });
expect(filterButton).toBeInTheDocument();
// Clear filter to show all
await user.click(filterButton);
const clearButton = await screen.findByRole('button', { name: /clear filter/i });
await user.click(clearButton);
expect(await screen.findByText('MBA')).toBeInTheDocument();
expect(await screen.findByText('Legacy library 1')).toBeInTheDocument();
expect(await screen.findByText('MBA 1')).toBeInTheDocument();
@@ -116,6 +126,37 @@ describe('<LegacyLibMigrationPage />', () => {
expect(nextButton).not.toBeDisabled();
});
it('should select all legacy libraries', async () => {
const user = userEvent.setup();
renderPage();
expect(await screen.findByText('Migrate Legacy Libraries')).toBeInTheDocument();
// The filter is Unmigrated by default
const filterButton = await screen.findByRole('button', { name: /unmigrated/i });
expect(filterButton).toBeInTheDocument();
// Clear filter to show all
await user.click(filterButton);
const clearButton = await screen.findByRole('button', { name: /clear filter/i });
await user.click(clearButton);
const selectAll = screen.getByRole('checkbox', { name: /select all/i });
await user.click(selectAll);
const library1 = screen.getByRole('checkbox', { name: 'MBA' });
const library2 = screen.getByRole('checkbox', { name: /legacy library 1 imported library/i });
const library3 = screen.getByRole('checkbox', { name: 'MBA 1' });
expect(library1).toBeChecked();
expect(library2).toBeChecked();
expect(library3).toBeChecked();
await user.click(selectAll);
expect(library1).not.toBeChecked();
expect(library2).not.toBeChecked();
expect(library3).not.toBeChecked();
});
it('should back to select legacy libraries', async () => {
renderPage();
expect(await screen.findByText('Migrate Legacy Libraries')).toBeInTheDocument();
@@ -250,10 +291,18 @@ describe('<LegacyLibMigrationPage />', () => {
});
it('should confirm migration', async () => {
const user = userEvent.setup();
renderPage();
expect(await screen.findByText('Migrate Legacy Libraries')).toBeInTheDocument();
expect(await screen.findByText('MBA')).toBeInTheDocument();
// The filter is 'unmigrated' by default.
// Clear the filter to select all libraries
const filterButton = screen.getByRole('button', { name: /unmigrated/i });
await user.click(filterButton);
const clearButton = await screen.findByRole('button', { name: /clear filter/i });
await user.click(clearButton);
const legacyLibrary1 = screen.getByRole('checkbox', { name: 'MBA' });
const legacyLibrary2 = screen.getByRole('checkbox', { name: /legacy library 1 imported library/i });
const legacyLibrary3 = screen.getByRole('checkbox', { name: 'MBA 1' });

View File

@@ -16,7 +16,7 @@ import Header from '@src/header';
import SubHeader from '@src/generic/sub-header/SubHeader';
import type { ContentLibrary } from '@src/library-authoring/data/api';
import type { LibraryV1Data } from '@src/studio-home/data/api';
import LibrariesList from '@src/studio-home/tabs-section/libraries-tab';
import { Filter, LibrariesList } from '@src/studio-home/tabs-section/libraries-tab';
import messages from './messages';
import { SelectDestinationView } from './SelectDestinationView';
@@ -157,6 +157,8 @@ export const LegacyLibMigrationPage = () => {
selectedIds={legacyLibrariesIds}
handleCheck={handleUpdateLegacyLibraries}
hideMigationAlert
initialFilter={[Filter.unmigrated]}
setSelectedLibraries={setLegacyLibraries}
/>
</Stepper.Step>
<Stepper.Step
@@ -182,29 +184,30 @@ export const LegacyLibMigrationPage = () => {
</Stepper.Step>
</Stepper>
</div>
<div className="content-buttons d-flex justify-content-between">
<Button variant="outline-primary" onClick={handleBack}>
{currentStep === 'select-libraries'
? intl.formatMessage(messages.cancel)
: intl.formatMessage(messages.back)}
</Button>
{currentStep !== 'confirmation-view' ? (
<Button onClick={handleNext} disabled={isNextDisabled()}>
{intl.formatMessage(messages.next)}
</Button>
) : (
<StatefulButton
state={confirmationButtonState}
disabledStates={['pending']}
labels={{
default: intl.formatMessage(messages.confirm),
pending: intl.formatMessage(messages.confirm),
}}
onClick={handleNext}
/>
)}
</div>
</Container>
<div className="content-buttons d-flex justify-content-between pl-6 pr-6 bg-white">
<Button className="mt-2 mb-2" variant="outline-primary" onClick={handleBack}>
{currentStep === 'select-libraries'
? intl.formatMessage(messages.cancel)
: intl.formatMessage(messages.back)}
</Button>
{currentStep !== 'confirmation-view' ? (
<Button className="mt-2 mb-2" onClick={handleNext} disabled={isNextDisabled()}>
{intl.formatMessage(messages.next)}
</Button>
) : (
<StatefulButton
className="mt-2 mb-2"
state={confirmationButtonState}
disabledStates={['pending']}
labels={{
default: intl.formatMessage(messages.confirm),
pending: intl.formatMessage(messages.confirm),
}}
onClick={handleNext}
/>
)}
</div>
</div>
<ExitModal
isExitModalOpen={isExitModalOpen}

View File

@@ -7,7 +7,7 @@ import type { ContentLibrary } from '@src/library-authoring/data/api';
import messages from './messages';
interface SelectDestinationViewProps {
destinationId: string | undefined;
destinationId?: string;
setDestinationId: (library: ContentLibrary) => void;
legacyLibCount: number;
}

View File

@@ -1,8 +1,5 @@
.legacy-library-migration-page {
.migration-container {
// Calculate all the screen size subtracting the height of the header and top/bottom margins
min-height: calc(calc(100vh - 60px) - calc(var(--pgn-spacing-spacer-base) * 6));
.courses-tab-container {
min-height: auto;
}
@@ -10,6 +7,14 @@
.migration-content {
flex: 1;
}
.card-item {
margin: 0 0 16px !important;
&.selected {
box-shadow: 0 0 0 2px var(--pgn-color-primary-700);
}
}
}
.confirmation-view {
@@ -17,4 +22,10 @@
margin-top: calc(var(--pgn-spacing-spacer-base)) !important;;
}
}
.content-buttons {
width: 100%;
position: fixed;
bottom: 0;
}
}

View File

@@ -52,7 +52,7 @@ const CompareChangesWidget = ({
{oldTitle}
</div>
)}
<div style={hasLocalChanges ? { marginLeft: '-35px', marginTop: '-15px' } : {}}>
<div style={hasLocalChanges ? { marginLeft: '-35px', marginTop: '-8px' } : {}}>
<IframeProvider>
<LibraryBlock
usageKey={oldUsageKey || usageKey}

View File

@@ -1,4 +1,4 @@
import React, { useCallback } from 'react';
import React, { useCallback, useEffect, useRef } from 'react';
import { useSelector } from 'react-redux';
import {
Card,
@@ -16,6 +16,7 @@ import { Link } from 'react-router-dom';
import { useWaffleFlags } from '@src/data/apiHooks';
import { COURSE_CREATOR_STATES } from '@src/constants';
import { parseLibraryKey } from '@src/generic/key-utils';
import classNames from 'classnames';
import { getStudioHomeData } from '../data/selectors';
import messages from '../messages';
@@ -141,7 +142,9 @@ interface BaseProps {
migratedToTitle?: string;
migratedToCollectionKey?: string | null;
selectMode?: 'single' | 'multiple';
isSelected?: boolean;
itemId?: string;
scrollIntoView?: boolean;
}
type Props = BaseProps & (
@@ -167,6 +170,7 @@ const CardItem: React.FC<Props> = ({
isLibraries = false,
courseKey = '',
selectMode,
isSelected = false,
itemId = '',
path,
url,
@@ -174,6 +178,7 @@ const CardItem: React.FC<Props> = ({
migratedToKey,
migratedToTitle,
migratedToCollectionKey,
scrollIntoView = false,
}) => {
const intl = useIntl();
const {
@@ -182,6 +187,7 @@ const CardItem: React.FC<Props> = ({
rerunCreatorStatus,
} = useSelector(getStudioHomeData);
const waffleFlags = useWaffleFlags();
const cardRef = useRef<HTMLDivElement>(null);
const destinationUrl: string = path ?? (
waffleFlags.useNewCourseOutlinePage && !isLibraries
@@ -217,66 +223,78 @@ const CardItem: React.FC<Props> = ({
return libUrl;
};
useEffect(() => {
/* istanbul ignore next */
if (scrollIntoView && cardRef.current && 'scrollIntoView' in cardRef.current) {
cardRef.current.scrollIntoView({ behavior: 'smooth' });
}
}, [scrollIntoView]);
return (
<Card className="card-item">
<Card.Header
size="sm"
title={(
<CardTitle
readOnlyItem={readOnlyItem}
selectMode={selectMode}
destinationUrl={destinationUrl}
title={title}
itemId={itemId}
isMigrated={isMigrated}
migratedToTitle={migratedToTitle}
migratedToKey={migratedToKey}
/>
)}
subtitle={getSubtitle()}
actions={showActions && (
<Dropdown>
<Dropdown.Toggle
as={IconButton}
iconAs={MoreHoriz}
variant="primary"
aria-label={intl.formatMessage(messages.btnDropDownText)}
/>
<Dropdown.Menu>
{isShowRerunLink && (
<Dropdown.Item
as={Link}
to={rerunLink ?? ''}
>
{messages.btnReRunText.defaultMessage}
<div ref={cardRef} className="w-100">
<Card className={classNames('card-item', {
selected: isSelected,
})}
>
<Card.Header
size="sm"
title={(
<CardTitle
readOnlyItem={readOnlyItem}
selectMode={selectMode}
destinationUrl={destinationUrl}
title={title}
itemId={itemId}
isMigrated={isMigrated}
migratedToTitle={migratedToTitle}
migratedToKey={migratedToKey}
/>
)}
subtitle={getSubtitle()}
actions={showActions && (
<Dropdown>
<Dropdown.Toggle
as={IconButton}
iconAs={MoreHoriz}
variant="primary"
aria-label={intl.formatMessage(messages.btnDropDownText)}
/>
<Dropdown.Menu>
{isShowRerunLink && (
<Dropdown.Item
as={Link}
to={rerunLink ?? ''}
>
{messages.btnReRunText.defaultMessage}
</Dropdown.Item>
)}
<Dropdown.Item href={lmsLink}>
{intl.formatMessage(messages.viewLiveBtnText)}
</Dropdown.Item>
)}
<Dropdown.Item href={lmsLink}>
{intl.formatMessage(messages.viewLiveBtnText)}
</Dropdown.Item>
</Dropdown.Menu>
</Dropdown>
)}
/>
{isMigrated && migratedToKey
&& (
<Card.Status className="bg-white pt-0 text-gray-500">
<Stack direction="horizontal" gap={2}>
<Icon src={AccessTime} size="sm" className="mb-1" />
{intl.formatMessage(messages.libraryMigrationStatusText)}
<b>
<MakeLinkOrSpan
when={!readOnlyItem}
to={collectionLink()}
className="text-info-500"
>
{migratedToTitle}
</MakeLinkOrSpan>
</b>
</Stack>
</Card.Status>
)}
</Card>
</Dropdown.Menu>
</Dropdown>
)}
/>
{isMigrated && migratedToKey
&& (
<Card.Status className="bg-white pt-0 text-gray-500">
<Stack direction="horizontal" gap={2}>
<Icon src={AccessTime} size="sm" className="mb-1" />
{intl.formatMessage(messages.libraryMigrationStatusText)}
<b>
<MakeLinkOrSpan
when={!readOnlyItem}
to={collectionLink()}
className="text-info-500"
>
{migratedToTitle}
</MakeLinkOrSpan>
</b>
</Stack>
</Card.Status>
)}
</Card>
</div>
);
};

View File

@@ -14,7 +14,7 @@ import { useNavigate, useLocation } from 'react-router-dom';
import { RequestStatus } from '@src/data/constants';
import { getLoadingStatuses, getStudioHomeData } from '../data/selectors';
import messages from './messages';
import LibrariesList from './libraries-tab';
import { LibrariesList } from './libraries-tab';
import LibrariesV2List from './libraries-v2-tab/index';
import CoursesTab from './courses-tab';
import { WelcomeLibrariesV2Alert } from './libraries-v2-tab/WelcomeLibrariesV2Alert';

View File

@@ -1,5 +1,5 @@
import { useCallback, useState } from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
import {
ActionRow, Form, Icon, Menu, MenuItem, Pagination, Row, SearchField,
} from '@openedx/paragon';
@@ -19,9 +19,11 @@ import { MigrateLegacyLibrariesAlert } from './MigrateLegacyLibrariesAlert';
const CardList = ({
data,
inSelectMode,
selectedIds,
}: {
data: LibraryV1Data[],
inSelectMode: boolean,
selectedIds?: string[];
}) => (
// eslint-disable-next-line react/jsx-no-useless-fragment
<>
@@ -46,6 +48,7 @@ const CardList = ({
url={url}
itemId={libraryKey}
selectMode={inSelectMode ? 'multiple' : undefined}
isSelected={selectedIds?.includes(libraryKey)}
isMigrated={isMigrated}
migratedToKey={migratedToKey}
migratedToTitle={migratedToTitle}
@@ -62,7 +65,7 @@ function findInValues<T extends {}>(arr: T[] | undefined, searchValue: string) {
)));
}
enum Filter {
export enum Filter {
migrated = 'migrated',
unmigrated = 'unmigrated',
}
@@ -141,19 +144,23 @@ const MigrationFilter = ({ filters, setFilters }: MigrationFilterProps) => {
interface LibrariesListProps {
selectedIds?: string[];
handleCheck?: (library: LibraryV1Data, action: 'add' | 'remove') => void;
setSelectedLibraries?: (libraries: LibraryV1Data[]) => void;
hideMigationAlert?: boolean;
initialFilter?: Filter[];
}
const LibrariesList = ({
export const LibrariesList = ({
selectedIds,
handleCheck,
setSelectedLibraries,
hideMigationAlert = false,
initialFilter = BaseFilterState,
}: LibrariesListProps) => {
const intl = useIntl();
const { isPending, data, isError } = useLibrariesV1Data();
const [currentPage, setCurrentPage] = useState<number>(1);
const [search, setSearch] = useState<string>('');
const [migrationFilter, setMigrationFilter] = useState<Filter[]>(BaseFilterState);
const [migrationFilter, setMigrationFilter] = useState<Filter[]>(initialFilter);
let filteredData = findInValues(data?.libraries, search || '') || [];
if (migrationFilter.length === 1) {
@@ -165,6 +172,10 @@ const LibrariesList = ({
const currentPageData = filteredData.slice((currentPage - 1) * perPage, currentPage * perPage);
const inSelectMode = handleCheck !== undefined;
const allChecked = filteredData.every(value => selectedIds?.includes(value.libraryKey));
const someChecked = filteredData.some(value => selectedIds?.includes(value.libraryKey));
const checkboxIsIndeterminate = someChecked && !allChecked;
const handleChangeCheckboxSet = useCallback((event) => {
if (handleCheck) {
const libraryId = event.target.value;
@@ -179,6 +190,14 @@ const LibrariesList = ({
}
}, [handleCheck, currentPageData]);
const handleSelectAll = useCallback(() => {
if (checkboxIsIndeterminate || selectedIds?.length === 0) {
setSelectedLibraries?.(filteredData);
} else {
setSelectedLibraries?.([]);
}
}, [checkboxIsIndeterminate, selectedIds, filteredData]);
if (isPending) {
return (
<Row className="m-0 mt-4 justify-content-center">
@@ -206,6 +225,16 @@ const LibrariesList = ({
{!hideMigationAlert && getConfig().ENABLE_LEGACY_LIBRARY_MIGRATOR === 'true' && (<MigrateLegacyLibrariesAlert />)}
<div className="courses-tab">
<ActionRow className="my-3">
{inSelectMode && (
<Form.Checkbox
checked={allChecked}
isIndeterminate={checkboxIsIndeterminate}
onChange={handleSelectAll}
className="ml-0.5 mr-3"
>
<FormattedMessage {...messages.selectAll} />
</Form.Checkbox>
)}
<SearchField
// istanbul ignore next
onSubmit={() => {}}
@@ -235,6 +264,7 @@ const LibrariesList = ({
<CardList
data={currentPageData}
inSelectMode={inSelectMode}
selectedIds={selectedIds}
/>
</Form.CheckboxSet>
) : (
@@ -259,5 +289,3 @@ const LibrariesList = ({
</>
);
};
export default LibrariesList;

View File

@@ -24,19 +24,23 @@ import LibrariesV2Filters from './libraries-v2-filters';
interface CardListProps {
hasV2Libraries: boolean;
selectMode?: 'single' | 'multiple';
selectedLibraryId?: string;
isFiltered: boolean;
isLoading: boolean;
data: LibrariesV2Response;
handleClearFilters: () => void;
scrollIntoView?: boolean;
}
const CardList: React.FC<CardListProps> = ({
hasV2Libraries,
selectMode,
selectedLibraryId,
isFiltered,
isLoading,
data,
handleClearFilters,
scrollIntoView = false,
}) => {
if (hasV2Libraries) {
return (
@@ -53,7 +57,9 @@ const CardList: React.FC<CardListProps> = ({
number={slug}
path={`/library/${id}`}
selectMode={selectMode}
isSelected={selectedLibraryId === id}
itemId={id}
scrollIntoView={scrollIntoView && selectedLibraryId === id}
/>
))
}
@@ -96,6 +102,7 @@ const LibrariesV2List: React.FC<Props> = ({
const [currentPage, setCurrentPage] = useState(1);
const [filterParams, setFilterParams] = useState({});
const [isCreateLibraryOpen, openCreateLibrary, closeCreateLibrary] = useToggle(false);
const [scrollIntoCard, setScrollIntoCard] = useState(false);
const isFiltered = Object.keys(filterParams).length > 0;
const inSelectMode = handleSelect !== undefined;
@@ -119,17 +126,19 @@ const LibrariesV2List: React.FC<Props> = ({
if (handleSelect) {
handleSelect(library);
closeCreateLibrary();
setScrollIntoCard(true);
}
}, [handleSelect, closeCreateLibrary]);
}, [handleSelect, closeCreateLibrary, setScrollIntoCard]);
const handleOnChangeRadioSet = useCallback((libraryId: string) => {
setScrollIntoCard(false);
if (handleSelect && data) {
const library = data.results.find((item) => item.id === libraryId);
if (library) {
handleSelect(library);
}
}
}, [data, handleSelect]);
}, [data, handleSelect, setScrollIntoCard]);
if (isPending && !isFiltered) {
return (
@@ -194,16 +203,17 @@ const LibrariesV2List: React.FC<Props> = ({
<CardList
hasV2Libraries={hasV2Libraries}
selectMode={inSelectMode ? 'single' : undefined}
selectedLibraryId={selectedLibraryId}
isFiltered={isFiltered}
isLoading={isPending}
data={data!}
handleClearFilters={handleClearFilters}
scrollIntoView={scrollIntoCard}
/>
</Form.RadioSet>
) : (
<CardList
hasV2Libraries={hasV2Libraries}
selectMode={inSelectMode ? 'single' : undefined}
isFiltered={isFiltered}
isLoading={isPending}
data={data!}

View File

@@ -131,6 +131,11 @@ const messages = defineMessages({
defaultMessage: 'Libraries documentation',
description: 'Link text for the libraries documentation link.',
},
selectAll: {
id: 'studio-home.libraries.migrate.select-all',
defaultMessage: 'Select All',
description: 'Button to select all libraries when migrate legacy libraries.',
},
});
export default messages;