From aa3420f8709d2e7b18ea26f57d853d553fa4e3ff Mon Sep 17 00:00:00 2001
From: Awais Ansari <79941147+awais-ansari@users.noreply.github.com>
Date: Mon, 2 Aug 2021 14:24:38 +0500
Subject: [PATCH] style: 1.4 content and style fixes  (#170)

* style: add spacing between footer and body content

* fix: TNL-8567 prevent card selection when click on show features

* style: TNL-8569 update page & resources margin for mobile view

* fix: TNL-8571 correct error state for blockout date field

* style: TNL-8569 set page&resources margins in mobile view

* style: TNL-8573 feature list should intersect with column

* style: TNL-8568 update font size and color for feature table

* fix: move formInvalid logic to legacy form and make it centralize
---
 src/CourseAuthoringPage.jsx                   | 10 ++-
 src/pages-and-resources/PagesAndResources.jsx |  4 +-
 .../app-settings-modal/AppSettingsModal.jsx   |  6 +-
 .../apps/legacy/LegacyConfigForm.jsx          | 20 ++++--
 .../apps/shared/BlackoutDatesField.jsx        | 66 +++++++++++--------
 .../discussion-topics/DiscussionTopics.jsx    | 10 +--
 .../discussions/app-list/AppCard.jsx          |  2 +-
 .../discussions/app-list/FeaturesList.jsx     |  3 +-
 .../discussions/app-list/FeaturesTable.jsx    |  1 +
 .../discussions/app-list/FeaturesTable.scss   |  4 ++
 src/pages-and-resources/pages/PageCard.jsx    | 16 +++--
 src/pages-and-resources/pages/PageCard.scss   |  9 +++
 src/utils.js                                  |  6 +-
 13 files changed, 97 insertions(+), 60 deletions(-)
 create mode 100644 src/pages-and-resources/discussions/app-list/FeaturesTable.scss
 create mode 100644 src/pages-and-resources/pages/PageCard.scss

diff --git a/src/CourseAuthoringPage.jsx b/src/CourseAuthoringPage.jsx
index d5e363344..17a076259 100644
--- a/src/CourseAuthoringPage.jsx
+++ b/src/CourseAuthoringPage.jsx
@@ -40,11 +40,17 @@ export default function CourseAuthoringPage({ courseId, children }) {
     />
   );
 
+  const AppFooter = () => (
+    <div className="mt-6">
+      <Footer />
+    </div>
+  );
+
   return (
     <div className="bg-light-200">
-      {inProgress ? <Loading /> : AppHeader()}
+      {inProgress ? <Loading /> : <AppHeader />}
       {children}
-      {!inProgress && <Footer />}
+      {!inProgress && <AppFooter />}
     </div>
   );
 }
diff --git a/src/pages-and-resources/PagesAndResources.jsx b/src/pages-and-resources/PagesAndResources.jsx
index c99e1409d..acdc3d6ea 100644
--- a/src/pages-and-resources/PagesAndResources.jsx
+++ b/src/pages-and-resources/PagesAndResources.jsx
@@ -39,8 +39,8 @@ function PagesAndResources({ courseId, intl }) {
 
   return (
     <PagesAndResourcesProvider courseId={courseId}>
-      <main className="container container-mw-md">
-        <div className="d-flex justify-content-between my-5 align-items-center">
+      <main className="container container-mw-md px-3">
+        <div className="d-flex justify-content-between my-4 my-md-5 align-items-center">
           <h3 className="m-0">{intl.formatMessage(messages.heading)}</h3>
           <Hyperlink
             destination={lmsCourseURL}
diff --git a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx b/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx
index 3a9633206..fcac1a146 100644
--- a/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx
+++ b/src/pages-and-resources/app-settings-modal/AppSettingsModal.jsx
@@ -65,8 +65,8 @@ function AppSettingsModal({
   const appInfo = useModel('courseApps', appId);
   const dispatch = useDispatch();
   const submitButtonState = updateSettingsRequestStatus === RequestStatus.IN_PROGRESS ? 'pending' : 'default';
-  const isTabletOrMobile = useIsMobile();
-  const modalVariant = isTabletOrMobile ? 'dark' : 'default';
+  const isMobile = useIsMobile();
+  const modalVariant = isMobile ? 'dark' : 'default';
 
   useEffect(() => {
     if (updateSettingsRequestStatus === RequestStatus.SUCCESSFUL) {
@@ -102,7 +102,7 @@ function AppSettingsModal({
       isOpen
       closeText={intl.formatMessage(messages.cancel)}
       dialogClassName="modal-dialog-centered modal-lg"
-      hasCloseButton={isTabletOrMobile}
+      hasCloseButton={isMobile}
       onClose={onClose}
       variant={modalVariant}
       isFullscreenOnMobile
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
index ab89ffe66..7451c9379 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
@@ -1,6 +1,7 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
 import { Card, Form } from '@edx/paragon';
+import { useDispatch } from 'react-redux';
 import { Formik } from 'formik';
 import * as Yup from 'yup';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
@@ -10,6 +11,7 @@ import AnonymousPostingFields from '../shared/AnonymousPostingFields';
 import DiscussionTopics from '../shared/discussion-topics/DiscussionTopics';
 import BlackoutDatesField, { blackoutDatesRegex } from '../shared/BlackoutDatesField';
 import LegacyConfigFormProvider from './LegacyConfigFormProvider';
+import { updateValidationStatus } from '../../../data/slice';
 
 import messages from '../shared/messages';
 import AppConfigFormDivider from '../shared/AppConfigFormDivider';
@@ -37,6 +39,13 @@ Yup.addMethod(Yup.object, 'uniqueProperty', function (propertyName, message) {
 function LegacyConfigForm({
   appConfig, onSubmit, formRef, intl, title,
 }) {
+  const [isFormInvalid, setIsFormInvalid] = useState(false);
+  const dispatch = useDispatch();
+
+  useEffect(() => {
+    dispatch(updateValidationStatus({ hasError: isFormInvalid }));
+  }, [isFormInvalid]);
+
   const [validDiscussionTopics, setValidDiscussionTopics] = useState(appConfig.discussionTopics);
   const legacyFormValidationSchema = Yup.object().shape({
     blackoutDates: Yup.string().matches(
@@ -79,6 +88,8 @@ function LegacyConfigForm({
           setValidDiscussionTopics,
           discussionTopicErrors,
         };
+        setIsFormInvalid(discussionTopicErrors.some((error) => error === true)
+          || Boolean(touched.blackoutDates && errors.blackoutDates));
 
         return (
           <LegacyConfigFormProvider value={contextValue}>
@@ -96,12 +107,7 @@ function LegacyConfigForm({
                 <AppConfigFormDivider thick />
                 <DivisionByGroupFields />
                 <AppConfigFormDivider thick />
-                <BlackoutDatesField
-                  errors={errors}
-                  onBlur={handleBlur}
-                  onChange={handleChange}
-                  values={values}
-                />
+                <BlackoutDatesField />
               </Form>
             </Card>
           </LegacyConfigFormProvider>
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx
index 0e39ea84f..b18e16141 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx
@@ -1,7 +1,7 @@
-import React from 'react';
-import PropTypes from 'prop-types';
+import React, { useState } from 'react';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
-import { Form } from '@edx/paragon';
+import { Form, TransitionReplace } from '@edx/paragon';
+import { useFormikContext } from 'formik';
 import messages from './messages';
 
 /**
@@ -68,49 +68,57 @@ import messages from './messages';
  */
 export const blackoutDatesRegex = /^\[(\[("[0-9]{4}-(0[1-9]|1[0-2])-[0-3][0-9](T([0-1][0-9]|2[0-3]):([0-5][0-9])){0,1}"),("[0-9]{4}-(0[1-9]|1[0-2])-[0-3][0-9](T([0-1][0-9]|2[0-3]):([0-5][0-9])){0,1}")\](,){0,1})*\]$/;
 
-function BlackoutDatesField({
-  onBlur,
-  onChange,
-  intl,
-  values,
-  errors,
-}) {
+const BlackoutDatesField = ({ intl }) => {
+  const [inFocus, setInFocus] = useState(false);
+  const {
+    handleChange, handleBlur, errors,
+    touched, values: appConfig,
+  } = useFormikContext();
+
+  const hasError = Boolean(touched.blackoutDates && errors.blackoutDates);
+
+  const handleFocusOut = (event) => {
+    handleBlur(event);
+    setInFocus(false);
+  };
+
   return (
     <>
       <h5 className="my-4 text-gray-500">{intl.formatMessage(messages.blackoutDates)}</h5>
       <Form.Group
         controlId="blackoutDates"
+        isInvalid={hasError && !inFocus}
+        className="m-2"
       >
         <Form.Control
-          value={values.blackoutDates}
-          onChange={onChange}
-          onBlur={onBlur}
-          className="mb-3"
+          value={appConfig.blackoutDates}
+          onChange={handleChange}
+          onBlur={(event) => handleFocusOut(event)}
+          className="mb-1"
           floatingLabel={intl.formatMessage(messages.blackoutDatesLabel)}
+          onFocus={() => setInFocus(true)}
         />
-        {errors.blackoutDates && (
-          <Form.Control.Feedback type="invalid">
-            {errors.blackoutDates}
-          </Form.Control.Feedback>
-        )}
-        <Form.Text muted>
+        <TransitionReplace key="blackoutDates">
+          {hasError && !inFocus ? (
+            <React.Fragment key="open">
+              <Form.Control.Feedback type="invalid" hasIcon={false}>
+                <div className="small">{errors.blackoutDates}</div>
+              </Form.Control.Feedback>
+            </React.Fragment>
+          ) : (
+            <React.Fragment key="closed" />
+          )}
+        </TransitionReplace>
+        <Form.Text muted className="mt-3">
           {intl.formatMessage(messages.blackoutDatesHelp)}
         </Form.Text>
       </Form.Group>
     </>
   );
-}
+};
 
 BlackoutDatesField.propTypes = {
-  onBlur: PropTypes.func.isRequired,
-  onChange: PropTypes.func.isRequired,
   intl: intlShape.isRequired,
-  values: PropTypes.shape({
-    blackoutDates: PropTypes.string,
-  }).isRequired,
-  errors: PropTypes.shape({
-    blackoutDates: PropTypes.string,
-  }).isRequired,
 };
 
 export default injectIntl(BlackoutDatesField);
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx
index 06dfb08ed..a17337f7b 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx
@@ -1,5 +1,4 @@
-import React, { useEffect, useContext, useCallback } from 'react';
-import { useDispatch } from 'react-redux';
+import React, { useContext, useCallback } from 'react';
 import { Add } from '@edx/paragon/icons';
 import { Button } from '@edx/paragon';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
@@ -8,7 +7,6 @@ import { v4 as uuid } from 'uuid';
 import _ from 'lodash';
 import messages from '../messages';
 import TopicItem from './TopicItem';
-import { updateValidationStatus } from '../../../../data/slice';
 import { LegacyConfigFormContext } from '../../legacy/LegacyConfigFormProvider';
 import filterItemFromObject from '../../../utils';
 
@@ -19,18 +17,12 @@ const DiscussionTopics = ({ intl }) => {
     setFieldValue,
   } = useFormikContext();
   const { discussionTopics, divideDiscussionIds } = appConfig;
-  const dispatch = useDispatch();
   const {
     discussionTopicErrors,
     validDiscussionTopics,
     setValidDiscussionTopics,
   } = useContext(LegacyConfigFormContext);
 
-  const isFormInvalid = discussionTopicErrors.some((error) => error === true);
-  useEffect(() => {
-    dispatch(updateValidationStatus({ hasError: isFormInvalid }));
-  }, [isFormInvalid]);
-
   const handleTopicDelete = async (topicIndex, topicId, remove) => {
     await remove(topicIndex);
     validateForm();
diff --git a/src/pages-and-resources/discussions/app-list/AppCard.jsx b/src/pages-and-resources/discussions/app-list/AppCard.jsx
index c93a79b98..2a92c5732 100644
--- a/src/pages-and-resources/discussions/app-list/AppCard.jsx
+++ b/src/pages-and-resources/discussions/app-list/AppCard.jsx
@@ -45,7 +45,7 @@ function AppCard({
           })}
         />
       </div>
-      <Card.Body className="m-2">
+      <Card.Body>
         <div className="h4 card-title">
           {intl.formatMessage(messages[`appName-${app.id}`])}
         </div>
diff --git a/src/pages-and-resources/discussions/app-list/FeaturesList.jsx b/src/pages-and-resources/discussions/app-list/FeaturesList.jsx
index 412732637..9f2f7b932 100644
--- a/src/pages-and-resources/discussions/app-list/FeaturesList.jsx
+++ b/src/pages-and-resources/discussions/app-list/FeaturesList.jsx
@@ -19,6 +19,7 @@ const NonSupportedFeature = (
 function FeaturesList({ app, features, intl }) {
   return (
     <Collapsible
+      onClick={(event) => event.stopPropagation()}
       title={(
         <>
           <Collapsible.Visible whenClosed>
@@ -32,7 +33,7 @@ function FeaturesList({ app, features, intl }) {
       styling="basic"
     >
       {features && features.map((feature) => (
-        <div key={`collapsible-${app.id}&${feature.id}`}>
+        <div key={`collapsible-${app.id}&${feature.id}`} className="d-flex mb-1">
           {app.featureIds.includes(feature.id)
             ? SupportedFeature
             : NonSupportedFeature}
diff --git a/src/pages-and-resources/discussions/app-list/FeaturesTable.jsx b/src/pages-and-resources/discussions/app-list/FeaturesTable.jsx
index f2731ef43..cc14d3a15 100644
--- a/src/pages-and-resources/discussions/app-list/FeaturesTable.jsx
+++ b/src/pages-and-resources/discussions/app-list/FeaturesTable.jsx
@@ -4,6 +4,7 @@ import { Remove, Check } from '@edx/paragon/icons';
 import { DataTable } from '@edx/paragon';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 import messages from './messages';
+import './FeaturesTable.scss';
 
 function FeaturesTable({ apps, features, intl }) {
   return (
diff --git a/src/pages-and-resources/discussions/app-list/FeaturesTable.scss b/src/pages-and-resources/discussions/app-list/FeaturesTable.scss
new file mode 100644
index 000000000..5f3417e12
--- /dev/null
+++ b/src/pages-and-resources/discussions/app-list/FeaturesTable.scss
@@ -0,0 +1,4 @@
+table {
+  font-size: 14px;
+  color: black;
+}
diff --git a/src/pages-and-resources/pages/PageCard.jsx b/src/pages-and-resources/pages/PageCard.jsx
index b0f5c8a13..c5992b28e 100644
--- a/src/pages-and-resources/pages/PageCard.jsx
+++ b/src/pages-and-resources/pages/PageCard.jsx
@@ -1,4 +1,5 @@
 import { history } from '@edx/frontend-platform';
+import classNames from 'classnames';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 import {
   Badge, Card, Icon, IconButton, Hyperlink,
@@ -8,6 +9,8 @@ import PropTypes from 'prop-types';
 import React, { useContext } from 'react';
 import messages from '../messages';
 import { PagesAndResourcesContext } from '../PagesAndResourcesProvider';
+import { useIsDesktop } from '../../utils';
+import './PageCard.scss';
 
 const CoursePageShape = PropTypes.shape({
   id: PropTypes.string.isRequired,
@@ -27,6 +30,7 @@ function PageCard({
   page,
 }) {
   const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext);
+  const isDesktop = useIsDesktop();
 
   const SettingsButton = () => {
     if (page.legacyLink) {
@@ -55,11 +59,13 @@ function PageCard({
 
   return (
     <Card
-      className="shadow card"
-      style={{
-        width: '19rem',
-        height: '14rem',
-      }}
+      className={classNames(
+        'shadow card',
+        {
+          'desktop-card': isDesktop,
+          'mobile-card': !isDesktop,
+        },
+      )}
     >
       <Card.Body className="d-flex flex-column justify-content-between">
         <Card.Title className="d-flex mb-0 align-items-center justify-content-between">
diff --git a/src/pages-and-resources/pages/PageCard.scss b/src/pages-and-resources/pages/PageCard.scss
new file mode 100644
index 000000000..457fbc7c7
--- /dev/null
+++ b/src/pages-and-resources/pages/PageCard.scss
@@ -0,0 +1,9 @@
+.desktop-card {
+  width: 19rem;
+  height: 14rem;
+}
+
+.mobile-card {
+  width: 100%;
+  height: 14rem;
+}
diff --git a/src/utils.js b/src/utils.js
index 35c607e16..24aa4a05b 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -6,5 +6,9 @@ export const executeThunk = async (thunk, dispatch, getState) => {
 };
 
 export function useIsMobile() {
-  return useMediaQuery({ query: '(max-width: 768px)' });
+  return useMediaQuery({ query: '(max-width: 767.98px)' });
+}
+
+export function useIsDesktop() {
+  return useMediaQuery({ query: '(min-width: 992px)' });
 }