fix: video gallery design fixes (#407)

src/editors/containers/VideoGallery/index.test.jsx

@@ -124,17 +124,17 @@ describe('VideoGallery', () => {
       expect(window.location.assign).toHaveBeenCalled();
     });
     it.each([
-      [/by date added \(newest\)/i, [2, 1, 3]],
-      [/by date added \(oldest\)/i, [3, 1, 2]],
-      [/by name \(ascending\)/i, [1, 2, 3]],
-      [/by name \(descending\)/i, [3, 2, 1]],
-      [/by duration \(longest\)/i, [3, 1, 2]],
-      [/by duration \(shortest\)/i, [2, 1, 3]],
+      [/newest/i, [2, 1, 3]],
+      [/oldest/i, [3, 1, 2]],
+      [/name A-Z/i, [1, 2, 3]],
+      [/name Z-A/i, [3, 2, 1]],
+      [/longest/i, [3, 1, 2]],
+      [/shortest/i, [2, 1, 3]],
     ])('videos can be sorted %s', async (sortBy, order) => {
       await renderComponent();
 
       fireEvent.click(screen.getByRole('button', {
-        name: /by date added \(newest\)/i,
+        name: /By newest/i,
       }));
       fireEvent.click(screen.getByRole('link', {
         name: sortBy,

src/editors/containers/VideoGallery/messages.js

@@ -25,32 +25,32 @@ export const messages = {
   // Sort Dropdown
   sortByDateNewest: {
     id: 'authoring.selectvideomodal.sort.datenewest.label',
-    defaultMessage: 'By date added (newest)',
+    defaultMessage: 'newest',
     description: 'Dropdown label for sorting by date (newest)',
   },
   sortByDateOldest: {
     id: 'authoring.selectvideomodal.sort.dateoldest.label',
-    defaultMessage: 'By date added (oldest)',
+    defaultMessage: 'oldest',
     description: 'Dropdown label for sorting by date (oldest)',
   },
   sortByNameAscending: {
     id: 'authoring.selectvideomodal.sort.nameascending.label',
-    defaultMessage: 'By name (ascending)',
+    defaultMessage: 'name A-Z',
     description: 'Dropdown label for sorting by name (ascending)',
   },
   sortByNameDescending: {
     id: 'authoring.selectvideomodal.sort.namedescending.label',
-    defaultMessage: 'By name (descending)',
+    defaultMessage: 'name Z-A',
     description: 'Dropdown label for sorting by name (descending)',
   },
   sortByDurationShortest: {
     id: 'authoring.selectvideomodal.sort.durationshortest.label',
-    defaultMessage: 'By duration (shortest)',
+    defaultMessage: 'shortest',
     description: 'Dropdown label for sorting by duration (shortest)',
   },
   sortByDurationLongest: {
     id: 'authoring.selectvideomodal.sort.durationlongest.label',
-    defaultMessage: 'By duration (longest)',
+    defaultMessage: 'longest',
     description: 'Dropdown label for sorting by duration (longest)',
   },
 

src/editors/sharedComponents/BaseModal/index.jsx

@@ -20,6 +20,7 @@ export const BaseModal = ({
   size,
   isFullscreenScroll,
   bodyStyle,
+  className,
 }) => (
   <ModalDialog
     isOpen={isOpen}
@@ -30,6 +31,7 @@ export const BaseModal = ({
     isFullscreenOnMobile
     isFullscreenScroll={isFullscreenScroll}
     title={title}
+    className={className}
   >
     <ModalDialog.Header style={{ zIndex: 1, boxShadow: '2px 2px 5px rgba(0, 0, 0, 0.3)' }}>
       <ModalDialog.Title>
@@ -59,6 +61,7 @@ BaseModal.defaultProps = {
   size: 'lg',
   isFullscreenScroll: true,
   bodyStyle: null,
+  className: undefined,
 };
 
 BaseModal.propTypes = {
@@ -72,6 +75,7 @@ BaseModal.propTypes = {
   size: PropTypes.string,
   isFullscreenScroll: PropTypes.bool,
   bodyStyle: PropTypes.shape({}),
+  className: PropTypes.string,
 };
 
 export default BaseModal;

src/editors/sharedComponents/SelectionModal/SearchSort.jsx

@@ -4,13 +4,14 @@ import PropTypes from 'prop-types';
 import {
   ActionRow, Dropdown, Form, Icon, IconButton, SelectMenu, MenuItem,
 } from '@edx/paragon';
-import { Close, Search } from '@edx/paragon/icons';
+import { Check, Close, Search } from '@edx/paragon/icons';
 import {
   FormattedMessage,
   useIntl,
 } from '@edx/frontend-platform/i18n';
 
 import messages from './messages';
+import './index.scss';
 import { sortKeys, sortMessages } from '../../containers/VideoGallery/utils';
 
 export const SearchSort = ({
@@ -55,9 +56,18 @@ export const SearchSort = ({
       </Form.Group>
 
       { !showSwitch && <ActionRow.Spacer /> }
-      <SelectMenu variant="link">
+      <SelectMenu variant="link" className="search-sort-menu">
         {Object.keys(sortKeys).map(key => (
-          <MenuItem key={key} onClick={onSortClick(key)} defaultSelected={key === sortBy}>
+          <MenuItem
+            key={key}
+            onClick={onSortClick(key)}
+            defaultSelected={key === sortBy}
+            iconAfter={(key === sortBy) ? Check : null}
+          >
+            <span className="search-sort-menu-by">
+              <FormattedMessage {...messages.sortBy} />
+              <span style={{ whiteSpace: 'pre-wrap' }}> </span>
+            </span>
             <FormattedMessage {...sortMessages[key]} />
           </MenuItem>
         ))}

src/editors/sharedComponents/SelectionModal/SearchSort.test.jsx

@@ -6,8 +6,9 @@ import {
 } from '@testing-library/react';
 import { IntlProvider } from '@edx/frontend-platform/i18n';
 
-import { sortKeys, sortMessages } from '../ImageUploadModal/SelectImageModal/utils';
-import { filterKeys, filterMessages } from '../../containers/VideoGallery/utils';
+import {
+  filterKeys, filterMessages, sortKeys, sortMessages,
+} from '../../containers/VideoGallery/utils';
 import { SearchSort } from './SearchSort';
 import messages from './messages';
 
@@ -51,23 +52,23 @@ describe('SearchSort component', () => {
     const { getByRole } = getComponent();
     await act(() => {
       fireEvent.click(screen.getByRole('button', {
-        name: /by date added \(oldest\)/i,
+        name: /By oldest/i,
       }));
     });
     Object.values(sortMessages)
       .forEach(({ defaultMessage }) => {
-        expect(getByRole('link', { name: defaultMessage }))
+        expect(getByRole('link', { name: `By ${defaultMessage}` }))
           .toBeInTheDocument();
       });
   });
   test('adds a sort option for each sortKey', async () => {
     const { getByRole } = getComponent();
     await act(() => {
-      fireEvent.click(screen.getByRole('button', { name: /by date added \(oldest\)/i }));
+      fireEvent.click(screen.getByRole('button', { name: /oldest/i }));
     });
     Object.values(sortMessages)
       .forEach(({ defaultMessage }) => {
-        expect(getByRole('link', { name: defaultMessage }))
+        expect(getByRole('link', { name: `By ${defaultMessage}` }))
           .toBeInTheDocument();
       });
   });

src/editors/sharedComponents/SelectionModal/index.jsx

@@ -16,6 +16,8 @@ import ErrorAlert from '../ErrorAlerts/ErrorAlert';
 import FetchErrorAlert from '../ErrorAlerts/FetchErrorAlert';
 import UploadErrorAlert from '../ErrorAlerts/UploadErrorAlert';
 
+import './index.scss';
+
 export const SelectionModal = ({
   isOpen,
   close,
@@ -85,27 +87,32 @@ export const SelectionModal = ({
           <SearchSort {...searchSortProps} />
         </div>
       )}
+      className="selection-modal"
     >
-      {/* Error Alerts */}
-      <FetchErrorAlert isFetchError={isFetchError} message={fetchError} />
-      <UploadErrorAlert isUploadError={isUploadError} message={uploadError} />
-      <ErrorAlert
-        dismissError={inputError.dismiss}
-        hideHeading
-        isError={inputError.show}
-      >
-        <FormattedMessage {...inputError.message} />
-      </ErrorAlert>
+      {/*
+        If the modal dialog content is zero height, it shows a bottom shadow
+        as if there was content to scroll to, so make the min-height 1px.
+      */}
+      <Stack gap={2} style={{ minHeight: '1px' }}>
+        {/* Error Alerts */}
+        <FetchErrorAlert isFetchError={isFetchError} message={fetchError} />
+        <UploadErrorAlert isUploadError={isUploadError} message={uploadError} />
+        <ErrorAlert
+          dismissError={inputError.dismiss}
+          hideHeading
+          isError={inputError.show}
+        >
+          <FormattedMessage {...inputError.message} />
+        </ErrorAlert>
 
-      {/* User Feedback Alerts */}
-      <ErrorAlert
-        dismissError={galleryError.dismiss}
-        hideHeading
-        isError={galleryError.show}
-      >
-        <FormattedMessage {...galleryError.message} />
-      </ErrorAlert>
-      <Stack gap={2}>
+        {/* User Feedback Alerts */}
+        <ErrorAlert
+          dismissError={galleryError.dismiss}
+          hideHeading
+          isError={galleryError.show}
+        >
+          <FormattedMessage {...galleryError.message} />
+        </ErrorAlert>
         {showGallery && <Gallery {...galleryPropsValues} />}
         <FileInput fileInput={fileInput} acceptedFiles={Object.values(acceptedFiles).join()} />
       </Stack>

src/editors/sharedComponents/SelectionModal/index.scss

@@ -0,0 +1,32 @@
+.search-sort-menu .pgn__menu-item-text {
+    text-transform: capitalize;
+}
+
+.search-sort-menu .pgn__menu .search-sort-menu-by {
+    display: none;
+}
+
+/* Sort options come in pairs of ascending and descending. */
+.search-sort-menu .pgn__menu > div:nth-child(even) {
+    border-bottom: 1px solid #f4f3f6;
+}
+
+.search-sort-menu .pgn__menu > div:last-child {
+    border-bottom: none;
+}
+
+.selection-modal .pgn__vstack > .alert {
+    margin-bottom: 0;
+    margin-top: 1.5rem;
+}
+
+/* Set top padding to 0 when there is an alert above. */
+.selection-modal .pgn__vstack > .alert + .gallery > .pgn__scrollable-body-content > .p-4 {
+    padding-top: 0 !important;
+}
+
+.selection-modal .pgn__vstack > .alert > .alert-icon {
+    /* Vertical margin equal to the vertical padding of the "Dismiss" button. */
+    margin-bottom: 0.4375rem;
+    margin-top: 0.4375rem;
+}

src/editors/sharedComponents/SelectionModal/messages.js

@@ -24,6 +24,11 @@ export const messages = {
     defaultMessage: 'Added {date} at {time}',
     description: 'File date-added string',
   },
+  sortBy: {
+    id: 'authoring.selectionmodal.sortBy',
+    defaultMessage: 'By',
+    description: '"By" before sort option',
+  },
 };
 
 export default messages;