fix: UX feedback suggestions from TNL-8730 [BD-38] [BB-4981] (#201)

* fix: UX feedback suggestions from TNL-8730
2021-10-21 12:50:36 +05:30
parent d6fda14d36
commit 9fd6cbf61b
6 changed files with 130 additions and 92 deletions
--- 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>
--- a/src/generic/FormikControl.jsx
+++ 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;
--- 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>
--- 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 = {
--- 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
--- 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.',
  },
 });