fix: add review changes, fix advanced tab hour selection and update t…

…ests
open-craft · Dec 25, 2023 · cc90031 · cc90031
1 parent de0d8e5
commit cc90031
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 40 deletions.
diff --git a/src/course-outline/CourseOutline.test.jsx b/src/course-outline/CourseOutline.test.jsx
@@ -489,7 +489,6 @@ describe('<CourseOutline />', () => {
     const {
       findAllByTestId,
       findByText,
-      findAllByPlaceholderText,
       findAllByRole,
       findByRole,
       findByTestId,
@@ -554,12 +553,13 @@ describe('<CourseOutline />', () => {
       false,
     ), store.dispatch);
     fireEvent.click(subsectionDropdownButton);
-    const configureBtn = await findByText(cardHeaderMessages.menuConfigure.defaultMessage);
+    const configureBtn = await within(firstSubsection).findByTestId('subsection-card-header__menu-configure-button');
     fireEvent.click(configureBtn);
 
-    const datePicker = await findAllByPlaceholderText('MM/DD/YYYY');
-    expect(datePicker[0]).toHaveValue('08/10/2025');
-    expect(datePicker[1]).toHaveValue('09/10/2025');
+    const releaseDatePicker = await findByTestId('release-date-picker');
+    expect(releaseDatePicker).toHaveValue('08/10/2025');
+    const dueDatePicker = await findByTestId('release-date-picker');
+    expect(dueDatePicker).toHaveValue('09/10/2025');
 
     expect(await findByText(newGraderType)).toBeInTheDocument();
     const advancedTab = await findByRole('tab', { name: configureModalMessages.advancedTabTitle.defaultMessage });

diff --git a/src/course-outline/configure-modal/AdvancedTab.jsx b/src/course-outline/configure-modal/AdvancedTab.jsx
@@ -21,7 +21,7 @@ const AdvancedTab = ({
     }
   };
 
-  const setSelected = (valueString) => {
+  const setCurrentTimeLimit = (valueString) => {
     const value = valueString.split(':');
     setDefaultTimeLimit(Number.parseInt(value[0], 10) * 60 + Number.parseInt(value[1], 10));
   };
@@ -70,7 +70,8 @@ const AdvancedTab = ({
           <h6 className="mt-4 text-gray-700"><FormattedMessage {...messages.timeAllotted} /></h6>
           <Form.Autosuggest
             value={defaultTimeLimit === null ? formatHour(timeLimits[0]) : formatHour(defaultTimeLimit)}
-            onSelected={(value) => setSelected(value)}
+            onChange={setCurrentTimeLimit}
+            onSelected={setCurrentTimeLimit}
             data-testid="hour-autosuggest"
           >
             {generateTimeLimits()}
@@ -83,8 +84,6 @@ const AdvancedTab = ({
 };
 
 AdvancedTab.propTypes = {
-  releaseDate: PropTypes.string.isRequired,
-  setReleaseDate: PropTypes.func.isRequired,
   isTimeLimited: PropTypes.bool.isRequired,
   setIsTimeLimited: PropTypes.func.isRequired,
   defaultTimeLimit: PropTypes.string.isRequired,

diff --git a/src/course-outline/configure-modal/BasicTab.jsx b/src/course-outline/configure-modal/BasicTab.jsx
@@ -16,16 +16,9 @@ const BasicTab = ({
   setDueDate,
 }) => {
   const intl = useIntl();
-  const onChangeReleaseDate = (value) => {
-    setReleaseDate(value);
-  };
 
   const onChangeGraderType = (e) => setGraderType(e.target.value);
 
-  const onChangeDueDate = (value) => {
-    setDueDate(value);
-  };
-
   const createOptions = () => courseGraders.map((option) => (
     <option value={option}> {option} </option>
   ));
@@ -40,14 +33,15 @@ const BasicTab = ({
           value={releaseDate}
           label={intl.formatMessage(messages.releaseDate)}
           controlName="state-date"
-          onChange={(date) => onChangeReleaseDate(date)}
+          onChange={setReleaseDate}
+          data-testid="release-date-picker"
         />
         <DatepickerControl
           type={DATEPICKER_TYPES.time}
           value={releaseDate}
           label={intl.formatMessage(messages.releaseTimeUTC)}
           controlName="start-time"
-          onChange={(date) => onChangeReleaseDate(date)}
+          onChange={setReleaseDate}
         />
       </Stack>
       {
@@ -71,14 +65,15 @@ const BasicTab = ({
                 value={dueDate}
                 label={intl.formatMessage(messages.dueDate)}
                 controlName="state-date"
-                onChange={(date) => onChangeDueDate(date)}
+                onChange={setDueDate}
+                data-testid="due-date-picker"
               />
               <DatepickerControl
                 type={DATEPICKER_TYPES.time}
                 value={dueDate}
                 label={intl.formatMessage(messages.dueTimeUTC)}
                 controlName="start-time"
-                onChange={(date) => onChangeDueDate(date)}
+                onChange={setDueDate}
               />
             </Stack>
           </div>

diff --git a/src/course-outline/configure-modal/ConfigureModal.jsx b/src/course-outline/configure-modal/ConfigureModal.jsx
@@ -17,6 +17,7 @@ import messages from './messages';
 import BasicTab from './BasicTab';
 import VisibilityTab from './VisibilityTab';
 import AdvancedTab from './AdvancedTab';
+import { COURSE_BLOCK_NAMES } from '../constants';
 
 const ConfigureModal = ({
   isOpen,
@@ -43,17 +44,17 @@ const ConfigureModal = ({
   const [graderType, setGraderType] = useState(format == null ? 'Not Graded' : format);
   const [dueDateState, setDueDateState] = useState('');
   const [isTimeLimitedState, setIsTimeLimitedState] = useState(false);
-  const [defaultTimeLimitMin, setDefaultTimeLimitMin] = useState(30);
+  const [defaultTimeLimitMin, setDefaultTimeLimitMin] = useState(defaultTimeLimitMinutes);
   const [hideAfterDueState, setHideAfterDueState] = useState(false);
   const [showCorrectnessState, setShowCorrectnessState] = useState(false);
-  const [isSubsection, setIsSubsection] = useState(category === 'sequential');
+  const [isSubsection, setIsSubsection] = useState(category === COURSE_BLOCK_NAMES.sequential.id);
 
   useEffect(() => {
     setReleaseDate(sectionStartDate);
   }, [sectionStartDate]);
 
   useEffect(() => {
-    setIsSubsection(category === 'sequential');
+    setIsSubsection(category === COURSE_BLOCK_NAMES.sequential.id);
   }, [category]);
 
   useEffect(() => {
@@ -133,13 +134,14 @@ const ConfigureModal = ({
     setShowCorrectnessState(showCorrectness);
     setIsVisibleToStaffOnly(visibilityState === VisibilityTypes.STAFF_ONLY);
     setGraderType(format);
+    onClose();
   };
 
   return (
     <ModalDialog
       className="configure-modal"
       isOpen={isOpen}
-      onClose={onClose}
+      onClose={handleClose}
       hasCloseButton
       isFullscreenOnMobile
     >
@@ -164,6 +166,7 @@ const ConfigureModal = ({
           </Tab>
           <Tab eventKey="visibility" title={intl.formatMessage(messages.visibilityTabTitle)}>
             <VisibilityTab
+              category={category}
               isSubsection={isSubsection}
               isVisibleToStaffOnly={isVisibleToStaffOnly}
               setIsVisibleToStaffOnly={setIsVisibleToStaffOnly}
@@ -190,7 +193,7 @@ const ConfigureModal = ({
       </ModalDialog.Body>
       <ModalDialog.Footer className="pt-1">
         <ActionRow>
-          <ModalDialog.CloseButton variant="tertiary" onClick={handleClose}>
+          <ModalDialog.CloseButton variant="tertiary">
             {intl.formatMessage(messages.cancelButton)}
           </ModalDialog.CloseButton>
           <Button onClick={handleSave} disabled={saveButtonDisabled}>

diff --git a/src/course-outline/configure-modal/ConfigureModal.scss b/src/course-outline/configure-modal/ConfigureModal.scss
@@ -5,8 +5,8 @@
   .configure-modal__header {
     padding-top: 1.5rem;
   }
-}
 
-.configure-modal__body {
-  overflow: visible;
+  .configure-modal__body {
+    overflow: visible;
+  }
 }
diff --git a/src/course-outline/configure-modal/ConfigureModal.test.jsx b/src/course-outline/configure-modal/ConfigureModal.test.jsx
@@ -148,7 +148,7 @@ describe('<ConfigureModal />', () => {
 
     const visibilityTab = getByRole('tab', { name: messages.visibilityTabTitle.defaultMessage });
     fireEvent.click(visibilityTab);
-    expect(getByText(messages.sectionVisibility.defaultMessage)).toBeInTheDocument();
+    expect(getByText('Section Visibility')).toBeInTheDocument();
     expect(getByText(messages.hideFromLearners.defaultMessage)).toBeInTheDocument();
   });
 
@@ -245,7 +245,7 @@ describe('<ConfigureModal />', () => {
 
     const visibilityTab = getByRole('tab', { name: messages.visibilityTabTitle.defaultMessage });
     fireEvent.click(visibilityTab);
-    expect(getByText(messages.subsectionVisibility.defaultMessage)).toBeInTheDocument();
+    expect(getByText('Subsection Visibility')).toBeInTheDocument();
     expect(getByText(messages.showEntireSubsection.defaultMessage)).toBeInTheDocument();
     expect(getByText(messages.showEntireSubsectionDescription.defaultMessage)).toBeInTheDocument();
     expect(getByText(messages.hideContentAfterDue.defaultMessage)).toBeInTheDocument();

diff --git a/src/course-outline/configure-modal/VisibilityTab.jsx b/src/course-outline/configure-modal/VisibilityTab.jsx
@@ -1,10 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { Alert, Form } from '@edx/paragon';
-import { FormattedMessage, injectIntl } from '@edx/frontend-platform/i18n';
+import { FormattedMessage, injectIntl, useIntl } from '@edx/frontend-platform/i18n';
 import messages from './messages';
+import { COURSE_BLOCK_NAMES } from '../constants';
 
 const VisibilityTab = ({
+  category,
   isVisibleToStaffOnly,
   setIsVisibleToStaffOnly,
   showWarning,
@@ -14,6 +16,8 @@ const VisibilityTab = ({
   showCorrectness,
   setShowCorrectness,
 }) => {
+  const intl = useIntl();
+  const visibilityTitle = COURSE_BLOCK_NAMES[category]?.name;
   const handleChange = (e) => {
     setIsVisibleToStaffOnly(e.target.checked);
   };
@@ -48,13 +52,9 @@ const VisibilityTab = ({
 
   return (
     <>
-      {
-        isSubsection ? (
-          <h5 className="mt-4 text-gray-700"><FormattedMessage {...messages.subsectionVisibility} /></h5>
-        ) : (
-          <h5 className="mt-4 text-gray-700"><FormattedMessage {...messages.sectionVisibility} /></h5>
-        )
-      }
+      <h5 className="mt-4 text-gray-700">
+        {intl.formatMessage(messages.visibilitySectionTitle, { visibilityTitle })}
+      </h5>
       <hr />
       {
         isSubsection ? (
@@ -117,6 +117,7 @@ const VisibilityTab = ({
 };
 
 VisibilityTab.propTypes = {
+  category: PropTypes.string.isRequired,
   isVisibleToStaffOnly: PropTypes.bool.isRequired,
   showWarning: PropTypes.bool.isRequired,
   setIsVisibleToStaffOnly: PropTypes.func.isRequired,

diff --git a/src/course-outline/configure-modal/messages.js b/src/course-outline/configure-modal/messages.js
@@ -25,9 +25,9 @@ const messages = defineMessages({
     id: 'course-authoring.course-outline.configure-modal.visibility-tab.title',
     defaultMessage: 'Visibility',
   },
-  sectionVisibility: {
+  visibilitySectionTitle: {
     id: 'course-authoring.course-outline.configure-modal.visibility-tab.section-visibility',
-    defaultMessage: 'Section Visibility',
+    defaultMessage: '{visibilityTitle} Visibility',
   },
   hideFromLearners: {
     id: 'course-authoring.course-outline.configure-modal.visibility-tab.hide-from-learners',