From 3dcb5a2889ef1b35cfc947a008bdfb3a5ee31829 Mon Sep 17 00:00:00 2001 From: David Joy Date: Wed, 7 Apr 2021 16:29:00 -0400 Subject: [PATCH] Update step names, form titles, and add dividers (#75) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: adjusting discussion selection step 1 message Removing a duplicate message too. * fix: adding a dividing line between all sections of the app config forms The line is its own component because it has some custom styling to make it go edge-to-edge. There’s a known issue here where a divider inside a TransitionReplace component won’t show its full width until the transition is finished. No idea how to fix that. * fix: moving app titles inside the config forms. Also moving the Card styling in too, since it sorta goes with the dividers and the overall look of the specific forms moreso than the parent. * fix: allowing dividers to be thicker between sections --- .../discussions/DiscussionsSettings.jsx | 2 +- .../app-config-form/AppConfigForm.jsx | 13 +- .../apps/legacy/LegacyConfigForm.jsx | 52 ++++---- .../apps/lti/LtiConfigForm.jsx | 112 +++++++++--------- .../apps/shared/AnonymousPostingFields.jsx | 2 + .../apps/shared/AppConfigFormDivider.jsx | 23 ++++ .../apps/shared/DivisionByGroupFields.jsx | 3 + .../apps/shared/InContextDiscussionFields.jsx | 5 + .../discussions/app-config-form/messages.js | 15 ++- .../discussions/app-list/messages.js | 10 +- .../discussions/messages.js | 6 +- 11 files changed, 145 insertions(+), 98 deletions(-) create mode 100644 src/pages-and-resources/discussions/app-config-form/apps/shared/AppConfigFormDivider.jsx diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.jsx index 5a0226adc..61fd7b8a3 100644 --- a/src/pages-and-resources/discussions/DiscussionsSettings.jsx +++ b/src/pages-and-resources/discussions/DiscussionsSettings.jsx @@ -31,7 +31,7 @@ function DiscussionsSettings({ courseId, intl }) { const isFirstStep = pathname === discussionsPath; const steps = [{ - label: intl.formatMessage(messages.selectDiscussionTool), + label: intl.formatMessage(messages.providerSelection), iconLabel: isFirstStep ? undefined : ( ), diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx index 4528ef97c..f59ddc75f 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx @@ -4,7 +4,7 @@ import React, { import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; import { useRouteMatch } from 'react-router'; -import { Card, Container } from '@edx/paragon'; +import { Container } from '@edx/paragon'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { history } from '@edx/frontend-platform'; import { useModel } from '../../../generic/model-store'; @@ -52,6 +52,7 @@ function AppConfigForm({ formRef={formRef} appConfig={appConfig} onSubmit={handleSubmit} + title={intl.formatMessage(messages[`appName-${app.id}`])} /> ); } else { @@ -61,17 +62,13 @@ function AppConfigForm({ app={app} appConfig={appConfig} onSubmit={handleSubmit} + title={intl.formatMessage(messages[`appName-${app.id}`])} /> ); } return ( - -

- {intl.formatMessage(messages.configureApp, { name: app.name })} -

- - {form} - + + {form} ); } 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 6a96c8dc2..c9911cf93 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,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { Form } from '@edx/paragon'; +import { Card, Form } from '@edx/paragon'; import { useFormik } from 'formik'; import * as Yup from 'yup'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; @@ -10,9 +10,10 @@ import AnonymousPostingFields from '../shared/AnonymousPostingFields'; import BlackoutDatesField, { blackoutDatesRegex } from '../shared/BlackoutDatesField'; import messages from '../shared/messages'; +import AppConfigFormDivider from '../shared/AppConfigFormDivider'; function LegacyConfigForm({ - appConfig, onSubmit, formRef, intl, + appConfig, onSubmit, formRef, intl, title, }) { const { handleSubmit, @@ -31,29 +32,31 @@ function LegacyConfigForm({ onSubmit, }); - const divider =
; - return ( -
-

Legacy Discussions

- - {divider} - - - + +
+

{title}

+ + + + + + + +
); } @@ -72,6 +75,7 @@ LegacyConfigForm.propTypes = { // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, intl: intlShape.isRequired, + title: PropTypes.string.isRequired, }; export default injectIntl(LegacyConfigForm); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index 312fa29dc..c753123f8 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -1,14 +1,15 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { Form, Hyperlink } from '@edx/paragon'; +import { Card, Form, Hyperlink } from '@edx/paragon'; import { useFormik } from 'formik'; import * as Yup from 'yup'; import messages from './messages'; +import AppConfigFormDivider from '../shared/AppConfigFormDivider'; function LtiConfigForm({ - appConfig, app, onSubmit, intl, formRef, + appConfig, app, onSubmit, intl, formRef, title, }) { const { handleSubmit, @@ -27,66 +28,70 @@ function LtiConfigForm({ }); return ( -
-

- - {intl.formatMessage(messages.documentationPage, { name: app.name })} - - ), - name: app.name, - }} - /> -

- - {intl.formatMessage(messages.consumerKey)} - - {errors.consumerKey && ( + + +

{title}

+ +

+ + {intl.formatMessage(messages.documentationPage, { name: app.name })} + + ), + name: app.name, + }} + /> +

+ + {intl.formatMessage(messages.consumerKey)} + + {errors.consumerKey && ( {errors.consumerKey} - )} - - - {intl.formatMessage(messages.consumerSecret)} - - {errors.consumerSecret && ( + )} + + + {intl.formatMessage(messages.consumerSecret)} + + {errors.consumerSecret && ( {errors.consumerSecret} - )} - - - {intl.formatMessage(messages.launchUrl)} - - {errors.launchUrl && ( + )} + + + {intl.formatMessage(messages.launchUrl)} + + {errors.launchUrl && ( {errors.launchUrl} - )} - - + )} +
+ + ); } @@ -105,6 +110,7 @@ LtiConfigForm.propTypes = { onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, + title: PropTypes.string.isRequired, }; export default injectIntl(LtiConfigForm); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx index 3e7781529..4317fec06 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx @@ -4,6 +4,7 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { TransitionReplace } from '@edx/paragon'; import FormSwitchGroup from '../../../../../generic/FormSwitchGroup'; import messages from './messages'; +import AppConfigFormDivider from './AppConfigFormDivider'; function AnonymousPostingFields({ onBlur, @@ -25,6 +26,7 @@ function AnonymousPostingFields({ {values.allowAnonymousPosts ? ( + + ); +} + +AppConfigFormDivider.propTypes = { + thick: PropTypes.bool, +}; + +AppConfigFormDivider.defaultProps = { + thick: false, +}; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx index e59cea46b..55e5b3ab0 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx @@ -4,6 +4,7 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Form, TransitionReplace } from '@edx/paragon'; import FormSwitchGroup from '../../../../../generic/FormSwitchGroup'; import messages from './messages'; +import AppConfigFormDivider from './AppConfigFormDivider'; function DivisionByGroupFields({ onBlur, @@ -26,6 +27,7 @@ function DivisionByGroupFields({ {values.divideByCohorts ? ( + + + {values.inContextDiscussion ? ( @@ -34,6 +36,7 @@ function InContextDiscussionFields({ label={intl.formatMessage(messages.gradedUnitPagesLabel)} helpText={intl.formatMessage(messages.gradedUnitPagesHelp)} /> + + + ) : } diff --git a/src/pages-and-resources/discussions/app-config-form/messages.js b/src/pages-and-resources/discussions/app-config-form/messages.js index 7c2103656..c94857855 100644 --- a/src/pages-and-resources/discussions/app-config-form/messages.js +++ b/src/pages-and-resources/discussions/app-config-form/messages.js @@ -29,10 +29,17 @@ const messages = defineMessages({ defaultMessage: 'Applied', description: 'Button label when the discussion configuration has been successfully submitted.', }, - selectDiscussionTool: { - id: 'authoring.discussions.selectDiscussionTool', - defaultMessage: 'Select discussion tool', - description: 'A label for the first step of a wizard where the user chooses a discussion tool to configure.', + + // App names + 'appName-piazza': { + id: 'authoring.discussions.appConfigForm.appName-piazza', + defaultMessage: 'Piazza', + description: 'The name of the Piazza app.', + }, + 'appName-legacy': { + id: 'authoring.discussions.appConfigForm.appName-legacy', + defaultMessage: 'edX Discussions', + description: 'The name of the Legacy edX Discussions app.', }, }); diff --git a/src/pages-and-resources/discussions/app-list/messages.js b/src/pages-and-resources/discussions/app-list/messages.js index 45711f9db..11752970f 100644 --- a/src/pages-and-resources/discussions/app-list/messages.js +++ b/src/pages-and-resources/discussions/app-list/messages.js @@ -32,23 +32,23 @@ const messages = defineMessages({ // Legacy 'appName-legacy': { - id: 'authoring.discussions.appName-legacy', - defaultMessage: 'Legacy edX Discussions', + id: 'authoring.discussions.appList.appName-legacy', + defaultMessage: 'edX Discussions', description: 'The name of the Legacy edX Discussions app.', }, 'appDescription-legacy': { - id: 'authoring.discussions.appDescription-legacy', + id: 'authoring.discussions.appList.appDescription-legacy', defaultMessage: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', description: 'A description of the Legacy edX Discussions app.', }, // Piazza 'appName-piazza': { - id: 'authoring.discussions.appName-piazza', + id: 'authoring.discussions.appList.appName-piazza', defaultMessage: 'Piazza', description: 'The name of the Piazza app.', }, 'appDescription-piazza': { - id: 'authoring.discussions.appDescription-piazza', + id: 'authoring.discussions.appList.appDescription-piazza', defaultMessage: 'Piazza is designed to connect students, TAs, and professors so every student can get the help they need when they need it.', description: 'A description of the Piazza app.', }, diff --git a/src/pages-and-resources/discussions/messages.js b/src/pages-and-resources/discussions/messages.js index fe64c112d..b7ea0d55c 100644 --- a/src/pages-and-resources/discussions/messages.js +++ b/src/pages-and-resources/discussions/messages.js @@ -34,9 +34,9 @@ const messages = defineMessages({ defaultMessage: 'Applied', description: 'Button label when the discussion configuration has been successfully submitted.', }, - selectDiscussionTool: { - id: 'authoring.discussions.selectDiscussionTool', - defaultMessage: 'Select discussion tool', + providerSelection: { + id: 'authoring.discussions.providerSelection', + defaultMessage: 'Provider selection', description: 'A label for the first step of a wizard where the user chooses a discussion tool to configure.', }, });