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(26339): Improve errors display in RJSF forms and handle focus on hidden widget #646

Merged
merged 18 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -1,11 +1,12 @@
import { FC, useMemo } from 'react'
import { FC, useEffect, useMemo } from 'react'
import { useTranslation } from 'react-i18next'
import { ArrayFieldTemplateItemType, getTemplate, getUiOptions } from '@rjsf/utils'
import { Box, ButtonGroup, FormControl, HStack, useDisclosure, VStack } from '@chakra-ui/react'
import { LuPanelTopClose, LuPanelTopOpen } from 'react-icons/lu'

import IconButton from '@/components/Chakra/IconButton.tsx'
import { CopyButton, MoveDownButton, MoveUpButton, RemoveButton } from '@/components/rjsf/__internals/IconButton.tsx'
import { useFormControlStore } from '@/components/rjsf/Form/useFormControlStore.ts'

// TODO[NVL] Need a better handling of the custom UISchema property, for the Adapter SDK
interface ArrayFieldItemCollapsableUISchema {
Expand All @@ -32,10 +33,13 @@ export const ArrayFieldItemTemplate: FC<ArrayFieldTemplateItemType> = (props) =>
} = props
const { t } = useTranslation('components')
const uiOptions = getUiOptions(uiSchema)
const { expandItems } = useFormControlStore()

const collapsableItems: ArrayFieldItemCollapsableUISchema | undefined = useMemo(() => {
return uiOptions.collapsable as ArrayFieldItemCollapsableUISchema | undefined
}, [uiOptions.collapsable])
const { isOpen, onToggle, getButtonProps, getDisclosureProps } = useDisclosure({

const { isOpen, onToggle, getButtonProps, getDisclosureProps, onOpen } = useDisclosure({
defaultIsOpen:
!collapsableItems ||
!collapsableItems?.titleKey ||
Expand All @@ -49,6 +53,10 @@ export const ArrayFieldItemTemplate: FC<ArrayFieldTemplateItemType> = (props) =>
return children.props.name
}, [children.props.formData, children.props.name, collapsableItems?.titleKey])

useEffect(() => {
if (props.children.props.idSchema.$id === expandItems.join('_')) onOpen()
}, [expandItems, onOpen, props.children.props.idSchema.$id])

const onCopyClick = useMemo(() => onCopyIndexClick(index), [index, onCopyIndexClick])
const onRemoveClick = useMemo(() => onDropIndexClick(index), [index, onDropIndexClick])
const onArrowUpClick = useMemo(() => onReorderClick(index, index - 1), [index, onReorderClick])
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FC, useCallback, useMemo, useState } from 'react'
import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { useTranslation } from 'react-i18next'
import debug from 'debug'
import { immutableJSONPatch, JSONPatchAdd, JSONPatchDocument } from 'immutable-json-patch'
Expand All @@ -13,11 +13,13 @@ import { DescriptionFieldTemplate } from '@/components/rjsf/Templates/Descriptio
import { BaseInputTemplate } from '@/components/rjsf/BaseInputTemplate.tsx'
import { ArrayFieldTemplate } from '@/components/rjsf/ArrayFieldTemplate.tsx'
import { ArrayFieldItemTemplate } from '@/components/rjsf/ArrayFieldItemTemplate.tsx'
import { ErrorListTemplate } from '@/components/rjsf/Templates/ErrorListTemplate.tsx'
import { ChakraRJSFormContext } from '@/components/rjsf/Form/types.ts'
import { customFormatsValidator } from '@/modules/ProtocolAdapters/utils/validation-utils.ts'
import { adapterJSFFields, adapterJSFWidgets } from '@/modules/ProtocolAdapters/utils/uiSchema.utils.ts'
import { customFocusError } from '@/components/rjsf/Form/error-focus.utils.ts'
import { TitleFieldTemplate } from '@/components/rjsf/Templates/TitleFieldTemplate.tsx'
import { ErrorListTemplate } from '@/components/rjsf/Templates/ErrorListTemplate.tsx'
import { useFormControlStore } from '@/components/rjsf/Form/useFormControlStore.ts'

interface CustomFormProps<T>
extends Pick<
Expand All @@ -40,6 +42,8 @@ const ChakraRJSForm: FC<CustomFormProps<unknown>> = ({
readonly,
}) => {
const { t } = useTranslation()
const { setTabIndex } = useFormControlStore()
const ref = useRef(null)
const [batchData, setBatchData] = useState<JSONPatchDocument | undefined>(undefined)
const defaultValues = useMemo(() => {
if (batchData) {
Expand All @@ -55,6 +59,15 @@ const ChakraRJSForm: FC<CustomFormProps<unknown>> = ({
[onSubmit]
)

useEffect(
() => {
setTabIndex(0)
return () => setTabIndex(0)
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
)

const context: ChakraRJSFormContext = {
...formContext,
onBatchUpload: (idSchema: IdSchema<unknown>, batch) => {
Expand All @@ -70,6 +83,7 @@ const ChakraRJSForm: FC<CustomFormProps<unknown>> = ({

setBatchData(operations)
},
focusOnError: customFocusError(ref),
}

const rjsfLog = debug(`RJSF:${id}`)
Expand All @@ -79,6 +93,7 @@ const ChakraRJSForm: FC<CustomFormProps<unknown>> = ({

return (
<Form
ref={ref}
id={id}
readonly={readonly}
schema={unspecifiedSchema}
Expand All @@ -95,17 +110,17 @@ const ChakraRJSForm: FC<CustomFormProps<unknown>> = ({
ErrorListTemplate,
TitleFieldTemplate,
}}
widgets={adapterJSFWidgets}
fields={adapterJSFFields}
onSubmit={onValidate}
liveValidate
// TODO[NVL] Removing HTML validation; see https://rjsf-team.github.io/react-jsonschema-form/docs/usage/validation/#html5-validation
noHtml5Validate
focusOnFirstError
onSubmit={onValidate}
validator={customFormatsValidator}
customValidate={customValidate}
widgets={adapterJSFWidgets}
fields={adapterJSFFields}
onError={(errors) => rjsfLog(t('error.rjsf.validation'), errors)}
showErrorList="bottom"
focusOnFirstError={context.focusOnError}
/>
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
import { expect } from 'vitest'

import {
_toPath,
deepGet,
isNumeric,
isPropertyBehindCollapsedElement,
isPropertyBehindTab,
} from '@/components/rjsf/Form/error-focus.utils.ts'

describe('_toPath', () => {
it.each([
{
path: '',
value: null,
},
{
path: 'a.b.c',
value: ['a', 'b', 'c'],
},
{
path: 'a[0].b.c',
value: ['a', '0', 'b', 'c'],
},
])('should return $value for $path', ({ path, value }) => {
expect(_toPath(path)).toStrictEqual(value)
})
})

describe('isNumeric', () => {
it.each(['1', '1000'])('$item should be a number', (item) => {
expect(isNumeric(item)).toStrictEqual(true)
})
it.each(['1.', '1.5', '-1'])('$item should not be a number', (item) => {
expect(isNumeric(item)).toStrictEqual(false)
})
})

const MOCK_OBJECT = {
foo: {
foz: [1, 2, 3],
bar: {
baz: ['a', 'b', 'c'],
},
},
}

describe('deepGet', () => {
it('should return null for an empty object', async () => {
expect(deepGet({}, [1])).toStrictEqual(null)
})

it('should return null for an empty key', async () => {
expect(deepGet(MOCK_OBJECT, [])).toStrictEqual(null)
})

it.each([
{
keys: ['foo', 'bar'],
value: {
baz: ['a', 'b', 'c'],
},
},
{ keys: ['foo', 'foz', 2], value: 3 },
{ keys: ['foo', 'bar', 'baz', 1], value: 'b' },
{ keys: ['foo', 'bar', 'baz', 8, 'foz'], value: null },
])('should return $value for $keys', ({ keys, value }) => {
expect(deepGet(MOCK_OBJECT, keys)).toStrictEqual(value)
})
})

describe('isPropertyBehindCollapsedElement', () => {
it.each([
{
property: '',
},
{
property: 'id',
},
{
property: '.property',
},
{
property: '.property.without.any.array',
},
])('should be undefined for $property', ({ property }) => {
expect(isPropertyBehindCollapsedElement(property, {})).toStrictEqual(undefined)
})

it.each([
{
property: '.property.0.with_no_items',
uiSchema: {
property: {},
},
value: undefined,
},
{
property: '.property.0.item.without_collapsable_tag',
uiSchema: {
property: {
items: {
'ui:title': 'ss',
},
},
},
value: undefined,
},
{
property: '.property.0.item.with_collapsable_tag',
uiSchema: {
property: {
items: {
'ui:title': 'ss',
'ui:collapsable': 'ss',
},
},
},
value: ['root', 'property', '0'],
},
{
property: '.property.0.subProp.1.another.level.2.item',
uiSchema: {
property: {
items: {
'ui:title': 'ss',
'ui:collapsable': 'ss',
},
},
},
value: ['root', 'property', '0'],
},
])('should return $value for $property', ({ property, uiSchema, value }) => {
expect(isPropertyBehindCollapsedElement(property, uiSchema)).toStrictEqual(value)
})
})

describe('isPropertyBehindTab', () => {
it('should return undefined if tabs are not defined', async () => {
expect(isPropertyBehindTab('whatever.the.property.path', { test: 1 })).toStrictEqual(undefined)
expect(isPropertyBehindTab('whatever.the.property.path', { 'ui:tabs': 1 })).toStrictEqual(undefined)
})

it('should return undefined if property is not defined', async () => {
expect(isPropertyBehindTab('', { 'ui:tabs': [] })).toStrictEqual(undefined)
})

it('should return undefined if property is not in the tabs', async () => {
expect(isPropertyBehindTab('.test.property', { 'ui:tabs': [] })).toStrictEqual(undefined)
})

it('should return undefined if property is not in the tabs', async () => {
expect(isPropertyBehindTab('.test.property', { 'ui:tabs': [{ wrongProperties: '' }] })).toStrictEqual(undefined)
})
it('should return undefined if property is not in the tabs', async () => {
const mockUISchema = { 'ui:tabs': [{ id: 'tab1', title: 'tab 1', properties: ['prop1', 'prop2'] }] }
expect(isPropertyBehindTab('.test.property', mockUISchema)).toStrictEqual(undefined)
})
it('should return tab1 if property is in the tab', async () => {
const mockUISchema = { 'ui:tabs': [{ id: 'tab1', title: 'tab 1', properties: ['test', 'prop2'] }] }
expect(isPropertyBehindTab('.test.property', mockUISchema)).toStrictEqual({
id: 'tab1',
title: 'tab 1',
properties: ['test', 'prop2'],
index: 0,
})
})
it('should return rab2 if property is in the tab', async () => {
const mockUISchema = {
'ui:tabs': [
{ id: 'tab1', title: 'tab 1', properties: ['prop123'] },
{ id: 'tab2', title: 'tab 2', properties: ['prop456', 'test'] },
{ id: 'tab3', title: 'tab 3', properties: ['prop789'] },
],
}
expect(isPropertyBehindTab('.test.property', mockUISchema)).toStrictEqual({
id: 'tab2',
title: 'tab 2',
properties: ['prop456', 'test'],
index: 1,
})
})
})
Loading
Loading