fix: Multiple UI/UX improvements (#2529)

This includes multiple improvements described in #2528

Author: ChrisChV

src/course-unit/preview-changes/index.tsx

@@ -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>

src/course-unit/preview-changes/messages.ts

@@ -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',
   },
 });

src/legacy-libraries-migration/LegacyLibMigrationPage.test.tsx

@@ -82,13 +82,23 @@ describe('<LegacyLibMigrationPage />', () => {
   });
 
   it('should select legacy libraries', async () => {
+    const user = userEvent.setup();
     renderPage();
     expect(await screen.findByText('Migrate Legacy Libraries')).toBeInTheDocument();
 
     const nextButton = screen.getByRole('button', { name: /next/i });
     // 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' });

src/legacy-libraries-migration/LegacyLibMigrationPage.tsx

@@ -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}

src/legacy-libraries-migration/SelectDestinationView.tsx

@@ -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;
 }

src/legacy-libraries-migration/index.scss

@@ -1,20 +1,31 @@
 .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;
     }
 
     .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 {
     .pgn__card-header-content {
       margin-top: calc(var(--pgn-spacing-spacer-base)) !important;;
     }
   }
+
+  .content-buttons {
+    width: 100%;
+    position: fixed;
+    bottom: 0;
+  }
 }

src/library-authoring/component-comparison/CompareChangesWidget.tsx

@@ -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}