From 4d3b68d28743f4530c56608b36ea5d25a122eca4 Mon Sep 17 00:00:00 2001
From: Chris Deery <3932645+cdeery@users.noreply.github.com>
Date: Tue, 21 Sep 2021 15:02:26 -0400
Subject: [PATCH] feat: [AA-906] implemented review feedback

---
 .../data/__snapshots__/redux.test.js.snap     |  2 +-
 src/course-home/outline-tab/OutlineTab.jsx    | 33 ++++------
 src/course-home/outline-tab/messages.js       | 34 ++++++-----
 .../outline-tab/widgets/FlagButton.jsx        | 12 ++--
 ...meCard.jsx => StartOrResumeCourseCard.jsx} |  6 +-
 .../widgets/WeeklyLearningGoal.jsx            | 60 ++++++++++---------
 6 files changed, 75 insertions(+), 72 deletions(-)
 rename src/course-home/outline-tab/widgets/{StartResumeCard.jsx => StartOrResumeCourseCard.jsx} (90%)

diff --git a/src/course-home/data/__snapshots__/redux.test.js.snap b/src/course-home/data/__snapshots__/redux.test.js.snap
index adfd7389..1be1b0eb 100644
--- a/src/course-home/data/__snapshots__/redux.test.js.snap
+++ b/src/course-home/data/__snapshots__/redux.test.js.snap
@@ -434,7 +434,7 @@ Object {
         "courseGoals": Object {
           "daysPerWeek": null,
           "goalOptions": Array [],
-          "numberOfDaysGoalsEnabled": false,
+          "weeklyLearningGoalEnabled": false,
           "selectedGoal": null,
           "subscribedToReminders": null,
         },
diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx
index 5a845028..d3b1cb45 100644
--- a/src/course-home/outline-tab/OutlineTab.jsx
+++ b/src/course-home/outline-tab/OutlineTab.jsx
@@ -10,7 +10,7 @@ import { AlertList } from '../../generic/user-messages';
 import CourseDates from './widgets/CourseDates';
 import CourseGoalCard from './widgets/CourseGoalCard';
 import CourseHandouts from './widgets/CourseHandouts';
-import StartResumeCard from './widgets/StartResumeCard';
+import StartOrResumeCourseCard from './widgets/StartOrResumeCourseCard';
 import CourseTools from './widgets/CourseTools';
 import { fetchOutlineTab } from '../data';
 import genericMessages from '../../generic/messages';
@@ -56,7 +56,7 @@ function OutlineTab({ intl }) {
     courseGoals: {
       goalOptions,
       selectedGoal,
-      numberOfDaysGoalsEnabled,
+      weeklyLearningGoalEnabled,
     } = {},
     datesBannerInfo,
     datesWidget: {
@@ -71,7 +71,7 @@ function OutlineTab({ intl }) {
     verifiedMode,
   } = useModel('outline', courseId);
 
-  const [courseGoalToDisplay, setCourseGoalToDisplay] = useState(selectedGoal);
+  const [deprecatedCourseGoalToDisplay, setDeprecatedCourseGoalToDisplay] = useState(selectedGoal);
   const [goalToastHeader, setGoalToastHeader] = useState('');
   const [expandAll, setExpandAll] = useState(false);
 
@@ -135,13 +135,6 @@ function OutlineTab({ intl }) {
         <div className="col-12 col-sm-auto p-0">
           <div role="heading" aria-level="1" className="h2">{title}</div>
         </div>
-        {resumeCourseUrl && !numberOfDaysGoalsEnabled && (
-          <div className="col-12 col-sm-auto p-0">
-            <Button variant="brand" block href={resumeCourseUrl} onClick={() => logResumeCourseClick()}>
-              {hasVisitedCourse ? intl.formatMessage(messages.resume) : intl.formatMessage(messages.start)}
-            </Button>
-          </div>
-        )}
       </div>
       {/** [MM-P2P] Experiment (className for optimizely trigger) */}
       <div className="row course-outline-tab">
@@ -175,17 +168,17 @@ function OutlineTab({ intl }) {
               <UpgradeToShiftDatesAlert model="outline" logUpgradeLinkClick={logUpgradeToShiftDatesLinkClick} />
             </>
           )}
-          {!courseGoalToDisplay && goalOptions && goalOptions.length > 0 && (
+          {!deprecatedCourseGoalToDisplay && goalOptions && goalOptions.length > 0 && (
             <CourseGoalCard
               courseId={courseId}
               goalOptions={goalOptions}
               title={title}
-              setGoalToDisplay={(newGoal) => { setCourseGoalToDisplay(newGoal); }}
+              setGoalToDisplay={(newGoal) => { setDeprecatedCourseGoalToDisplay(newGoal); }}
               setGoalToastHeader={(newHeader) => { setGoalToastHeader(newHeader); }}
             />
           )}
-          {resumeCourseUrl && numberOfDaysGoalsEnabled && (
-          <StartResumeCard
+          {resumeCourseUrl && (
+          <StartOrResumeCourseCard
             hasVisitedCourse={hasVisitedCourse}
             resumeCourseUrl={resumeCourseUrl}
             logResumeCourseClick={logResumeCourseClick}
@@ -221,19 +214,19 @@ function OutlineTab({ intl }) {
               courseId={courseId}
               username={username}
             />
-            {courseGoalToDisplay && goalOptions && goalOptions.length > 0 && (
+            {deprecatedCourseGoalToDisplay && goalOptions && goalOptions.length > 0 && (
               <UpdateGoalSelector
                 courseId={courseId}
                 goalOptions={goalOptions}
-                selectedGoal={courseGoalToDisplay}
-                setGoalToDisplay={(newGoal) => { setCourseGoalToDisplay(newGoal); }}
+                selectedGoal={deprecatedCourseGoalToDisplay}
+                setGoalToDisplay={(newGoal) => { setDeprecatedCourseGoalToDisplay(newGoal); }}
                 setGoalToastHeader={(newHeader) => { setGoalToastHeader(newHeader); }}
               />
             )}
-            {numberOfDaysGoalsEnabled && (
+            {weeklyLearningGoalEnabled && (
               <WeeklyLearningGoal
-                selectedGoal={selectedGoal}
-                daysPerWeek={selectedGoal && 'daysPerWeek' in selectedGoal ? selectedGoal.daysPerWeek : 0}
+                daysPerWeek={selectedGoal && 'daysPerWeek' in selectedGoal ? selectedGoal.daysPerWeek : null}
+                subscribedToReminders={selectedGoal && 'subscribedToReminders' in selectedGoal ? selectedGoal.subscribedToReminders : false}
                 courseId={courseId}
               />
             )}
diff --git a/src/course-home/outline-tab/messages.js b/src/course-home/outline-tab/messages.js
index da013da9..57deea15 100644
--- a/src/course-home/outline-tab/messages.js
+++ b/src/course-home/outline-tab/messages.js
@@ -73,6 +73,7 @@ const messages = defineMessages({
   resumeBlurb: {
     id: 'learning.outline.resumeBlurb',
     defaultMessage: 'Pick up where you left off',
+    description: 'Text describing to the learner that they can return to the last section they visited within the course.',
   },
   resume: {
     id: 'learning.outline.resume',
@@ -88,7 +89,7 @@ const messages = defineMessages({
   },
   setWeeklyGoalDetail: {
     id: 'learning.outline.setWeeklyGoalDetail',
-    defaultMessage: 'Setting a goal motivates you to finish the course. You can always change it later',
+    defaultMessage: 'Setting a goal motivates you to finish the course. You can always change it later.',
   },
   start: {
     id: 'learning.outline.start',
@@ -128,28 +129,33 @@ const messages = defineMessages({
     defaultMessage: 'Welcome to',
     description: 'This precedes the title of the course',
   },
-  goalButtonTitleCasual: {
-    id: 'learning.outline.goalButtonTitleCasual',
+  setLearningGoalButtonScreenReaderText: {
+    id: 'learning.outline.goalButton.casual.title',
+    defaultMessage: 'Set a learning goal style.',
+    description: 'screen reader text informing learner they can select an intensity of learning goal',
+  },
+  casualGoalButtonTitle: {
+    id: 'learning.outline.goalButton.screenReader.text',
     defaultMessage: 'Casual',
   },
-  goalButtonTextCasual: {
-    id: 'learning.outline.goalButtonTextCasual',
+  casualGoalButtonText: {
+    id: 'learning.outline.goalButton.casual.text',
     defaultMessage: '1 day a week',
   },
-  goalButtonTitleRegular: {
-    id: 'learning.outline.goalButtonTitleRegular',
+  regularGoalButtonTitle: {
+    id: 'learning.outline.goalButton.regular.title',
     defaultMessage: 'Regular',
   },
-  goalButtonTextRegular: {
-    id: 'learning.outline.goalButtonTextRegular',
+  regularGoalButtonText: {
+    id: 'learning.outline.goalButton.regular.text',
     defaultMessage: '3 days a week',
   },
-  goalButtonTitleIntense: {
-    id: 'learning.outline.goalButtonTitleIntense',
+  intenseGoalButtonTitle: {
+    id: 'learning.outline.goalButton.intense.title',
     defaultMessage: 'Intense',
   },
-  goalButtonTextIntense: {
-    id: 'learning.outline.goalButtonTextIntense',
+  intenseGoalButtonText: {
+    id: 'learning.outline.goalButton.intense.text',
     defaultMessage: '5 days a week',
   },
   setGoalReminder: {
@@ -158,7 +164,7 @@ const messages = defineMessages({
   },
   goalReminderDetail: {
     id: 'learning.outline.goalReminderDetail',
-    defaultMessage: 'If we notice you\'re not quite at your goal, we\'ll send you an email reminder.',
+    defaultMessage: 'If we notice you’re not quite at your goal, we’ll send you an email reminder.',
   },
   proctoringInfoPanel: {
     id: 'learning.proctoringPanel.header',
diff --git a/src/course-home/outline-tab/widgets/FlagButton.jsx b/src/course-home/outline-tab/widgets/FlagButton.jsx
index 3eafb820..7db7a321 100644
--- a/src/course-home/outline-tab/widgets/FlagButton.jsx
+++ b/src/course-home/outline-tab/widgets/FlagButton.jsx
@@ -1,26 +1,22 @@
 import React, { useState } from 'react';
+import classNames from 'classnames';
 
 import PropTypes from 'prop-types';
 
 function FlagButton({
   icon,
+  srText,
   title,
   text,
   isEnabled,
   handleSelect,
 }) {
-  const baseOutlineStyle = 'col flex-grow-1 p-3 border border-light rounded bg-white';
-  const selectedOutlineStyle = 'col flex-grow-1 p-3 border border-dark rounded bg-white';
   const [isHighlight, setHighlight] = useState(false);
 
-  function getOutlineStyle() {
-    return isEnabled || isHighlight ? selectedOutlineStyle : baseOutlineStyle;
-  }
-
   return (
     <button
       type="button"
-      className={getOutlineStyle()}
+      className={classNames('col flex-grow-1 p-3 border border-light rounded bg-white', { 'border-dark': isEnabled || isHighlight })}
       onMouseEnter={() => setHighlight(true)}
       onMouseLeave={() => setHighlight(false)}
       onClick={() => handleSelect()}
@@ -28,6 +24,7 @@ function FlagButton({
       <div className=" justify-content-center">
         {icon}
       </div>
+      <span className="sr-only sr-only-focusable">{srText}</span>
       <div className="text-center small">
         {title}
       </div>
@@ -40,6 +37,7 @@ function FlagButton({
 
 FlagButton.propTypes = {
   icon: PropTypes.node.isRequired,
+  srText: PropTypes.string.isRequired,
   title: PropTypes.string.isRequired,
   text: PropTypes.string,
   isEnabled: PropTypes.bool,
diff --git a/src/course-home/outline-tab/widgets/StartResumeCard.jsx b/src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx
similarity index 90%
rename from src/course-home/outline-tab/widgets/StartResumeCard.jsx
rename to src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx
index cb967751..950f4b72 100644
--- a/src/course-home/outline-tab/widgets/StartResumeCard.jsx
+++ b/src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx
@@ -6,7 +6,7 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 
 import messages from '../messages';
 
-function StartResumeCard({
+function StartOrResumeCourseCard({
   hasVisitedCourse,
   resumeCourseUrl,
   logResumeCourseClick,
@@ -33,11 +33,11 @@ function StartResumeCard({
   );
 }
 
-StartResumeCard.propTypes = {
+StartOrResumeCourseCard.propTypes = {
   hasVisitedCourse: PropTypes.bool.isRequired,
   resumeCourseUrl: PropTypes.string.isRequired,
   logResumeCourseClick: PropTypes.func.isRequired,
   intl: intlShape.isRequired,
 };
 
-export default injectIntl(StartResumeCard);
+export default injectIntl(StartOrResumeCourseCard);
diff --git a/src/course-home/outline-tab/widgets/WeeklyLearningGoal.jsx b/src/course-home/outline-tab/widgets/WeeklyLearningGoal.jsx
index 0f5166c5..5c34fabf 100644
--- a/src/course-home/outline-tab/widgets/WeeklyLearningGoal.jsx
+++ b/src/course-home/outline-tab/widgets/WeeklyLearningGoal.jsx
@@ -14,14 +14,14 @@ import FlagButton from './FlagButton';
 import { saveWeeklyCourseGoal } from '../../data';
 
 function WeeklyLearningGoal({
-  selectedGoal,
+  daysPerWeek,
+  subscribedToReminders,
   courseId,
   intl,
 }) {
+  const [daysPerWeekGoal, setDaysPerWeekGoal] = useState(daysPerWeek);
   // eslint-disable-next-line react/prop-types
-  const [daysPerWeekGoal, setDaysPerWeekGoal] = useState(selectedGoal && 'daysPerWeek' in selectedGoal ? selectedGoal.daysPerWeek : 0);
-  // eslint-disable-next-line react/prop-types
-  const [isGetReminderChecked, setGetReminderChecked] = useState(selectedGoal && 'subscribedToReminders' in selectedGoal ? selectedGoal.subscribedToReminders : false);
+  const [isGetReminderSelected, setGetReminderSelected] = useState(subscribedToReminders);
   const LevelToDays = {
     CASUAL: 3,
     REGULAR: 4,
@@ -31,17 +31,14 @@ function WeeklyLearningGoal({
 
   function handleSelect(days) {
     setDaysPerWeekGoal(days);
-    saveWeeklyCourseGoal(courseId, days, isGetReminderChecked);
-    // TODO: add Toast? Or is that just for the previous feature because the UI disappears?
+    saveWeeklyCourseGoal(courseId, days, isGetReminderSelected);
   }
 
   function handleSubscribeToReminders(event) {
-    const isGetReminders = event.target.checked;
-    setGetReminderChecked(isGetReminders);
-    saveWeeklyCourseGoal(courseId, daysPerWeekGoal, isGetReminders);
+    const isGetReminderChecked = event.target.checked;
+    setGetReminderSelected(isGetReminderChecked);
+    saveWeeklyCourseGoal(courseId, daysPerWeekGoal, isGetReminderChecked);
   }
-  const buttonRowStyle = 'row w-100 m-0 p-0 justify-content-around'; // 'row w-100 m-0 flex-grow-1 p-0 justify content-end';
-  const flagButtonStyle = 'col-auto col-md-12 col-xl-auto m-0 p-0 pb-md-3 pb-xl-0'; // 'col-auto flex-grow-1 p-0';
 
   return (
     <div className="row w-100 m-0 p-0">
@@ -53,46 +50,50 @@ function WeeklyLearningGoal({
           <Card.Text>
             {intl.formatMessage(messages.setWeeklyGoalDetail)}
           </Card.Text>
-          <div className={buttonRowStyle}>
-            <div className={flagButtonStyle}>
+          <div className="row w-100 m-0 p-0 justify-content-around">
+            <div className="col-auto col-md-12 col-xl-auto m-0 p-0 pb-md-3 pb-xl-0">
               <FlagButton
                 icon={<FlagCasualIcon />}
-                title={intl.formatMessage(messages.goalButtonTitleCasual)}
-                text={intl.formatMessage(messages.goalButtonTextCasual)}
-                isEnabled={daysPerWeekGoal === LevelToDays.CASUAL}
+                srText={intl.formatMessage(messages.setLearningGoalButtonScreenReaderText)}
+                title={intl.formatMessage(messages.casualGoalButtonTitle)}
+                text={intl.formatMessage(messages.casualGoalButtonText)}
+                isEnabled={LevelToDays.CASUAL === daysPerWeekGoal}
                 handleSelect={() => { handleSelect(LevelToDays.CASUAL); }}
               />
             </div>
-            <div className={flagButtonStyle}>
+            <div className="col-auto col-md-12 col-xl-auto m-0 p-0 pb-md-3 pb-xl-0">
               <FlagButton
                 icon={<FlagRegularIcon />}
-                title={intl.formatMessage(messages.goalButtonTitleRegular)}
-                text={intl.formatMessage(messages.goalButtonTextRegular)}
-                isEnabled={daysPerWeekGoal === LevelToDays.REGULAR}
+                srText={intl.formatMessage(messages.setLearningGoalButtonScreenReaderText)}
+                title={intl.formatMessage(messages.regularGoalButtonTitle)}
+                text={intl.formatMessage(messages.regularGoalButtonText)}
+                isEnabled={LevelToDays.REGULAR === daysPerWeekGoal}
                 handleSelect={() => { handleSelect(LevelToDays.REGULAR); }}
               />
             </div>
-            <div className={flagButtonStyle}>
+            <div className="col-auto col-md-12 col-xl-auto m-0 p-0 pb-md-3 pb-xl-0">
               <FlagButton
                 icon={<FlagIntenseIcon />}
-                title={intl.formatMessage(messages.goalButtonTitleIntense)}
-                text={intl.formatMessage(messages.goalButtonTextIntense)}
-                isEnabled={daysPerWeekGoal === LevelToDays.INTENSE}
+                srText={intl.formatMessage(messages.setLearningGoalButtonScreenReaderText)}
+                title={intl.formatMessage(messages.intenseGoalButtonTitle)}
+                text={intl.formatMessage(messages.intenseGoalButtonText)}
+                isEnabled={LevelToDays.INTENSE === daysPerWeekGoal}
                 handleSelect={() => { handleSelect(LevelToDays.INTENSE); }}
               />
             </div>
           </div>
           <div className="row p-3">
             <Form.Switch
-              checked={isGetReminderChecked}
+              checked={isGetReminderSelected}
               onChange={(event) => handleSubscribeToReminders(event)}
+              disabled={!daysPerWeekGoal}
             >
               {intl.formatMessage(messages.setGoalReminder)}
             </Form.Switch>
           </div>
         </Card.Body>
         {/* This is supposed to fill with gray in the bottom of the card */}
-        {isGetReminderChecked && (
+        {isGetReminderSelected && (
           <Card.Footer className="border-0 px-2.5">
             <div className="row w-100  m-0  small align-center">
               <div className="d-flex align-items-center pr-1.5">
@@ -111,9 +112,14 @@ function WeeklyLearningGoal({
 }
 
 WeeklyLearningGoal.propTypes = {
-  selectedGoal: PropTypes.shape({}).isRequired,
+  daysPerWeek: PropTypes.number,
+  subscribedToReminders: PropTypes.bool,
   courseId: PropTypes.string.isRequired,
   intl: intlShape.isRequired,
 };
 
+WeeklyLearningGoal.defaultProps = {
+  daysPerWeek: null,
+  subscribedToReminders: false,
+};
 export default injectIntl(WeeklyLearningGoal);