Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(WCAG): Associate field descriptions to inputs #4896

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export type PortableTextEditableProps = {
scrollSelectionIntoView?: ScrollSelectionIntoViewFunction
selection?: EditorSelection
spellCheck?: boolean
describedBy?: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we rather support arbitrary props forwarding here, as there are already quite a few props to this component?

That way we could send in our own HTML props whatever they might be, and don't have to be overly specific about props we don't really care about in this context.

So something like this:

      <SlateEditable
        {...props}
        autoFocus={false}
        className={props.className || 'pt-editable'}
        decorate={decorate}
        onBlur={handleOnBlur}
        onCopy={handleCopy}
        onDOMBeforeInput={handleOnBeforeInput}
        onFocus={handleOnFocus}
        onKeyDown={handleKeyDown}
        onPaste={handlePaste}
        readOnly={readOnly}
        renderElement={renderElement}
        renderLeaf={renderLeaf}
        scrollSelectionIntoView={scrollSelectionIntoViewToSlate}
      />

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be simpler, but the problem that I see doing that is that we are memoizing the component and we will need to listen to those changes in the useMemo.
If we add the props object to the useMemo, every re render will recalculate the useMemo again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we rather support arbitrary props forwarding here

For that to work we'll also have to make PortableTextEditableProps extend ComponentProps<typeof SlateEditable>, right? I don't have the full picture, but seems nice to keep the props interface minimal here? Guess it's also something we can always consider doing later too.

On a related note: Any reason to not name it aria-describedby here? That would help us avoid ambiguity if we decided to support arbitrary props forwarding some time in the future.

}

/**
Expand All @@ -91,6 +92,7 @@ export const PortableTextEditable = forwardRef(function PortableTextEditable(
selection: propsSelection,
scrollSelectionIntoView,
spellCheck,
describedBy,
...restProps
} = props

Expand Down Expand Up @@ -405,6 +407,7 @@ export const PortableTextEditable = forwardRef(function PortableTextEditable(
renderLeaf={renderLeaf}
style={props.style}
scrollSelectionIntoView={scrollSelectionIntoViewToSlate}
aria-describedby={describedBy}
/>
),
[
Expand All @@ -420,6 +423,7 @@ export const PortableTextEditable = forwardRef(function PortableTextEditable(
renderElement,
renderLeaf,
scrollSelectionIntoViewToSlate,
describedBy,
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('initialization', () => {
<div>
<div>
<div
aria-describedby="desc_foo"
aria-multiline="true"
autocapitalize="false"
autocorrect="false"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export const PortableTextEditorTester = forwardRef(function PortableTextEditorTe
<PortableTextEditable
selection={props.selection || undefined}
renderPlaceholder={props.renderPlaceholder}
describedBy="desc_foo"
/>
</PortableTextEditor>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import {FormNodeValidation} from '@sanity/types'
import {Box, Flex, Stack, Text} from '@sanity/ui'
import React, {memo} from 'react'
import {constructDescriptionId} from '../../members/common/constructDescriptionId'
import {FormFieldValidationStatus} from './FormFieldValidationStatus'

/** @internal */
Expand Down Expand Up @@ -45,7 +46,7 @@ export const FormFieldHeaderText = memo(function FormFieldHeaderText(
</Flex>

{description && (
<Text muted size={1}>
<Text muted size={1} id={constructDescriptionId(inputId, description)}>
{description}
</Text>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {FormNodeValidation} from '@sanity/types'
import {FormNodePresence} from '../../../presence'
import {DocumentFieldActionNode} from '../../../config'
import {useFieldActions} from '../../field'
import {constructDescriptionId} from '../../members/common/constructDescriptionId'
import {FormFieldValidationStatus} from './FormFieldValidationStatus'
import {FormFieldSetLegend} from './FormFieldSetLegend'
import {focusRingStyle} from './styles'
Expand Down Expand Up @@ -43,6 +44,7 @@ export interface FormFieldSetProps {
* @beta
*/
validation?: FormNodeValidation[]
inputId: string
}

function getChildren(children: React.ReactNode | (() => React.ReactNode)): React.ReactNode {
Expand Down Expand Up @@ -109,6 +111,7 @@ export const FormFieldSet = forwardRef(function FormFieldSet(
tabIndex,
title,
validation = EMPTY_ARRAY,
inputId,
...restProps
} = props

Expand Down Expand Up @@ -170,7 +173,7 @@ export const FormFieldSet = forwardRef(function FormFieldSet(
</Flex>

{description && (
<Text muted size={1}>
<Text muted size={1} id={constructDescriptionId(inputId, description)}>
{description}
</Text>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,11 @@ export function Compositor(props: Omit<InputProps, 'schemaType' | 'arrayFunction
renderPreview,
],
)

const describedBy = props.elementProps['aria-describedby']
const editorNode = useMemo(
() => (
<Editor
describedBy={describedBy}
hasFocus={hasFocus}
hotkeys={editorHotkeys}
isActive={isActive}
Expand Down Expand Up @@ -390,6 +391,7 @@ export function Compositor(props: Omit<InputProps, 'schemaType' | 'arrayFunction
onPaste,
path,
readOnly,
describedBy,
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ interface EditorProps {
scrollElement: HTMLElement | null
setPortalElement?: (portalElement: HTMLDivElement | null) => void
setScrollElement: (scrollElement: HTMLElement | null) => void
describedBy: string | undefined
}

const renderDecorator: RenderDecoratorFunction = (props) => {
Expand Down Expand Up @@ -84,6 +85,7 @@ export function Editor(props: EditorProps) {
scrollElement,
setPortalElement,
setScrollElement,
describedBy,
} = props
const {isTopLayer} = useLayer()
const editableRef = useRef<HTMLDivElement | null>(null)
Expand Down Expand Up @@ -148,6 +150,7 @@ export function Editor(props: EditorProps) {
selection={initialSelection}
style={noOutlineStyle}
spellCheck={spellcheck}
describedBy={describedBy}
/>
),
[
Expand All @@ -161,6 +164,7 @@ export function Editor(props: EditorProps) {
renderPlaceholder,
scrollSelectionIntoView,
spellcheck,
describedBy,
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ export function ReferenceField(props: ReferenceFieldProps) {
level={props.level}
title={props.title}
validation={props.validation}
inputId={props.inputId}
>
{isEditing ? (
<Box>{children}</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ export function ReferenceItem<Item extends ReferenceItemValue = ReferenceItemVal
description={schemaType.description}
__unstable_presence={presence}
validation={validation}
inputId={inputId}
>
{children}
</FormFieldSet>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {createProtoValue} from '../../../utils/createProtoValue'
import {isEmptyItem} from '../../../store/utils/isEmptyItem'
import {useResolveInitialValueForType} from '../../../../store'
import {resolveInitialArrayValues} from '../../common/resolveInitialArrayValues'
import {constructDescriptionId} from '../../common/constructDescriptionId'

/**
*
Expand Down Expand Up @@ -242,8 +243,12 @@ export function ArrayOfObjectsItem(props: MemberItemProps) {
onFocus: handleFocus,
id: member.item.id,
ref: focusRef,
'aria-describedby': constructDescriptionId(
member.item.id,
member.item.schemaType.description,
),
}),
[handleBlur, handleFocus, member.item.id],
[handleBlur, handleFocus, member.item.id, member.item.schemaType.description],
)

const inputProps = useMemo((): Omit<ObjectInputProps, 'renderDefault'> => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import {insert, PatchArg, PatchEvent, set, unset} from '../../../patch'
import {useFormCallbacks} from '../../../studio/contexts/FormCallbacks'
import {resolveNativeNumberInputValue} from '../../common/resolveNativeNumberInputValue'
import {constructDescriptionId} from '../../common/constructDescriptionId'

/**
*
Expand Down Expand Up @@ -111,6 +112,10 @@ export function ArrayOfPrimitivesItem(props: PrimitiveMemberItemProps) {
value: resolveNativeInputValue(member.item.schemaType, member.item.value, localValue),
readOnly: Boolean(member.item.readOnly),
placeholder: member.item.schemaType.placeholder,
'aria-describedby': constructDescriptionId(
member.item.id,
member.item.schemaType.description,
),
}),
[
handleBlur,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react'

/**
* Creates a description id from a field id, for use with aria-describedby in the field,
* and added to the descriptive element id.
* @internal
*/
export function constructDescriptionId(
Copy link
Member

@bjoerge bjoerge Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor detail, but I think it's more common for us to use create (or get) as prefix for this type of utility functions, so maybe rename to createDescriptionId if it's not too much trouble?

id: string | undefined,
description: React.ReactNode | undefined,
): string | undefined {
if (!description || !id) return undefined
return `desc_${id}`
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const MemberFieldSet = memo(function MemberFieldSet(props: {
onExpand={handleExpand}
columns={member?.fieldSet?.columns}
data-testid={`fieldset-${member.fieldSet.name}`}
inputId={member.fieldSet.name}
>
{member.fieldSet.members.map((fieldsetMember) => {
if (fieldsetMember.kind === 'error') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {resolveInitialArrayValues} from '../../common/resolveInitialArrayValues'
import {applyAll} from '../../../patch/applyPatch'
import {useFormPublishedId} from '../../../useFormPublishedId'
import {DocumentFieldActionNode} from '../../../../config'
import {constructDescriptionId} from '../../common/constructDescriptionId'

/**
* Responsible for creating inputProps and fieldProps to pass to ´renderInput´ and ´renderField´ for an array input
Expand Down Expand Up @@ -298,8 +299,12 @@ export function ArrayOfObjectsField(props: {
onFocus: handleFocus,
id: member.field.id,
ref: focusRef,
'aria-describedby': constructDescriptionId(
member.field.id,
member.field.schemaType.description,
),
}),
[handleBlur, handleFocus, member.field.id],
[handleBlur, handleFocus, member.field.id, member.field.schemaType.description],
)

const client = useClient(DEFAULT_STUDIO_CLIENT_OPTIONS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {readAsText} from '../../../studio/uploads/file/readAsText'
import {accepts} from '../../../studio/uploads/accepts'
import {applyAll} from '../../../patch/applyPatch'
import {useFormBuilder} from '../../../useFormBuilder'
import {constructDescriptionId} from '../../common/constructDescriptionId'

function move<T>(arr: T[], from: number, to: number): T[] {
const copy = arr.slice()
Expand Down Expand Up @@ -301,8 +302,12 @@ export function ArrayOfPrimitivesField(props: {
onFocus: handleFocus,
id: member.field.id,
ref: focusRef,
'aria-describedby': constructDescriptionId(
member.field.id,
member.field.schemaType.description,
),
}),
[handleBlur, handleFocus, member.field.id],
[handleBlur, handleFocus, member.field.id, member.field.schemaType.description],
)

const plainTextUploader = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {FormCallbacksProvider, useFormCallbacks} from '../../../studio/contexts/
import {createProtoValue} from '../../../utils/createProtoValue'
import {applyAll} from '../../../patch/applyPatch'
import {useFormBuilder} from '../../../useFormBuilder'
import {constructDescriptionId} from '../../common/constructDescriptionId'

/**
* Responsible for creating inputProps and fieldProps to pass to ´renderInput´ and ´renderField´ for an object input
Expand Down Expand Up @@ -183,8 +184,12 @@ export const ObjectField = function ObjectField(props: {
onFocus: handleFocus,
id: member.field.id,
ref: focusRef,
'aria-describedby': constructDescriptionId(
member.field.id,
member.field.schemaType.description,
),
}),
[handleBlur, handleFocus, member.field.id],
[handleBlur, handleFocus, member.field.id, member.field.schemaType.description],
)

const inputProps = useMemo((): Omit<ObjectInputProps, 'renderDefault'> => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {FormPatch, PatchEvent, set, unset} from '../../../patch'
import {useFormCallbacks} from '../../../studio/contexts/FormCallbacks'
import {resolveNativeNumberInputValue} from '../../common/resolveNativeNumberInputValue'
import {useFormBuilder} from '../../../useFormBuilder'
import {constructDescriptionId} from '../../common/constructDescriptionId'

/**
* Responsible for creating inputProps and fieldProps to pass to ´renderInput´ and ´renderField´ for a primitive field/input
Expand Down Expand Up @@ -105,6 +106,10 @@ export function PrimitiveField(props: {
value: resolveNativeNumberInputValue(member.field.schemaType, member.field.value, localValue),
readOnly: Boolean(member.field.readOnly),
placeholder: member.field.schemaType.placeholder,
'aria-describedby': constructDescriptionId(
member.field.id,
member.field.schemaType.description,
),
}),
[
handleBlur,
Expand Down
8 changes: 7 additions & 1 deletion packages/sanity/src/core/form/studio/FormBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,13 @@ function RootInput() {

const rootInputProps: Omit<ObjectInputProps, 'renderDefault'> = {
focusPath,
elementProps: {ref: focusRef, id, onBlur: handleBlur, onFocus: handleFocus},
elementProps: {
ref: focusRef,
id,
onBlur: handleBlur,
onFocus: handleFocus,
'aria-describedby': undefined, // Root input should not have any aria-describedby
},
changed: members.some((m) => m.kind === 'field' && m.field.changed),
focused,
groups,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function ObjectOrArrayField(field: ObjectFieldProps | ArrayFieldProps) {
onExpand={field.onExpand}
title={field.title}
validation={field.validation}
inputId={field.inputId}
>
{field.children}
</FormFieldSet>
Expand Down Expand Up @@ -152,6 +153,7 @@ function ImageOrFileField(field: ObjectFieldProps) {
onExpand={field.onExpand}
title={field.title}
validation={field.validation}
inputId={field.inputId}
>
{field.children}
</FormFieldSet>
Expand Down
2 changes: 2 additions & 0 deletions packages/sanity/src/core/form/types/inputProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ export interface PrimitiveInputElementProps {
onFocus: FocusEventHandler
onBlur: FocusEventHandler
ref: React.MutableRefObject<any>
'aria-describedby': string | undefined
}

/**
Expand All @@ -402,6 +403,7 @@ export interface ComplexElementProps {
onFocus: FocusEventHandler
onBlur: FocusEventHandler
ref: React.MutableRefObject<any>
'aria-describedby': string | undefined
}

/**
Expand Down
Loading