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

Forms: Combined error messages are inflexible and presented in English #4052

Closed
kimroen opened this issue Oct 3, 2024 · 4 comments · Fixed by #4176
Closed

Forms: Combined error messages are inflexible and presented in English #4052

kimroen opened this issue Oct 3, 2024 · 4 comments · Fixed by #4176
Labels
bug Something isn't working enhancement New feature or request released

Comments

@kimroen
Copy link
Contributor

kimroen commented Oct 3, 2024

🐛 Bug Report

When rendering a field with multiple validation props, a combined error message is shown. Example:

must NOT have more than 3 characters and must match pattern "^[a-zA-Z]$"

This error message has multiple problems:

  • It is in English, unlike other messages
  • It doesn't start with a capital letter
  • It doesn't end with a period
  • It is not customizable (in full or in part)

Here's an example from our application:

CleanShot 2024-10-03 at 16 29 32@2x

To Reproduce

Here is a CodeSandbox demonstrating the issue

Expected behavior

The error message is displayed in Norwegian or English, depending on the set language. As a bonus, It would be nice to be able to customize (showing the pattern isn't great, for instance)

Eufemia Version

Browser JS: 10.51.1

Browser CSS: 10.51.1

@kimroen
Copy link
Contributor Author

kimroen commented Oct 12, 2024

As one might expect, this is because when there's more than one error from ajv, we're just passing through the errors directly, concatenated with " and ".

This happens here:

const error = ajvErrorsToOneFormError(
schemaValidatorRef.current.errors,
valueRef.current
)

This calls the function ajvErrorsToOneFormError:

export function ajvErrorsToOneFormError(
errors?: ErrorObject[] | null,
value?: unknown
): FormError | undefined {
if (!errors || errors.length === 0) {
return
}
if (errors.length === 1) {
const error = ajvErrorsTransformation(errors[0], value)
if (!error) {
return undefined
}
return ajvErrorToFormError(error)
}
const errorMessages = errors?.map((error) => error.message)
return new FormError(errorMessages.join(' and '))
}

Here, if there's just one error we transform it into a sensible FormError instance with extracted information, but if there are more then we fall back to some fairly simple behavior where we extract message and concat them all together:

const errorMessages = errors?.map((error) => error.message)
return new FormError(errorMessages.join(' and '))

I made this codesandbox where you can enter some text and see what the error objects look like (as well as replicating the concatenation behavior that leads to this error message):

Showing the error object output of validating the text from earlier examples

Now, I suppose the question is: What should it do instead?

Do we want it to combine multiple errors into one, or should we display multiple errors? Should we make ajv stop on the first error to avoid this all together (this matches the behavior of passing both a schema and validate function, for instance)?

@tujoworker
Copy link
Member

Thank you for the good report and detailed feedback 🫶

@tujoworker tujoworker added bug Something isn't working enhancement New feature or request labels Oct 24, 2024
@tujoworker
Copy link
Member

Here is a fork of your good CSB that includes the fix. Thank you so much.

tujoworker pushed a commit that referenced this issue Oct 30, 2024
## [10.54.0](v10.53.0...v10.54.0) (2024-10-30)

### 📝 Documentation

* add documentation about `emptyValue` ([#4174](#4174)) ([52457ba](52457ba)), closes [#4070](#4070)

### 🐛 Bug Fixes

* **Field.Number:** `decimalLimit={0}` when `currency` should work ([#4167](#4167)) ([68123ea](68123ea))
* **Forms:** correct alignment of Wizard status message (error, warning, info) ([#4185](#4185)) ([a307cda](a307cda))
* **Forms:** ensure autocomplete="off" when autoComplete on Form.Handler is false ([#4184](#4184)) ([6795b9f](6795b9f))
* **Forms:** ensure correct sorting in Field.SelectCountry ([#4196](#4196)) ([8db7720](8db7720)), closes [#4195](#4195)
* **Forms:** fix schema validation for required paths with matching name ([#4189](#4189)) ([04caf61](04caf61)), closes [#4179](#4179)
* **Forms:** only run onBlurValidator when no other errors are present ([#4172](#4172)) ([d2797f1](d2797f1)), closes [#4074](#4074)
* **Forms:** render multiple (combined) Ajv errors with translated messages ([#4176](#4176)) ([2b62ef7](2b62ef7)), closes [#4052](#4052)
* **Forms:** updates country names in SelectCountry ([#4187](#4187)) ([bb1b67c](bb1b67c))
* **NumberFormat:** should not throw exception when providing an invalid currency ([#4192](#4192)) ([a0700cd](a0700cd))
* **Table:** fix visible hidden rows for additional expandable table rows ([#4188](#4188)) ([16b6101](16b6101))

### ✨ Features

* add CountryFlag component ([#4181](#4181)) ([76a0c47](76a0c47))
* **Dialog:** add `verticalAlignment` property with `top` alignment support ([#4190](#4190)) ([3ace8b0](3ace8b0))
* **Forms:** add `onDone`, `onCancel` and `onEdit` to Form.Section containers ([#4199](#4199)) ([2f27ad0](2f27ad0))
* **Forms:** add `transformData` to the onSubmit event of Form.Handler ([#4183](#4183)) ([748b604](748b604))
* **Forms:** deprecate `useErrorMessage` hook for when creating your own fields ([#4177](#4177)) ([2ae86f2](2ae86f2)), closes [#4162](#4162)
* **Forms:** deprecate Ajv `validationRule` in FormError and deprecate `errorMessages` keys like `pattern` in favor of Eufemia translation keys like `Field.errorPattern` ([#4162](#4162)) ([b50387a](b50387a))
* **Keyboard navigation:** add support for `key` property (useful for fire events like {ArrowDown} during tests) ([#4182](#4182)) ([cc1ffa8](cc1ffa8))
* **Upload:** `text` and `title` is possible to remove ([#4163](#4163)) ([b6db4a4](b6db4a4))
* **Upload:** removes subtitle displaying file extension ([#4156](#4156)) ([9bf9b9e](9bf9b9e))
* **Upload:** updates drag and drop texts in Norwegian ([#4170](#4170)) ([ad5bbec](ad5bbec))
@tujoworker
Copy link
Member

🎉 This issue has been resolved in version 10.54.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request released
Development

Successfully merging a pull request may close this issue.

2 participants