From a599a7dc485b8c4110093b1e93f9e51a1a37293b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 9 Feb 2024 16:41:01 +0100 Subject: [PATCH] Link content validation errors with corresponding inputs (#6042) --- .../frontend_apps/components/BookSelector.tsx | 6 ++- .../frontend_apps/components/ErrorModal.tsx | 4 +- .../frontend_apps/components/JSTORPicker.tsx | 9 ++++- .../components/URLFormWithPreview.tsx | 6 ++- .../frontend_apps/components/URLPicker.tsx | 10 ++++- package.json | 2 +- yarn.lock | 40 +++++++++++++++---- 7 files changed, 60 insertions(+), 17 deletions(-) diff --git a/lms/static/scripts/frontend_apps/components/BookSelector.tsx b/lms/static/scripts/frontend_apps/components/BookSelector.tsx index b28b118976..bec54fa100 100644 --- a/lms/static/scripts/frontend_apps/components/BookSelector.tsx +++ b/lms/static/scripts/frontend_apps/components/BookSelector.tsx @@ -6,7 +6,7 @@ import { Thumbnail, } from '@hypothesis/frontend-shared'; import classNames from 'classnames'; -import { useEffect, useRef, useState } from 'preact/hooks'; +import { useEffect, useId, useRef, useState } from 'preact/hooks'; import type { Book } from '../api-types'; import { formatErrorMessage } from '../errors'; @@ -46,6 +46,7 @@ export default function BookSelector({ const vsService = useService(VitalSourceService); const inputRef = useRef(null); + const errorId = useId(); // is a request in-flight via the vitalsource service? const [isLoadingBook, setIsLoadingBook] = useState(false); @@ -183,6 +184,7 @@ export default function BookSelector({ placeholder="e.g. https://bookshelf.vitalsource.com/#/books/012345678..." readOnly={isLoadingBook} spellcheck={false} + aria-describedby={errorId} /> + {error} )} diff --git a/lms/static/scripts/frontend_apps/components/ErrorModal.tsx b/lms/static/scripts/frontend_apps/components/ErrorModal.tsx index 58d28e83c6..e34609a79a 100644 --- a/lms/static/scripts/frontend_apps/components/ErrorModal.tsx +++ b/lms/static/scripts/frontend_apps/components/ErrorModal.tsx @@ -1,5 +1,5 @@ import { Button, ModalDialog } from '@hypothesis/frontend-shared'; -import type { ModalDialogProps } from '@hypothesis/frontend-shared/lib/components/feedback/ModalDialog'; +import type { PanelModalDialogProps } from '@hypothesis/frontend-shared/lib/components/feedback/ModalDialog'; import type { ComponentChildren } from 'preact'; import { useRef } from 'preact/hooks'; @@ -61,7 +61,7 @@ type ErrorModalBaseProps = { }; /** `title` is optional for this component but required by `Modal` */ -export type ErrorModalProps = Omit & +export type ErrorModalProps = Omit & ErrorModalBaseProps; /** diff --git a/lms/static/scripts/frontend_apps/components/JSTORPicker.tsx b/lms/static/scripts/frontend_apps/components/JSTORPicker.tsx index 06e2999551..faa61a4a3e 100644 --- a/lms/static/scripts/frontend_apps/components/JSTORPicker.tsx +++ b/lms/static/scripts/frontend_apps/components/JSTORPicker.tsx @@ -9,7 +9,7 @@ import { Thumbnail, } from '@hypothesis/frontend-shared'; import classnames from 'classnames'; -import { useRef, useState } from 'preact/hooks'; +import { useId, useRef, useState } from 'preact/hooks'; import type { JSTORMetadata, JSTORThumbnail } from '../api-types'; import { formatErrorMessage } from '../errors'; @@ -41,6 +41,7 @@ export default function JSTORPicker({ onSelectURL, }: JSTORPickerProps) { const [error, setError] = useState(null); + const errorId = useId(); // Selected JSTOR article ID or DOI, updated when the user confirms what // they have pasted/typed into the input field. @@ -189,11 +190,13 @@ export default function JSTORPicker({ elementRef={inputRef} id={inputId} name="jstorURL" + feedback={renderedError ? 'error' : undefined} onChange={() => onURLChange()} onInput={() => resetCurrentArticle()} onKeyDown={onKeyDown} placeholder="e.g. https://www.jstor.org/stable/1234" spellcheck={false} + aria-describedby={errorId} /> {renderedError} + + {renderedError} + )} diff --git a/lms/static/scripts/frontend_apps/components/URLFormWithPreview.tsx b/lms/static/scripts/frontend_apps/components/URLFormWithPreview.tsx index cf6be5ba64..9e8343b672 100644 --- a/lms/static/scripts/frontend_apps/components/URLFormWithPreview.tsx +++ b/lms/static/scripts/frontend_apps/components/URLFormWithPreview.tsx @@ -7,6 +7,7 @@ import { } from '@hypothesis/frontend-shared'; import classnames from 'classnames'; import type { ComponentChildren, RefObject } from 'preact'; +import { useId } from 'preact/hooks'; import { useUniqueId } from '../utils/hooks'; import UIMessage from './UIMessage'; @@ -52,6 +53,7 @@ export default function URLFormWithPreview({ }: URLFormWithPreviewProps) { const orientation = thumbnail?.orientation ?? 'square'; const inputId = useUniqueId('url'); + const errorId = useId(); const onChange = () => onURLChange(inputRef.current!.value); const onKeyDown = (event: KeyboardEvent) => { if (event.key === 'Enter') { @@ -103,11 +105,13 @@ export default function URLFormWithPreview({ elementRef={inputRef} id={inputId} name="URL" + feedback={error ? 'error' : undefined} onChange={onChange} onKeyDown={onKeyDown} onInput={onInput} placeholder={urlPlaceholder} spellcheck={false} + aria-describedby={errorId} /> + {error} )} diff --git a/lms/static/scripts/frontend_apps/components/URLPicker.tsx b/lms/static/scripts/frontend_apps/components/URLPicker.tsx index c2e99c2996..d02c5a5ba6 100644 --- a/lms/static/scripts/frontend_apps/components/URLPicker.tsx +++ b/lms/static/scripts/frontend_apps/components/URLPicker.tsx @@ -1,5 +1,5 @@ import { Button, ModalDialog, Input } from '@hypothesis/frontend-shared'; -import { useRef, useState } from 'preact/hooks'; +import { useId, useRef, useState } from 'preact/hooks'; import { isYouTubeURL } from '../utils/youtube'; import UIMessage from './UIMessage'; @@ -31,6 +31,7 @@ export default function URLPicker({ // Holds an error message corresponding to client-side validation of the // input field const [error, setError] = useState(null); + const errorId = useId(); const submit = (event: Event) => { event.preventDefault(); @@ -99,12 +100,17 @@ export default function URLPicker({ name="url" placeholder="e.g. https://example.com/article.pdf" required + aria-describedby={errorId} /> {/** setting a height here "preserves space" for this error display * and prevents the dialog size from jumping when an error is rendered */}
- {!!error && {error}} + {!!error && ( + + {error} + + )}
diff --git a/package.json b/package.json index 60c410e50f..f2467ea630 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "@babel/preset-react": "^7.23.3", "@babel/preset-typescript": "^7.23.3", "@hypothesis/frontend-build": "^3.0.0", - "@hypothesis/frontend-shared": "^7.1.0", + "@hypothesis/frontend-shared": "^7.4.0", "@rollup/plugin-babel": "^6.0.4", "@rollup/plugin-commonjs": "^25.0.7", "@rollup/plugin-node-resolve": "^15.2.3", diff --git a/yarn.lock b/yarn.lock index 298e673a48..7b5eeecd9f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1968,15 +1968,15 @@ __metadata: languageName: node linkType: hard -"@hypothesis/frontend-shared@npm:^7.1.0": - version: 7.1.0 - resolution: "@hypothesis/frontend-shared@npm:7.1.0" +"@hypothesis/frontend-shared@npm:^7.4.0": + version: 7.4.0 + resolution: "@hypothesis/frontend-shared@npm:7.4.0" dependencies: highlight.js: ^11.6.0 - wouter-preact: ^2.10.0-alpha.1 + wouter-preact: ^3.0.0 peerDependencies: preact: ^10.4.0 - checksum: 3dc7b2c82df68d7b17fa166c8ac78ad8317924129b241207ee5ae91a9e2688bcac5be7262735c98e8ec64aaa2654cdcfa4dd1476191b67b3bb2e8b96174339e2 + checksum: 5744240cc37d0f2d30ff34f240cb95606c0f846fda8b52fba18d60ec5e402fde39629c3ff32593b36db3949a9556dc7c2f7985d2c281215d7877d87151d5b458 languageName: node linkType: hard @@ -7651,7 +7651,7 @@ __metadata: "@babel/preset-react": ^7.23.3 "@babel/preset-typescript": ^7.23.3 "@hypothesis/frontend-build": ^3.0.0 - "@hypothesis/frontend-shared": ^7.1.0 + "@hypothesis/frontend-shared": ^7.4.0 "@hypothesis/frontend-testing": ^1.2.0 "@rollup/plugin-babel": ^6.0.4 "@rollup/plugin-commonjs": ^25.0.7 @@ -8177,6 +8177,13 @@ __metadata: languageName: node linkType: hard +"mitt@npm:^3.0.1": + version: 3.0.1 + resolution: "mitt@npm:3.0.1" + checksum: b55a489ac9c2949ab166b7f060601d3b6d893a852515ae9eca4e11df01c013876df777ea109317622b5c1c60e8aae252558e33c8c94e14124db38f64a39614b1 + languageName: node + linkType: hard + "mixin-deep@npm:^1.2.0": version: 1.3.2 resolution: "mixin-deep@npm:1.3.2" @@ -9563,6 +9570,13 @@ __metadata: languageName: node linkType: hard +"regexparam@npm:^3.0.0": + version: 3.0.0 + resolution: "regexparam@npm:3.0.0" + checksum: c8649af1538ccc12b5c5d250525f61bd370227dce41f4fb908433a9651e18b7be21dd8f8518c322dd9ebd75f7caaaea4921e374c39a469c11d4f9d0c738043e0 + languageName: node + linkType: hard + "regexpu-core@npm:^5.1.0": version: 5.1.0 resolution: "regexpu-core@npm:5.1.0" @@ -11523,7 +11537,7 @@ __metadata: languageName: node linkType: hard -"wouter-preact@npm:^2.10.0-alpha.1, wouter-preact@npm:^2.11.0": +"wouter-preact@npm:^2.11.0": version: 2.11.0 resolution: "wouter-preact@npm:2.11.0" peerDependencies: @@ -11532,6 +11546,18 @@ __metadata: languageName: node linkType: hard +"wouter-preact@npm:^3.0.0": + version: 3.0.0 + resolution: "wouter-preact@npm:3.0.0" + dependencies: + mitt: ^3.0.1 + regexparam: ^3.0.0 + peerDependencies: + preact: ^10.0.0 + checksum: 253927b4dbf0906220f71f103f3934e649debc4a5f2c94c84e3097a9072c3c2d967b559421b626acb555437ab3aa416ed0faabedd2cf95fb3c61fdbb83370bca + languageName: node + linkType: hard + "wrap-ansi-cjs@npm:wrap-ansi@^7.0.0, wrap-ansi@npm:^7.0.0": version: 7.0.0 resolution: "wrap-ansi@npm:7.0.0"