From 8ca31db153e7c94d3884731faf09e7f1a26f9b6d Mon Sep 17 00:00:00 2001
From: Jure Triglav <juretriglav@gmail.com>
Date: Fri, 11 Sep 2020 11:56:54 +0200
Subject: [PATCH] feat: focus on errors in submission after submit
---
.../src/components/FormTemplate.js | 41 ++++-------
.../src/components/SubmitPage.js | 20 ++++--
.../src/components/ValidatedField.js | 69 +++++++++++++++++++
app/theme/elements/TextField.js | 21 ++++--
app/theme/index.js | 2 +-
cypress/integration/001-submission_spec.js | 16 +----
.../submission_with_errors_spec.js | 51 ++++++++++++++
7 files changed, 166 insertions(+), 54 deletions(-)
create mode 100644 app/components/component-submit/src/components/ValidatedField.js
create mode 100644 cypress/integration/submission_with_errors_spec.js
diff --git a/app/components/component-submit/src/components/FormTemplate.js b/app/components/component-submit/src/components/FormTemplate.js
index 1610b24e4e..b5620371a7 100644
--- a/app/components/component-submit/src/components/FormTemplate.js
+++ b/app/components/component-submit/src/components/FormTemplate.js
@@ -8,7 +8,6 @@ import {
CheckboxGroup,
Button,
Attachment,
- ValidatedFieldFormik,
} from '@pubsweet/ui'
import * as validators from 'xpub-validators'
import { AbstractEditor } from 'xpub-edit'
@@ -16,30 +15,9 @@ import { Section as Container, Select, FilesUpload } from '../../../shared'
import { Heading1, Section, Legend, SubNote } from '../style'
import AuthorsInput from './AuthorsInput'
import LinksInput from './LinksInput'
+import ValidatedFieldFormik from './ValidatedField'
import Confirm from './Confirm'
-// TODO: https://github.com/formium/formik/issues/146#issuecomment-474775723
-// const useFocusOnError = ({ fieldRef, name }) => {
-// const formik = useFormikContext()
-// const prevSubmitCountRef = React.useRef(formik.submitCount)
-// const firstErrorKey = Object.keys(formik.errors)[0]
-// React.useEffect(() => {
-// if (prevSubmitCountRef.current !== formik.submitCount && !formik.isValid) {
-// if (fieldRef.current && firstErrorKey === name) fieldRef.current.focus()
-// }
-// prevSubmitCountRef.current = formik.submitCount
-// }, [formik.submitCount, formik.isValid, firstErrorKey])
-// }
-
-// const Wrapper = styled.div`
-// font-family: ${th('fontInterface')};
-// line-height: 1.3;
-// margin: auto;
-// max-width: 60em;
-
-// overflow: ${({ confirming }) => confirming && 'hidden'};
-// `
-
const Intro = styled.div`
font-style: italic;
line-height: 1.4;
@@ -57,9 +35,6 @@ const ModalWrapper = styled.div`
top: 0;
`
-// Due to migration to new Data Model
-// Attachement component needs different data structure to work
-// needs to change the pubsweet ui Attachement to support the new Data Model
const filesToAttachment = file => ({
name: file.filename,
url: file.url,
@@ -347,7 +322,7 @@ export default ({
) : null}
{values.status !== 'submitted' && form.haspopup === 'false' && (
- <Button onClick={handleSubmit} primary type="submit">
+ <Button onClick={() => handleSubmit()} primary type="submit">
Submit your research object
</Button>
)}
@@ -355,7 +330,17 @@ export default ({
{values.status !== 'submitted' && form.haspopup === 'true' && (
<div>
<Button
- onClick={() => validateForm() && toggleConfirming()}
+ onClick={async () => {
+ const hasErrors = Object.keys(await validateForm()).length !== 0
+
+ // If there are errors, do a fake submit
+ // to focus on the error
+ if (hasErrors) {
+ handleSubmit()
+ } else {
+ toggleConfirming()
+ }
+ }}
primary
type="button"
>
diff --git a/app/components/component-submit/src/components/SubmitPage.js b/app/components/component-submit/src/components/SubmitPage.js
index 9012c140e8..da4fcb32c6 100644
--- a/app/components/component-submit/src/components/SubmitPage.js
+++ b/app/components/component-submit/src/components/SubmitPage.js
@@ -139,8 +139,18 @@ const SubmitPage = ({ match, history, ...props }) => {
if (error) return JSON.stringify(error)
const manuscript = data?.manuscript
- const getFile = data?.getFile
-
+ const form = data?.getFile
+
+ // Set the initial values based on the form
+ let initialValues = {}
+ const fieldNames = form.children.map(field => field.name)
+ fieldNames.forEach(fieldName => set(initialValues, fieldName, null))
+ initialValues = Object.assign({}, manuscript, {
+ submission: Object.assign(
+ initialValues.submission,
+ JSON.parse(manuscript.submission),
+ ),
+ })
const updateManuscript = input =>
update({
variables: {
@@ -176,9 +186,7 @@ const SubmitPage = ({ match, history, ...props }) => {
<Formik
displayName="submit"
handleChange={handleChange}
- initialValues={Object.assign({}, manuscript, {
- submission: JSON.parse(manuscript.submission),
- })}
+ initialValues={initialValues}
onSubmit={async (values, { validateForm, setSubmitting, ...other }) => {
// TODO: Change this to a more Formik idiomatic form
const isValid = Object.keys(await validateForm()).length === 0
@@ -188,7 +196,7 @@ const SubmitPage = ({ match, history, ...props }) => {
{props => (
<Submit
confirming={confirming}
- forms={cloneDeep(getFile)}
+ forms={cloneDeep(form)}
manuscript={manuscript}
onChange={handleChange}
toggleConfirming={toggleConfirming}
diff --git a/app/components/component-submit/src/components/ValidatedField.js b/app/components/component-submit/src/components/ValidatedField.js
new file mode 100644
index 0000000000..36fad0c651
--- /dev/null
+++ b/app/components/component-submit/src/components/ValidatedField.js
@@ -0,0 +1,69 @@
+import React from 'react'
+import { FastField, ErrorMessage, useFormikContext } from 'formik'
+import { get } from 'lodash'
+import styled from 'styled-components'
+import { th } from '@pubsweet/ui-toolkit'
+
+const MessageWrapper = styled.div`
+ font-family: ${th('fontInterface')};
+ display: flex;
+ color: ${th('colorError')};
+ &:not(:last-child) {
+ margin-bottom: ${th('gridUnit')};
+ }
+ font-size: ${th('fontSizeBaseSmall')};
+ line-height: ${th('lineHeightBaseSmall')};
+`
+
+// const ValidatedFieldComponent = ({ component: Component }) =>
+const useFocusOnError = ({ fieldRef, name }) => {
+ const formik = useFormikContext()
+ const prevSubmitCountRef = React.useRef(formik.submitCount)
+ let firstErrorKey = Object.keys(formik.errors)[0]
+ // If the error is in a nested object, we need to look deeper
+ // e.g. errors = { submission: { name: 'error' } }
+ const nestedError = formik.errors[firstErrorKey]
+ if (nestedError !== null && typeof nestedError === 'object') {
+ firstErrorKey = `${firstErrorKey}.${Object.keys(nestedError)[0]}` // e.g. submission.name
+ }
+ React.useEffect(() => {
+ if (prevSubmitCountRef.current !== formik.submitCount && !formik.isValid) {
+ if (fieldRef.current && firstErrorKey === name) fieldRef.current.focus()
+ }
+ prevSubmitCountRef.current = formik.submitCount
+ }, [formik.submitCount, formik.isValid, firstErrorKey])
+}
+
+const ValidatedField = ({ component: Component, ...props }) => {
+ // TODO: https://github.com/formium/formik/issues/146#issuecomment-474775723
+ const fieldRef = React.useRef()
+ const { name } = props
+ useFocusOnError({ fieldRef, name })
+
+ return (
+ <FastField {...props}>
+ {({ form: { errors, touched }, field, meta }) => {
+ let validationStatus
+ if (get(touched, name)) validationStatus = 'success'
+ if (get(touched, name) && get(errors, name)) validationStatus = 'error'
+ return (
+ <div>
+ <Component
+ {...field}
+ {...props}
+ innerRefProp={fieldRef}
+ validationStatus={validationStatus}
+ />
+
+ {/* live region DOM node must be initially present for changes to be announced */}
+ <MessageWrapper role="alert">
+ <ErrorMessage name={name} />
+ </MessageWrapper>
+ </div>
+ )
+ }}
+ </FastField>
+ )
+}
+
+export default ValidatedField
diff --git a/app/theme/elements/TextField.js b/app/theme/elements/TextField.js
index acf5abc04e..da8f4ac0ca 100644
--- a/app/theme/elements/TextField.js
+++ b/app/theme/elements/TextField.js
@@ -2,14 +2,11 @@ import { css } from 'styled-components'
import { th, grid } from '@pubsweet/ui-toolkit'
export default {
- // TODO
- // -- input padding: breaking the grid?
- // -- small placeholder text? maybe by default?
Input: css`
border-color: ${props => {
switch (props.validationStatus) {
case 'success':
- return props.theme.colorSuccess
+ return props.theme.colorBorder
case 'warning':
return props.theme.colorWarning
case 'error':
@@ -21,7 +18,7 @@ export default {
color: ${props => {
switch (props.validationStatus) {
case 'success':
- return props.theme.colorSuccess
+ return props.theme.colorText
case 'warning':
return props.theme.colorWarning
case 'error':
@@ -34,7 +31,19 @@ export default {
transition: ${th('transitionDuration')} ${th('transitionTimingFunction')};
&:focus {
- border-color: ${th('colorPrimary')};
+ box-shadow: ${th('boxShadow')};
+ border-color: ${props => {
+ switch (props.validationStatus) {
+ case 'success':
+ return props.theme.colorSuccess
+ case 'warning':
+ return props.theme.colorWarning
+ case 'error':
+ return props.theme.colorError
+ default:
+ return props.theme.colorPrimary
+ }
+ }};
color: inherit;
}
diff --git a/app/theme/index.js b/app/theme/index.js
index 9ecac132a2..1818f2e6cf 100644
--- a/app/theme/index.js
+++ b/app/theme/index.js
@@ -73,7 +73,7 @@ const cokoTheme = {
// $borderColor: var($colorFurniture);
/* Shadow (for tooltip) */
- boxShadow: '0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 2px 0 rgba(0, 0, 0, 0.06)',
+ boxShadow: '0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 4px 0 rgba(0, 0, 0, 0.06)',
// boxShadow: '4px 4px 16px #cdcdcd',
/* Transition */
transitionDuration: '0.2s', // TODO -- julien: not 0.05s
diff --git a/cypress/integration/001-submission_spec.js b/cypress/integration/001-submission_spec.js
index b43d1ea9e3..992c05fb40 100644
--- a/cypress/integration/001-submission_spec.js
+++ b/cypress/integration/001-submission_spec.js
@@ -1,18 +1,8 @@
-// const URLsubmission = {
-// title:
-// }
-const login = name => {
- cy.task('createToken', name).then(token => {
- cy.setToken(token)
- cy.visit('/journal/dashboard')
- })
-}
-
describe('URL submission test', () => {
it('can submit a URL and some metadata', () => {
cy.task('restore', 'initialState')
- login('Emily Clay')
+ cy.login('Emily Clay')
cy.get('button')
.contains('New submission')
@@ -143,7 +133,7 @@ describe('URL submission test', () => {
cy.task('restore', 'submission_complete')
// Admin logs in to assign senior editor
- login('Sinead Sullivan')
+ cy.login('Sinead Sullivan')
cy.get('nav')
.contains('Manuscripts')
.click()
@@ -157,7 +147,7 @@ describe('URL submission test', () => {
.contains('Joanne Pilger')
.click()
- login('Joanne Pilger')
+ cy.login('Joanne Pilger')
cy.contains('My URL submission')
cy.contains('Control Panel').click()
diff --git a/cypress/integration/submission_with_errors_spec.js b/cypress/integration/submission_with_errors_spec.js
new file mode 100644
index 0000000000..999a73f2d2
--- /dev/null
+++ b/cypress/integration/submission_with_errors_spec.js
@@ -0,0 +1,51 @@
+describe('Submission with errors test', () => {
+ it('can submit a URL and some metadata', () => {
+ cy.task('restore', 'initialState')
+
+ cy.login('Emily Clay')
+
+ cy.get('button')
+ .contains('New submission')
+ .click()
+ cy.get('button')
+ .contains('Submit a URL instead')
+ .click()
+
+ cy.get('body').contains('Submission created')
+
+ cy.get('button')
+ .contains('Submit your research object')
+ .click()
+
+ cy.get('body').contains('Enter at least 4 characters')
+
+ cy.get('[data-testid="submission.name"]')
+ .click()
+ .type('Emily Clay')
+
+ cy.get('button')
+ .contains('Submit your research object')
+ .click()
+
+ cy.get('body').contains('Enter at least 2 characters')
+
+ cy.get('[data-testid="submission.keywords"]')
+ .click()
+ .type('some, keywords')
+
+ // Change the title so that we can look for it
+ cy.get('input[data-testid="meta.title"]')
+ .click()
+ .clear()
+ .type('My Fixed URL Submission')
+
+ cy.get('button')
+ .contains('Submit your research object')
+ .click()
+ cy.get('button')
+ .contains('Submit your manuscript')
+ .click()
+
+ cy.get('body').contains('My Fixed URL Submission')
+ })
+})
--
GitLab