From 9fd6cbf61b3d0d7e436bf33e50cea21d956291bd Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@opencraft.com>
Date: Thu, 21 Oct 2021 12:50:36 +0530
Subject: [PATCH] fix: UX feedback suggestions from TNL-8730 [BD-38] [BB-4981]
 (#201)

* fix: UX feedback suggestions from TNL-8730
---
 src/generic/CollapsableEditor.jsx             |  2 +-
 src/generic/FormikControl.jsx                 | 51 ++++++++++
 .../app-settings-modal/AppSettingsModal.jsx   | 25 +++--
 src/pages-and-resources/teams/GroupEditor.jsx | 93 +++++++------------
 src/pages-and-resources/teams/Settings.jsx    | 36 ++++---
 src/pages-and-resources/teams/messages.js     | 15 +--
 6 files changed, 130 insertions(+), 92 deletions(-)
 create mode 100644 src/generic/FormikControl.jsx

diff --git a/src/generic/CollapsableEditor.jsx b/src/generic/CollapsableEditor.jsx
index 3703963d8..a01f358c2 100644
--- a/src/generic/CollapsableEditor.jsx
+++ b/src/generic/CollapsableEditor.jsx
@@ -28,7 +28,7 @@ const CollapsableEditor = ({
       className="collapsible-trigger d-flex border-0 align-items-center"
       style={{ justifyContent: 'unset' }}
     >
-      <div className="d-flex flex-grow-1">
+      <div className="d-flex flex-grow-1 w-75">
         {title}
       </div>
       <Collapsible.Visible whenClosed>
diff --git a/src/generic/FormikControl.jsx b/src/generic/FormikControl.jsx
new file mode 100644
index 000000000..606f1989b
--- /dev/null
+++ b/src/generic/FormikControl.jsx
@@ -0,0 +1,51 @@
+import { Form } from '@edx/paragon';
+import { getIn, useFormikContext } from 'formik';
+import PropTypes from 'prop-types';
+import React from 'react';
+import FormikErrorFeedback from './FormikErrorFeedback';
+
+function FormikControl({
+  name,
+  label,
+  help,
+  className,
+  ...params
+}) {
+  const {
+    touched, errors, handleChange, handleBlur, setFieldError,
+  } = useFormikContext();
+  const fieldTouched = getIn(touched, name);
+  const fieldError = getIn(errors, name);
+  const handleFocus = (e) => setFieldError(e.target.name, undefined);
+
+  return (
+    <Form.Group className={className}>
+      {label}
+      <Form.Control
+        {...params}
+        name={name}
+        className="pb-2"
+        onChange={handleChange}
+        onBlur={handleBlur}
+        onFocus={handleFocus}
+        isInvalid={fieldTouched && fieldError}
+      />
+      <FormikErrorFeedback name={name}>
+        <Form.Text>{help}</Form.Text>
+      </FormikErrorFeedback>
+    </Form.Group>
+  );
+}
+
+FormikControl.propTypes = {
+  name: PropTypes.element.isRequired,
+  label: PropTypes.element.isRequired,
+  help: PropTypes.element.isRequired,
+  className: PropTypes.string.isRequired,
+  value: PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.number,
+  ]).isRequired,
+};
+
+export default FormikControl;
diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx b/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx
index c6b9fe716..e3e58554c 100644
--- a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx
+++ b/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx
@@ -1,4 +1,6 @@
-import React, { useContext, useEffect, useState } from 'react';
+import React, {
+  useContext, useEffect, useRef, useState,
+} from 'react';
 import PropTypes from 'prop-types';
 
 import { Formik } from 'formik';
@@ -124,6 +126,7 @@ function AppSettingsModal({
   const { courseId } = useContext(PagesAndResourcesContext);
   const loadingStatus = useSelector(getLoadingStatus);
   const updateSettingsRequestStatus = useSelector(getSavingStatus);
+  const alertRef = useRef(null);
   const [saveError, setSaveError] = useState(false);
   const appInfo = useModel('courseApps', appId);
   const dispatch = useDispatch();
@@ -147,7 +150,17 @@ function AppSettingsModal({
     if (onSettingsSave) {
       success = success && await onSettingsSave(values);
     }
-    setSaveError(!success);
+    await setSaveError(!success);
+    !success && alertRef?.current.scrollIntoView(); // eslint-disable-line no-unused-expressions
+  };
+
+  const handleFormikSubmit = ({ handleSubmit, errors }) => async (event) => {
+    // If submitting the form with errors, show the alert and scroll to it.
+    await handleSubmit(event);
+    if (Object.keys(errors).length > 0) {
+      await setSaveError(true);
+      alertRef?.current.scrollIntoView(); // eslint-disable-line no-unused-expressions
+    }
   };
 
   const learnMoreLink = appInfo.documentationLinks?.learnMoreConfiguration && (
@@ -178,9 +191,7 @@ function AppSettingsModal({
         onSubmit={handleFormSubmit}
       >
         {(formikProps) => (
-          <Form
-            onSubmit={formikProps.handleSubmit}
-          >
+          <Form onSubmit={handleFormikSubmit(formikProps)}>
             <AppSettingsModalBase
               title={title}
               isOpen
@@ -197,12 +208,12 @@ function AppSettingsModal({
                     complete: intl.formatMessage(messages.saved),
                   }}
                   state={submitButtonState}
-                  onClick={formikProps.handleSubmit}
+                  onClick={handleFormikSubmit(formikProps)}
                 />
               )}
             >
               {saveError && (
-                <Alert variant="danger" icon={Info}>
+                <Alert variant="danger" icon={Info} ref={alertRef}>
                   <Alert.Heading>
                     {intl.formatMessage(messages.errorSavingTitle)}
                   </Alert.Heading>
diff --git a/src/pages-and-resources/teams/GroupEditor.jsx b/src/pages-and-resources/teams/GroupEditor.jsx
index d95f83ea0..b0d62edd6 100644
--- a/src/pages-and-resources/teams/GroupEditor.jsx
+++ b/src/pages-and-resources/teams/GroupEditor.jsx
@@ -1,13 +1,11 @@
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
-import {
-  Button, Form, TransitionReplace,
-} from '@edx/paragon';
+import { Button, Form, TransitionReplace } from '@edx/paragon';
 import PropTypes from 'prop-types';
 import React, { useState } from 'react';
 import { GroupTypes, TeamSizes } from '../../data/constants';
 
 import CollapsableEditor from '../../generic/CollapsableEditor';
-import FormikErrorFeedback from '../../generic/FormikErrorFeedback';
+import FormikControl from '../../generic/FormikControl';
 import messages from './messages';
 
 // Maps a team type to its corresponding intl message
@@ -27,7 +25,7 @@ const TeamTypeNameMessage = {
 };
 
 function GroupEditor({
-  intl, group, onDelete, onChange, onBlur, fieldNameCommonBase, errors, setFieldError,
+  intl, group, onDelete, onChange, onBlur, fieldNameCommonBase, errors,
 }) {
   const [isDeleting, setDeleting] = useState(false);
   const [isOpen, setOpen] = useState(group.id === null);
@@ -35,7 +33,6 @@ function GroupEditor({
   const cancelDeletion = () => setDeleting(false);
 
   const handleToggle = (open) => setOpen(Boolean(errors.name || errors.maxTeamSize || errors.description) || open);
-  const handleFocus = (e) => setFieldError(e.target.name, undefined);
 
   const formGroupClasses = 'mb-4 mx-2';
 
@@ -45,7 +42,7 @@ function GroupEditor({
         ? (
           <div className="d-flex flex-column card rounded mb-3 px-3 py-2 p-4" key="isDeleting">
             <h3>{intl.formatMessage(messages.groupDeleteHeading)}</h3>
-            <p>{intl.formatMessage(messages.groupDeleteBody)}</p>
+            {intl.formatMessage(messages.groupDeleteBody).split('\n').map(text => <p>{text}</p>)}
             <div className="d-flex flex-row justify-content-end">
               <Button variant="muted" size="sm" onClick={cancelDeletion}>
                 {intl.formatMessage(messages.cancel)}
@@ -73,44 +70,31 @@ function GroupEditor({
                     {intl.formatMessage(messages.configureGroup)}
                   </div>
                 ) : (
-                  <div className="d-flex flex-column flex-shrink-1 small">
-                    <span className="small text-gray-500">{intl.formatMessage(TeamTypeNameMessage[group.type].label)}</span>
-                    <span className="text-truncate text-black">{group.name || '<new>'}</span>
-                    <span className="small text-muted text-gray-500">{group.description || '<new>'}</span>
+                  <div className="d-flex flex-column flex-shrink-1 small mw-100">
+                    <div className="small text-gray-500">{intl.formatMessage(TeamTypeNameMessage[group.type].label)}</div>
+                    <div className="h4 text-truncate my-1">{group.name}</div>
+                    <div className="small text-truncate text-muted text-gray-500">{group.description}</div>
                   </div>
                 )
             }
           >
-            <Form.Group className={`${formGroupClasses} mt-2.5`}>
-              <Form.Control
-                className="pb-2"
-                name={`${fieldNameCommonBase}.name`}
-                floatingLabel={intl.formatMessage(messages.groupFormNameLabel)}
-                defaultValue={group.name}
-                onChange={onChange}
-                onBlur={onBlur}
-                onFocus={handleFocus}
-              />
-              <FormikErrorFeedback name={`${fieldNameCommonBase}.name`}>
-                <Form.Text>{intl.formatMessage(messages.groupFormNameHelp)}</Form.Text>
-              </FormikErrorFeedback>
-            </Form.Group>
-            <Form.Group className={formGroupClasses}>
-              <Form.Control
-                className="pb-2"
-                as="textarea"
-                rows={4}
-                name={`${fieldNameCommonBase}.description`}
-                floatingLabel={intl.formatMessage(messages.groupFormDescriptionLabel)}
-                defaultValue={group.description}
-                onChange={onChange}
-                onBlur={onBlur}
-                onFocus={handleFocus}
-              />
-              <FormikErrorFeedback name={`${fieldNameCommonBase}.description`}>
-                <Form.Text>{intl.formatMessage(messages.groupFormDescriptionHelp)}</Form.Text>
-              </FormikErrorFeedback>
-            </Form.Group>
+            <FormikControl
+              name={`${fieldNameCommonBase}.name`}
+              value={group.name}
+              floatingLabel={intl.formatMessage(messages.groupFormNameLabel)}
+              help={intl.formatMessage(messages.groupFormNameHelp)}
+              className={`${formGroupClasses} mt-2.5`}
+            />
+            <FormikControl
+              name={`${fieldNameCommonBase}.description`}
+              value={group.description}
+              floatingLabel={intl.formatMessage(messages.groupFormDescriptionLabel)}
+              help={intl.formatMessage(messages.groupFormDescriptionHelp)}
+              as="textarea"
+              rows={4}
+              style={{ minHeight: '2.5rem' }}
+              className={formGroupClasses}
+            />
             <Form.Group className={formGroupClasses}>
               <Form.Label className="h4 my-3">
                 {intl.formatMessage(messages.groupFormTypeLabel)}
@@ -135,22 +119,16 @@ function GroupEditor({
                 ))}
               </Form.RadioSet>
             </Form.Group>
-            <Form.Group className="mx-2">
-              <Form.Label className="h4 pb-4">{intl.formatMessage(messages.teamSize)}</Form.Label>
-              <Form.Control
-                type="number"
-                name={`${fieldNameCommonBase}.maxTeamSize`}
-                floatingLabel={intl.formatMessage(messages.groupFormMaxSizeLabel)}
-                value={group.maxTeamSize}
-                placeholder={TeamSizes.DEFAULT}
-                onChange={onChange}
-                onBlur={onBlur}
-                onFocus={handleFocus}
-              />
-              <FormikErrorFeedback name={`${fieldNameCommonBase}.maxTeamSize`}>
-                <Form.Text>{intl.formatMessage(messages.groupFormMaxSizeHelp)}</Form.Text>
-              </FormikErrorFeedback>
-            </Form.Group>
+            <FormikControl
+              type="number"
+              name={`${fieldNameCommonBase}.maxTeamSize`}
+              floatingLabel={intl.formatMessage(messages.groupFormMaxSizeLabel)}
+              value={group.maxTeamSize}
+              help={intl.formatMessage(messages.groupFormMaxSizeHelp)}
+              label={<Form.Label className="h4 pb-4">{intl.formatMessage(messages.teamSize)}</Form.Label>}
+              className="mx-2"
+              placeholder={TeamSizes.DEFAULT}
+            />
           </CollapsableEditor>
         )}
     </TransitionReplace>
@@ -177,7 +155,6 @@ GroupEditor.propTypes = {
   onDelete: PropTypes.func.isRequired,
   onChange: PropTypes.func.isRequired,
   onBlur: PropTypes.func.isRequired,
-  setFieldError: PropTypes.func.isRequired,
 };
 
 GroupEditor.defaultProps = {
diff --git a/src/pages-and-resources/teams/Settings.jsx b/src/pages-and-resources/teams/Settings.jsx
index 22186647d..32313c311 100644
--- a/src/pages-and-resources/teams/Settings.jsx
+++ b/src/pages-and-resources/teams/Settings.jsx
@@ -8,8 +8,7 @@ import React from 'react';
 import { v4 as uuid } from 'uuid';
 import * as Yup from 'yup';
 import { GroupTypes, TeamSizes } from '../../data/constants';
-
-import FormikErrorFeedback from '../../generic/FormikErrorFeedback';
+import FormikControl from '../../generic/FormikControl';
 import { setupYupExtensions, useAppSetting } from '../../utils';
 import AppSettingsModal from '../app-settings-modal/AppSettingsModal';
 import GroupEditor from './GroupEditor';
@@ -26,7 +25,7 @@ function TeamSettings({
     name: '',
     description: '',
     type: GroupTypes.OPEN,
-    maxTeamSize: TeamSizes.DEFAULT,
+    maxTeamSize: null,
     id: null,
     key: uuid(),
   };
@@ -90,6 +89,11 @@ function TeamSettings({
               .default(null),
           }),
         )
+          .when('enabled', {
+            is: true,
+            then: Yup.array().min(1),
+            otherwise: Yup.array().length(0),
+          })
           .default([])
           .uniqueProperty('name', intl.formatMessage(messages.groupFormNameExists)),
       }}
@@ -98,25 +102,18 @@ function TeamSettings({
     >
       {
         ({
-          handleChange, handleBlur, values, errors, setFieldError,
+          handleChange, handleBlur, values, errors,
         }) => (
           <>
             <h4 className="my-3 pb-2">{intl.formatMessage(messages.teamSize)}</h4>
-            <Form.Group className="pb-1">
-              <Form.Control
-                className="pb-2"
-                type="number"
-                name="maxTeamSize"
-                floatingLabel={intl.formatMessage(messages.maxTeamSize)}
-                onChange={handleChange}
-                onBlur={handleBlur}
-                value={values.maxTeamSize}
-                onFocus={(event) => setFieldError(event.target.name, undefined)}
-              />
-              <FormikErrorFeedback name="maxTeamSize">
-                <Form.Text>{intl.formatMessage(messages.maxTeamSizeHelp)}</Form.Text>
-              </FormikErrorFeedback>
-            </Form.Group>
+            <FormikControl
+              name="maxTeamSize"
+              value={values.maxTeamSize}
+              floatingLabel={intl.formatMessage(messages.maxTeamSize)}
+              help={intl.formatMessage(messages.maxTeamSizeHelp)}
+              className="pb-1"
+              type="number"
+            />
             <div className="bg-light-200 d-flex flex-column mx-n4 px-4 py-4 border border-top mb-n3.5">
               <h4>{intl.formatMessage(messages.groups)}</h4>
               <Form.Text className="mb-3">{intl.formatMessage(messages.groupsHelp)}</Form.Text>
@@ -132,7 +129,6 @@ function TeamSettings({
                         onDelete={() => remove(index)}
                         onChange={handleChange}
                         onBlur={handleBlur}
-                        setFieldError={setFieldError}
                       />
                     ))}
                     <Button
diff --git a/src/pages-and-resources/teams/messages.js b/src/pages-and-resources/teams/messages.js
index 15c3445a7..4842d6d26 100644
--- a/src/pages-and-resources/teams/messages.js
+++ b/src/pages-and-resources/teams/messages.js
@@ -11,7 +11,7 @@ const messages = defineMessages({
   },
   enableTeamsHelp: {
     id: 'authoring.pagesAndResources.teams.enableTeams.help',
-    defaultMessage: 'Allow learners to work in small groups on specific projects or activities.',
+    defaultMessage: 'Allow learners to work together on specific projects or activities.',
   },
   enableTeamsLink: {
     id: 'authoring.pagesAndResources.teams.enableTeams.link',
@@ -39,7 +39,7 @@ const messages = defineMessages({
   },
   maxTeamSizeTooHigh: {
     id: 'authoring.pagesAndResources.teams.teamSize.maxTeamSizeTooHigh',
-    defaultMessage: 'Max team size cannot be great than {max}',
+    defaultMessage: 'Max team size cannot be greater than {max}',
   },
   groups: {
     id: 'authoring.pagesAndResources.teams.groups.heading',
@@ -47,7 +47,7 @@ const messages = defineMessages({
   },
   groupsHelp: {
     id: 'authoring.pagesAndResources.teams.groups.help',
-    defaultMessage: 'Groups are spaced where learners can create or join teams.',
+    defaultMessage: 'Groups are spaces where learners can create or join teams.',
   },
   configureGroup: {
     id: 'authoring.pagesAndResources.teams.configureGroup.heading',
@@ -63,7 +63,7 @@ const messages = defineMessages({
   },
   groupFormNameEmpty: {
     id: 'authoring.pagesAndResources.teams.group.name.error.empty',
-    defaultMessage: 'Enter a unique name for your group',
+    defaultMessage: 'Enter a unique name for this group',
   },
   groupFormNameExists: {
     id: 'authoring.pagesAndResources.teams.group.name.error.exists',
@@ -79,7 +79,7 @@ const messages = defineMessages({
   },
   groupFormDescriptionError: {
     id: 'authoring.pagesAndResources.teams.group.description.error',
-    defaultMessage: 'Enter a description for your group',
+    defaultMessage: 'Enter a description for this group',
   },
   groupFormTypeLabel: {
     id: 'authoring.pagesAndResources.teams.group.type.label',
@@ -151,7 +151,10 @@ const messages = defineMessages({
   },
   groupDeleteBody: {
     id: 'authoring.pagesAndResources.teams.deleteGroup.body',
-    defaultMessage: 'edX recommends that you do not delete groups once your course is running.',
+    defaultMessage: `edX recommends that you do not delete groups once your course is running.\n
+    Your group will no longer be visible in the LMS and learners will not be able to leave teams associated with it.\n
+    Please delete learners from teams before deleting the associated group.`,
+    description: 'Message displayed to admins when deleting a group. Make sure to include the \\n line breaks so that the final text is rendered properly.',
   },
 });