feat: PR fixes

openedx · Dec 16, 2022 · dc1d591 · dc1d591
1 parent d77cd5e
commit dc1d591
Show file tree

Hide file tree

Showing 16 changed files with 148 additions and 97 deletions.
diff --git a/src/components/ContentHighlights/ContentHighlightCardItem.jsx b/src/components/ContentHighlights/ContentHighlightCardItem.jsx
@@ -2,35 +2,42 @@ import React from 'react';
 import { Card, Hyperlink } from '@edx/paragon';
 import Truncate from 'react-truncate';
 import PropTypes from 'prop-types';
-import { getContentHighlightCardFooter } from './data/constants';
+import { getContentHighlightCardFooter } from './data/utils';
+import SkeletonContentCard from './SkeletonContentCard';
 
 const ContentHighlightCardItem = ({
   isLoading,
   title,
-  hyperlink,
+  href,
   contentType,
   partners,
   cardImageUrl,
   price,
 }) => {
+  if (isLoading) {
+    return (
+      <SkeletonContentCard />
+    );
+  }
   const cardInfo = {
-    cardTitle: (<Truncate lines={3} title={title}>{title}</Truncate>),
-    cardLogoAlt: partners?.length === 1 ? `${partners[0].name}'s logo` : undefined,
+    cardImgSrc: cardImageUrl,
     cardLogoSrc: partners?.length === 1 ? partners[0].logoImageUrl : undefined,
+    cardLogoAlt: partners?.length === 1 ? `${partners[0].name}'s logo` : undefined,
+    cardTitle: <Truncate lines={3} title={title}>{title}</Truncate>,
     cardSubtitle: partners?.map(p => p.name).join(', '),
-    cardFooter: getContentHighlightCardFooter(price, contentType),
+    cardFooter: getContentHighlightCardFooter({ price, contentType }),
   };
-  if (hyperlink) {
+  if (href) {
     cardInfo.cardTitle = (
-      <Hyperlink destination={hyperlink} target="_blank">
+      <Hyperlink destination={href} target="_blank">
         <Truncate lines={3} title={title}>{title}</Truncate>
       </Hyperlink>
     );
   }
   return (
-    <Card isLoading={isLoading}>
+    <Card>
       <Card.ImageCap
-        src={cardImageUrl}
+        src={cardInfo.cardImgSrc}
         srcAlt=""
         logoSrc={cardInfo.cardLogoSrc}
         logoAlt={cardInfo.cardLogoAlt}
@@ -55,7 +62,7 @@ ContentHighlightCardItem.propTypes = {
   isLoading: PropTypes.bool,
   cardImageUrl: PropTypes.string,
   title: PropTypes.string.isRequired,
-  hyperlink: PropTypes.string,
+  href: PropTypes.string,
   contentType: PropTypes.oneOf(['course', 'program', 'learnerpathway']).isRequired,
   partners: PropTypes.arrayOf(PropTypes.shape({
     name: PropTypes.string,
@@ -67,7 +74,7 @@ ContentHighlightCardItem.propTypes = {
 
 ContentHighlightCardItem.defaultProps = {
   isLoading: false,
-  hyperlink: undefined,
+  href: undefined,
   cardImageUrl: undefined,
   price: undefined,
 };

diff --git a/src/components/ContentHighlights/ContentHighlightSet.jsx b/src/components/ContentHighlights/ContentHighlightSet.jsx
@@ -9,7 +9,7 @@ const ContentHighlightSet = () => {
   const { highlightSetUUID } = useParams();
   const { highlightSet, isLoading } = useHighlightSet(highlightSetUUID);
   return (
-    <Container fluid className="mt-5">
+    <Container className="mt-5">
       <CurrentContentHighlightItemsHeader isLoading={isLoading} highlightTitle={highlightSet?.title} />
       <ContentHighlightsCardItemContainer isLoading={isLoading} highlightedContent={highlightSet?.highlightedContent} />
     </Container>

diff --git a/src/components/ContentHighlights/ContentHighlightsCardItemsContainer.jsx b/src/components/ContentHighlights/ContentHighlightsCardItemsContainer.jsx
@@ -1,37 +1,46 @@
 import React from 'react';
-import { CardGrid } from '@edx/paragon';
+import { CardGrid, Alert } from '@edx/paragon';
 import PropTypes from 'prop-types';
 import { connect } from 'react-redux';
 import ContentHighlightCardItem from './ContentHighlightCardItem';
-import { generateAboutPageUrl } from './data/constants';
+import {
+  DEFAULT_ERROR_MESSAGE, HIGHLIGHTS_CARD_GRID_COLUMN_SIZES, MAX_CONTENT_ITEMS_PER_HIGHLIGHT_SET,
+} from './data/constants';
+import SkeletonContentCardContainer from './SkeletonContentCardContainer';
+import { generateAboutPageUrl } from './data/utils';
 
 const ContentHighlightsCardItemsContainer = ({ enterpriseSlug, isLoading, highlightedContent }) => {
+  if (isLoading) {
+    return (
+      <SkeletonContentCardContainer length={MAX_CONTENT_ITEMS_PER_HIGHLIGHT_SET} />
+    );
+  }
   if (!highlightedContent || highlightedContent?.length === 0) {
-    return <div data-testid="empty-highlighted-content" />;
+    return (
+      <Alert data-testid="empty-highlighted-content" variant="warning">
+        {DEFAULT_ERROR_MESSAGE.EMPTY_HIGHLIGHT_SET}
+      </Alert>
+    );
   }
   return (
-    <CardGrid
-      columnSizes={{
-        xs: 12,
-        lg: 6,
-        xl: 4,
-      }}
-    >
+    <CardGrid columnSizes={HIGHLIGHTS_CARD_GRID_COLUMN_SIZES}>
       {highlightedContent.map(({
         uuid, title, contentType, authoringOrganizations, contentKey,
       }) => (
-
         <ContentHighlightCardItem
           isLoading={isLoading}
           key={uuid}
           cardImageUrl="https://picsum.photos/200/300"
           title={title}
-          hyperlink={generateAboutPageUrl(enterpriseSlug, contentType?.toLowerCase(), contentKey)}
+          href={generateAboutPageUrl({
+            enterpriseSlug,
+            contentType: contentType?.toLowerCase(),
+            contentKey,
+          })}
           contentType={contentType?.toLowerCase()}
           partners={authoringOrganizations}
         />
       ))}
-      {isLoading && <div data-testid="card-item-skeleton" />}
     </CardGrid>
   );
 };

diff --git a/src/components/ContentHighlights/ContentHighlightsDashboard.jsx b/src/components/ContentHighlights/ContentHighlightsDashboard.jsx
@@ -30,7 +30,7 @@ const ContentHighlightsDashboardBase = ({ children }) => {
   }, [history, location, locationState]);
 
   return (
-    <Container fluid className="my-5">
+    <Container className="my-5">
       <ContentHighlightHelmet title="Highlights" />
       {children}
       <Toast

diff --git a/src/components/ContentHighlights/CurrentContentHighlightItemsHeader.jsx b/src/components/ContentHighlights/CurrentContentHighlightItemsHeader.jsx
@@ -7,9 +7,11 @@ import DeleteHighlightSet from './DeleteHighlightSet';
 const CurrentContentHighlightItemsHeader = ({ isLoading, highlightTitle }) => {
   if (isLoading) {
     return (
-      <div data-testid="header-skeleton">
+      <ActionRow data-testid="header-skeleton">
+        <h2><Skeleton /></h2>
+        <ActionRow.Spacer />
         <Skeleton />
-      </div>
+      </ActionRow>
     );
   }
   return (

diff --git a/src/components/ContentHighlights/HighlightSetSection.jsx b/src/components/ContentHighlights/HighlightSetSection.jsx
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import { CardGrid } from '@edx/paragon';
 
 import ContentHighlightSetCard from './ContentHighlightSetCard';
+import { HIGHLIGHTS_CARD_GRID_COLUMN_SIZES } from './data/constants';
 
 const HighlightSetSection = ({
   title: sectionTitle,
@@ -15,13 +16,7 @@ const HighlightSetSection = ({
   return (
     <div>
       <h3 className="mb-3">{sectionTitle}</h3>
-      <CardGrid
-        columnSizes={{
-          xs: 12,
-          lg: 6,
-          xl: 4,
-        }}
-      >
+      <CardGrid columnSizes={HIGHLIGHTS_CARD_GRID_COLUMN_SIZES}>
         {highlightSets.map(({
           title,
           uuid,

diff --git a/src/components/ContentHighlights/HighlightStepper/ContentConfirmContentCard.jsx b/src/components/ContentHighlights/HighlightStepper/ContentConfirmContentCard.jsx
@@ -5,7 +5,7 @@ import { IconButton, Icon } from '@edx/paragon';
 import { connect } from 'react-redux';
 import ContentHighlightCardItem from '../ContentHighlightCardItem';
 import { useContentHighlightsContext } from '../data/hooks';
-import { generateAboutPageUrl } from '../data/constants';
+import { generateAboutPageUrl } from '../data/utils';
 
 const ContentConfirmContentCard = ({ enterpriseSlug, original }) => {
   const { deleteSelectedRowId } = useContentHighlightsContext();
@@ -38,19 +38,21 @@ const ContentConfirmContentCard = ({ enterpriseSlug, original }) => {
     <React.Fragment key={key}>
       <ContentHighlightCardItem
         title={title}
-        hyperlink={generateAboutPageUrl(enterpriseSlug, contentType?.toLowerCase(), aggregationKey?.split(':')[1])}
+        href={generateAboutPageUrl({
+          enterpriseSlug,
+          contentType: contentType?.toLowerCase(),
+          contentKey: aggregationKey?.split(':')[1],
+        })}
         contentType={contentType}
         partners={partners}
         cardImageUrl={cardImageUrl || originalImageUrl}
         price={firstEnrollablePaidSeatPrice}
 
       />
       <IconButton
-        invertColors
-        isActive
         src={Delete}
         iconAs={Icon}
-        alt="Delete"
+        alt={`Remove ${title} from highlight collection`}
         onClick={() => setDeleteKey(aggregationKey)}
         className="ml-1 flex-shrink-0"
       />

diff --git a/src/components/ContentHighlights/HighlightStepper/ContentSearchResultCard.jsx b/src/components/ContentHighlights/HighlightStepper/ContentSearchResultCard.jsx
@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { connect } from 'react-redux';
 import ContentHighlightCardItem from '../ContentHighlightCardItem';
-import { generateAboutPageUrl } from '../data/constants';
+import { generateAboutPageUrl } from '../data/utils';
 
 const ContentSearchResultCard = ({ enterpriseSlug, original }) => {
   const {
@@ -17,7 +17,11 @@ const ContentSearchResultCard = ({ enterpriseSlug, original }) => {
   return (
     <ContentHighlightCardItem
       title={title}
-      hyperlink={generateAboutPageUrl(enterpriseSlug, contentType?.toLowerCase(), aggregationKey?.split(':')[1])}
+      href={generateAboutPageUrl({
+        enterpriseSlug,
+        contentType: contentType?.toLowerCase(),
+        contentKey: aggregationKey?.split(':')[1],
+      })}
       contentType={contentType}
       partners={partners}
       cardImageUrl={cardImageUrl || originalImageUrl}

diff --git a/src/components/ContentHighlights/HighlightStepper/HighlightStepperConfirmContent.jsx b/src/components/ContentHighlights/HighlightStepper/HighlightStepperConfirmContent.jsx
@@ -9,48 +9,37 @@ import {
   Col,
   Icon,
   CardGrid,
+  Alert,
 } from '@edx/paragon';
 import { Assignment } from '@edx/paragon/icons';
 import { camelCaseObject } from '@edx/frontend-platform';
 import { Configure, InstantSearch, connectStateResults } from 'react-instantsearch-dom';
 import { configuration } from '../../../config';
 
 import {
-  STEPPER_STEP_TEXT, MAX_CONTENT_ITEMS_PER_HIGHLIGHT_SET,
+  STEPPER_STEP_TEXT,
+  MAX_CONTENT_ITEMS_PER_HIGHLIGHT_SET,
+  HIGHLIGHTS_CARD_GRID_COLUMN_SIZES,
+  DEFAULT_ERROR_MESSAGE,
 } from '../data/constants';
 import { ContentHighlightsContext } from '../ContentHighlightsContext';
-import SkeletonContentCard from '../SkeletonContentCard';
 import ContentConfirmContentCard from './ContentConfirmContentCard';
+import SkeletonContentCardContainer from '../SkeletonContentCardContainer';
 
 export const BaseReviewContentSelections = ({
   searchResults,
   isSearchStalled,
 }) => {
   if (isSearchStalled) {
     return (
-      <div data-testid="skeleton-container">
-        <CardGrid
-          columnSizes={{
-            xs: 12,
-            md: 6,
-            lg: 4,
-            xl: 3,
-          }}
-        >
-          {/* eslint-disable */}
-        {[...new Array(8)].map((element, index) => 
-        <SkeletonContentCard key={index} />)}
-        {/* eslint-enable */}
-        </CardGrid>
-      </div>
+      <SkeletonContentCardContainer length={MAX_CONTENT_ITEMS_PER_HIGHLIGHT_SET} />
     );
   }
   if (!searchResults) {
     return (<div data-testid="base-content-no-results" />);
   }
   const { hits } = camelCaseObject(searchResults);
 
-  // eslint-disable-next-line react-hooks/rules-of-hooks
   const transformedCardData = hits.map((highlightedContent) => {
     const {
       aggregationKey, title, cardImageUrl, contentType, partners, originalImageUrl, firstEnrollablePaidSeatPrice,
@@ -67,14 +56,7 @@ export const BaseReviewContentSelections = ({
     return original;
   });
   return (
-    <CardGrid
-      columnSizes={{
-        xs: 12,
-        md: 6,
-        lg: 4,
-        xl: 3,
-      }}
-    >
+    <CardGrid columnSizes={HIGHLIGHTS_CARD_GRID_COLUMN_SIZES}>
       {transformedCardData.map((original) => (
         <ContentConfirmContentCard key={original.aggregationKey} original={original} />))}
     </CardGrid>
@@ -130,11 +112,14 @@ export const SelectedContent = ({ enterpriseId }) => {
       filterString += ')';
     }
     return filterString;
-  // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [currentSelectedRowIds]);
+  }, [currentSelectedRowIds, enterpriseId]);
 
   if (currentSelectedRowIds.length === 0) {
-    return (<div data-testid="selected-content-no-results" />);
+    return (
+      <Alert data-testid="selected-content-no-results">
+        {DEFAULT_ERROR_MESSAGE.EMPTY_SELECTEDROWIDS}
+      </Alert>
+    );
   }
 
   return (

diff --git a/...mponents/ContentHighlights/HighlightStepper/tests/HighlightStepperConfirmContent.test.jsx b/...mponents/ContentHighlights/HighlightStepper/tests/HighlightStepperConfirmContent.test.jsx
@@ -8,6 +8,7 @@ import thunk from 'redux-thunk';
 import { Provider } from 'react-redux';
 import HighlightStepperConfirmContent, { BaseReviewContentSelections, SelectedContent } from '../HighlightStepperConfirmContent';
 import {
+  DEFAULT_ERROR_MESSAGE,
   testCourseAggregation,
 } from '../../data/constants';
 import { ContentHighlightsContext } from '../../ContentHighlightsContext';
@@ -67,7 +68,7 @@ describe('BaseReviewContentSelections', () => {
         <BaseReviewContentSelections isSearchStalled />
       </HighlightStepperConfirmContentWrapper>,
     );
-    expect(screen.getByTestId('skeleton-container')).toBeInTheDocument();
+    expect(screen.getAllByTestId('card-item-skeleton')).toBeTruthy();
   });
   it('should render selected card content', () => {
     renderWithRouter(
@@ -87,5 +88,6 @@ describe('SelectedContent', () => {
       </HighlightStepperConfirmContentWrapper>,
     );
     expect(screen.getByTestId('selected-content-no-results')).toBeInTheDocument();
+    expect(screen.getByText(DEFAULT_ERROR_MESSAGE.EMPTY_SELECTEDROWIDS)).toBeInTheDocument();
   });
 });
diff --git a/src/components/ContentHighlights/SkeletonContentCard.jsx b/src/components/ContentHighlights/SkeletonContentCard.jsx
@@ -2,7 +2,7 @@ import React from 'react';
 import { Card } from '@edx/paragon';
 
 const SkeletonContentCard = () => (
-  <Card isLoading>
+  <Card isLoading data-testid="card-item-skeleton">
     <Card.ImageCap logoSkeleton />
     <Card.Section className="mt-2" />
     <Card.Footer />

diff --git a/src/components/ContentHighlights/SkeletonContentCardContainer.jsx b/src/components/ContentHighlights/SkeletonContentCardContainer.jsx
@@ -0,0 +1,29 @@
+import PropTypes from 'prop-types';
+import { CardGrid } from '@edx/paragon';
+import SkeletonContentCard from './SkeletonContentCard';
+import { HIGHLIGHTS_CARD_GRID_COLUMN_SIZES } from './data/constants';
+
+const SkeletonContentCardContainer = ({ length, columnSizes }) => (
+  <CardGrid columnSizes={columnSizes}>
+    {[
+      ...new Array(length),
+    // eslint-disable-next-line react/no-array-index-key
+    ].map((_, index) => <SkeletonContentCard key={index} />)};
+  </CardGrid>
+);
+
+SkeletonContentCardContainer.propTypes = {
+  length: PropTypes.number.isRequired,
+  columnSizes: PropTypes.shape({
+    xs: PropTypes.number,
+    md: PropTypes.number,
+    lg: PropTypes.number,
+    xl: PropTypes.number,
+  }),
+};
+
+SkeletonContentCardContainer.defaultProps = {
+  columnSizes: HIGHLIGHTS_CARD_GRID_COLUMN_SIZES,
+};
+
+export default SkeletonContentCardContainer;