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(Forms): auto-open Form.Section container when fields have errors and add validateInitially prop #3878

Merged
merged 9 commits into from
Sep 12, 2024

Conversation

tujoworker
Copy link
Member

@tujoworker tujoworker commented Aug 28, 2024

This feature allows a dev to put on a Form.Section. When a field inside has an error/ or is validating negatively, the section will automatically switch to edit mode and show these errors, without pressing submit on the form.

Demo.

Quick example:

<Form.Section
  path="/nestedPath"
  required
  validateInitially
>
  <MyEditContainer />
  <MyViewContainer />
</Form.Section>

@tujoworker tujoworker requested review from langz and andlbrei August 28, 2024 13:20
Copy link

vercel bot commented Aug 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
eufemia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 12, 2024 9:23am

Copy link

codesandbox-ci bot commented Aug 28, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Contributor

@andlbrei andlbrei left a comment

Choose a reason for hiding this comment

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

That was quick 😄
Looks good 👍

@tujoworker tujoworker force-pushed the feat/section-error-mode branch from d543368 to f2ddece Compare August 28, 2024 13:38
@tujoworker
Copy link
Member Author

Here is a quick demo/example (have not tested it yet): https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/demos/#show-errors-on-the-whole-section

@andlbrei
Copy link
Contributor

The edit container opens fine 👍
Cancel is allowed though, which I guess it shouldn't be 🤔 if we want to force the user to fix the errors

Copy link
Contributor

@langz langz left a comment

Choose a reason for hiding this comment

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

Looks good :)

Is it a potential error that when pressing the code button under the example, the screen sort of flashes?
Or is it just because of some scroll and/or animation stuff?

Screen.Recording.2024-08-28.at.20.59.21.mov

Further, just a nitpick, but it's a bit unfortunate that it autoscrolls to the new example when visiting the docs https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/
Makes it so that we can't link to other headings in the same docs, like https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/#overwrite-props, as it will autoscroll to our new example.

Could that also be a problem in an application, or something they would/could opt-out of?
Let's say if you enter a page, and on the bottom of the page they have this form with validateFieldsInitially, would it automatically scroll the user down to the bottom of the page?

@langz
Copy link
Contributor

langz commented Aug 28, 2024

Just to be curious, this of course requires that all fields used in the form.section supports/has the validateInitially property, right?

If so, is it so that this validateInitially property is automatically supported in all our fields, or do we have to manually add support for this in all our fields? Further, if other applications make their own fields, do they have to do anything to support this prop validateInitially? If so, could we make it clear in our docs that we and/or apps should/must support the validateInitially property?

@tujoworker
Copy link
Member Author

@andlbrei Nice finding – the latest commit should fix it. But I have not tested it. Can you check it out?

@tujoworker
Copy link
Member Author

Just to be curious, this of course requires that all fields used in the form.section supports/has the validateInitially property, right?

@langz Actually no.

And if a dev makes their own Field, they should use the useFieldProps hook. This is documented here. And with that, the validateInitially is supported. But it is not based on that in this PR. The "magic" happens within the error boundary and in the useFieldProps hook via the showBoundaryErrors variable.

@tujoworker
Copy link
Member Author

it's a bit unfortunate that it autoscrolls to the new example when visiting the docs https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/

Very true, we should not set focus when it opens initially. Will have to fix it + test.

@andlbrei
Copy link
Contributor

@tujoworker works fine now 👍

@tujoworker
Copy link
Member Author

tujoworker commented Sep 1, 2024

@andlbrei How about renaming the prop from validateFieldsInitially to:

  • containerMode="validate-fields-initially" or
  • containerMode="open-when-fields-error" or
  • containerMode="openWhenFieldsError"

@andlbrei
Copy link
Contributor

andlbrei commented Sep 1, 2024

@tujoworker I have no objections to that 👍

@tujoworker tujoworker changed the title feat(Forms): add validateFieldsInitially to Form.Section feat(Forms): add containerMode="openWhenFieldValidationError" to Form.Section Sep 1, 2024
@tujoworker tujoworker force-pushed the feat/section-error-mode branch from 08cbcac to 6180465 Compare September 1, 2024 13:09
@tujoworker
Copy link
Member Author

Then I think we go for containerMode="openWhenFieldValidationError" – its pretty long, but it describes what its doing. That I think is important.

@andlbrei
Copy link
Contributor

@tujoworker Looks very good now 👍 I like the default to auto, that might make a lot of sense.

@langz
Copy link
Contributor

langz commented Sep 11, 2024

@tujoworker You write "To make these errors "visible", you can add validateInitially to the section." While the behaviour seems to be like the code documentation says: "When set to openWhenFieldValidationError, the mode will initially be "edit" if fields contain errors. Fields will then automatically get validateInitially and show their error messages."

validateInitially does not seem to be a valid prop on Form.Section.

To re-state the use case I have discussed previously: We have some data we fetch before rendering the form. Sometimes a section will be populated with this pre-loaded data, in which case view mode is what I want, and sometimes the section will need to be filled out by the user because we do not have any stored data from the user, in which case we want edit mode. This is attainable by using openWhenFieldValidationError.

So the validation error in this case is pretty specific; it is simply for required fields that are empty.

In this case I would prefer the section to open in edit mode if there are errors, using mode="openWhenFieldValidationError" and not show any error messages. I just want to guide the user to fill inn fields where they are missing, and if they try to finish editing the section while there still are errors, then I'd like them to get the error messages.

However, let's say the pre-loaded data has the wrong format, then it might be best to display the error message straight away. I would think that this would be possible using validateInitially on the respective fields within the section, but that assumption might not be correct.

@andlbrei, thanks so much for testing and giving feedback, I see that both you and I did not test the latest commit of @tujoworker as it wasn't pushed until recently.
Do you have the time to quickly check if the feedback you gave in this comment is still valid when testing now?

https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/#show-errors-on-the-whole-section

@andlbrei
Copy link
Contributor

@langz I did it while you wrote your message I think 😅

@langz
Copy link
Contributor

langz commented Sep 11, 2024

it's a bit unfortunate that it autoscrolls to the new example when visiting the docs https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/

Very true, we should not set focus when it opens initially. Will have to fix it + test.

I think this "issue" is back again, when testing in the deploy preview now, it autoscrolls to a different example/header than what I'm linking to(linking to usage header, but scrolls to something else): https://eufemia-git-feat-section-error-mode-eufemia.vercel.app/uilib/extensions/forms/Form/Section/#usage

Screen.Recording.2024-09-11.at.12.15.30.mov

Copy link
Contributor

@langz langz left a comment

Choose a reason for hiding this comment

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

Looks very good now, I love the API improvements, great work ❤️

Just one thought/question, what will happen if setting validateInitially={true} on Form.Section which has a single field with validateInitially={false}, which property dictates if the field will be validated initially? Perhaps we should add a test for the answer of this question?

@tujoworker
Copy link
Member Author

Perhaps we should add a test for the answer of this question?

Done.

@tujoworker
Copy link
Member Author

I think this "issue" is back again

Ohh noo. But added a better test and fix ✅

Copy link
Contributor

@langz langz left a comment

Choose a reason for hiding this comment

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

🥇

@tujoworker tujoworker force-pushed the feat/section-error-mode branch from 36e10ed to 1dd23c9 Compare September 12, 2024 09:04
@tujoworker tujoworker merged commit 9b49006 into main Sep 12, 2024
10 checks passed
@tujoworker tujoworker deleted the feat/section-error-mode branch September 12, 2024 11:50
tujoworker pushed a commit that referenced this pull request Sep 12, 2024
## [10.48.0](v10.47.0...v10.48.0) (2024-09-12)

### 🐛 Bug Fixes

* **countries:** Fixes wrong country code for Martinique [#3915](#3915) ([a9f86e4](a9f86e4))
* **countries:** Remove outdated countries [#3915](#3915) ([36ef5cf](36ef5cf))
* **Forms:** rename 'Macedonia' to 'North Macedonia' ([#3918](#3918)) ([a4eb8a4](a4eb8a4))
* **Forms:** rename "Hviterussland" to "Belarus" ([#3917](#3917)) ([702118a](702118a))
* removes outdated countries based on ISO 3166-1 alpha-2 ([#3916](#3916)) ([a045acd](a045acd))

### ✨ Features

* **Forms:** add `toolbarVariant="minimumOneItem"` to Iterate.Toolbar for hiding buttons when there is only one item in the array ([#3919](#3919)) ([3367a77](3367a77)), closes [#3877](#3877)
* **Forms:** add `validator` support to Iterate.Array ([#3926](#3926)) ([6fd439e](6fd439e))
* **Forms:** auto-open Form.Section container when fields have errors and add `validateInitially` prop  ([#3878](#3878)) ([9b49006](9b49006))
* **Forms:** auto-open iterate containers when validation errors and make `Iterate.Toolbar` fully customizable ([#3877](#3877)) ([52326bf](52326bf)), closes [#3919](#3919)
* **Forms:** show optional label when a field uses `required={false}` and add `labelSuffix` prop to each field ([#3921](#3921)) ([60e440a](60e440a))
* **Wizard:** add `preventNavigation` callback function to `onStepChange` ([#3924](#3924)) ([5ec2772](5ec2772))
@tujoworker
Copy link
Member Author

🎉 This PR is included in version 10.48.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
Development

Successfully merging this pull request may close these issues.

3 participants