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.

Sign up for GitHub

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

Jump to bottom

feat(Forms): add Form.InfoOverlay to display error, success (receipt), or custom messages to users #4357

Merged
merged 6 commits into from
Dec 6, 2024
Merged

feat(Forms): add Form.InfoOverlay to display error, success (receipt), or custom messages to users #4357

merged 6 commits into from
Dec 6, 2024

Conversation

tujoworker
Copy link
Member

@tujoworker tujoworker commented Dec 4, 2024 •
edited
Loading

PR Preview.

@tujoworker tujoworker requested a review from langz December 4, 2024 08:10
@vercel Vercel
Copy link

vercel bot commented Dec 4, 2024 •
edited
Loading

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 Dec 6, 2024 8:59am

@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Dec 4, 2024 •
edited
Loading

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.

@andlbrei
Copy link
Contributor

andlbrei commented Dec 4, 2024 •
edited
Loading

I did a quick test in CSB, looks good! 👍 There are some quirks.

And I wonder if it is possible to customize the success and error pages?
I see in the code that it's hard coded, which is fine.
Our design has some images in addition to the text, but I don't think that is important.

Screen.Recording.2024-12-04.at.10.02.01.mov

@langz
Copy link
Contributor

langz commented Dec 4, 2024 •
edited
Loading

Small observation when entering the docs is that it does not start/show the top of the page, but scrolls down the page, not very important though.

@langz
Copy link
Contributor

langz commented Dec 4, 2024

When "trying again", should I as a user get updated feedback that something went wrong again?
When just pressing "Prøv igjen"-button even though it nicely displays a loader in the button, I don't feel like the error message "Vi klarte ikke å sende inn skjemaet." is based on my latest action(pressing "Prøv igjen"-button).

Screen.Recording.2024-12-04.at.10.42.15.mov

Perhaps we could remove the existing error message when displaying the loader in the button?
That's just a suggestion. Could possible be solved in other ways.

@tujoworker
Copy link
Member Author

but scrolls down the page, not very important though.

It is actually important that this not happens. The focus gets set initially. Which should it not. Let me fix that.

@tujoworker
Copy link
Member Author

@andlbrei

The title of the status page takes a few "tries" it seems to render in the proper style.

I think its because the heading font is a different font and needs to be downloaded. If you would have a heading on the first view, it should not happen.

I need to press submit twice to get the status page up.

I will look into this asap.

And I wonder if it is possible to customize the success and error pages?

Just the texts.

Our design has some images in addition to the text, but I don't think that is important.

Yes. We will add the illustrations in a followup PR.

@langz
Copy link
Contributor

langz commented Dec 4, 2024 •
edited
Loading

Thoughts in regards to naming, Form.Status:

  • I'm thinking about the status of the form, sort of displaying the status of the form(how many errors, etc, it has).
  • Or that it would functionality-wise be something related to GlobalStatus or FormStatus 🤔

Other suggestions:

  • Form.SubmissionStatus
  • Form.SubmitStatus
  • Form.Receipt
  • Form.StatusReceipt
  • Form.Submit.Error
  • Form.Submit.Success
  • Form.SubmitMessage.Error
  • Form.SubmitMessage.Success
  • Form.Message.Error
  • Form.Message.Success

Since we have a lot of Form.SubmitX components (Form.SubmitButton, Form.SubmitIndicator, Form.SubmitConfirmation).
I think I like these suggestions the best:

  • Form.SubmitMessage
  • Form.SubmitMessage.Error
  • Form.SubmitMessage.Success

langz
langz reviewed Dec 4, 2024
View reviewed changes
packages/dnb-design-system-portal/src/docs/uilib/extensions/forms/Form/Status/properties.mdx Outdated

### Success

<PropertiesTable props={StatusSuccessProperties} />
Copy link
Contributor

Choose a reason for hiding this comment

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

We could perhaps add an example which shows how to use these properties.
I'm not 100% sure how to do so when reading the docs my self, as the examples as of now only use Form.Status.setStatus

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have a prop to show the status at the moment. What could be call it? I guess just status="error".

Copy link
Contributor

@langz langz Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if we are following each other here 😅

To me, when I read the property docs https://eufemia-git-feat-forms-status-eufemia.vercel.app/uilib/extensions/forms/Form/Status/properties/#error

image

I would think that I as a consumer/user could use these props. How can I use the cancelButton property? Could we add an example or so, so that I can see how to use the prop?

@tujoworker tujoworker mentioned this pull request Dec 5, 2024
Merged
@tujoworker
Copy link
Member Author

Perhaps we could remove the existing error message when displaying the loader in the button?
That's just a suggestion. Could possible be solved in other ways.

I have changed the text. Maybe it's better now?

@tujoworker
Copy link
Member Author

tujoworker commented Dec 5, 2024 •
edited
Loading

Regarding the naming. I think I want to avoid to use submit in it. Just because it can be used in other cases as well. Like in between a Wizard step. In many ways, it acts like a "modal".

  1. And we can make it possible to render something totally custom as well (`status="custom").
  2. And maybe we should change the "success" status to "complete", just because we then go ever further away from thinking it is just "submit" related.

Something like this?

  • Form.Message
  • Form.ViewMessage
  • Form.InfoMessage
  • Form.StatusView
  • Form.InformationView
  • Form.InfoPanel
  • Form.InfoOverlay

tujoworker added a commit that referenced this pull request Dec 5, 2024
@tujoworker
fix(Forms): avoid unnecessary rerenders in Form.Handler (#4363)
7de5e49 
The reason behind this is that I saw a negative side-effect in PR #4357
when it comes to the initial render, where we don't want an animation,
but it did animate.
@langz
Copy link
Contributor

langz commented Dec 5, 2024 •
edited
Loading

Perhaps we could remove the existing error message when displaying the loader in the button?
That's just a suggestion. Could possible be solved in other ways.

I have changed the text. Maybe it's better now?

I don't feel it's about the text content, but that I as a user don't get any feedback when clicking "prøv igjen", and it fails over and over again. It just displays the same text (which can be fine if it's the same error message). But there's no "flashing" of the text and/or feedback. I'm not sure if I see the latest error message or the first error message.
But maybe this will be something that will have to be fixed at the application side? 🤔

Screen.Recording.2024-12-05.at.20.39.23.mov

@langz
Copy link
Contributor

langz commented Dec 5, 2024 •
edited
Loading

Regarding the naming. I think I want to avoid to use submit in it. Just because it can be used in other cases as well. Like in between a Wizard step. In many ways, it acts like a "modal".

  1. And we can make it possible to render something totally custom as well (`status="custom").
  2. And maybe we should change the "success" status to "complete", just because we then go ever further away from thinking it is just "submit" related.

Something like this?

  • Form.Message
  • Form.ViewMessage
  • Form.InfoMessage
  • Form.StatusView
  • Form.InformationView
  • Form.InfoPanel
  • Form.InfoOverlay

I see, sounds smart :)

Perhaps Form.Message? And Form.Message.setMessage?

But it doesn't feel like a normal component to me? Sort of like a Provider (probably not) or something else?
It's like you wrap your whole form in it, and then use Form.Message.setMessage to replace children of it with one of many different messages available.

@tujoworker
Copy link
Member Author

tujoworker commented Dec 6, 2024 •
edited
Loading

  • Renamed to InfoOverlay with setContent.
  • Added support to be used without Form.Handler.
  • Added support to be used with content prop (without setContent).
  • Enhanced docs with info about how to use the props and how to customize.

Here is the new build preview.

@tujoworker tujoworker changed the title feat(Forms): introduce Form.Status components for showing error and success messages (receipt) feat(Forms): introduce Form.InfoOverlay components for showing error and success messages (receipt) Dec 6, 2024
langz
langz approved these changes Dec 6, 2024
View reviewed changes
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 :)
My last thought is regarding the default texts/translations that we provide.
Should we provide them? Do we think they'll ever be used?
Perhaps most of them will be used, so that gives some value. But I can't imagine Vi har mottatt din informasjon. to ever be used in an app, as it just seems to generic for me. Same for Tilbake til forsiden, is it that likely that they'll be navigated to the home page?

Just keep it as it is, just a few last thoughts 💚

@tujoworker
Copy link
Member Author

Yes, good thoughts.
They are based on designs by Erik I got via @andlbrei. But the defaults can be adjusted for sure when ever needed. Either by us or the projects.

@andlbrei
Copy link
Contributor

andlbrei commented Dec 6, 2024

Yes, good thoughts. They are based on designs by Erik I got via @andlbrei. But the defaults can be adjusted for sure when ever needed. Either by us or the projects.

@langz can confirm that those are what we are going to use. 😅

@tujoworker tujoworker changed the title feat(Forms): introduce Form.InfoOverlay components for showing error and success messages (receipt) feat(Forms): add Form.InfoOverlay to display error, success (receipt), or custom messages to users Dec 6, 2024
@tujoworker tujoworker merged commit 9dd8402 into main Dec 6, 2024
10 checks passed
@tujoworker tujoworker deleted the feat/forms-status branch December 6, 2024 09:14
tujoworker pushed a commit that referenced this pull request Dec 10, 2024
@semantic-release-bot
chore(release): 10.60.0 [skip ci]
b916b0d 
## [10.60.0](v10.59.0...v10.60.0) (2024-12-10)

### 📝 Documentation

* **Upload:** adds `exists` to file object ([#4346](#4346)) ([52f8b2f](52f8b2f))
* **Upload:** adds exists to docs ([b6baa64](b6baa64))

### ✨ Features

* **Field.Upload:** add `onFileClick` event ([#4369](#4369)) ([c892eec](c892eec))
* **Forms:** add `Form.InfoOverlay` to display error, success (receipt), or custom messages to users ([#4357](#4357)) ([9dd8402](9dd8402))
* **Forms:** add `onAnimationEnd` property to Form.Visibility ([#4356](#4356)) ([87728b4](87728b4)), closes [#4350](#4350)
* **Forms:** add `onVisible` property to Form.Visibility ([#4350](#4350)) ([41306d8](41306d8))
* host fonts in the public directory ([#4359](#4359)) ([e6e08b2](e6e08b2))
* **Upload:** add support for async `onFileClick` event ([#4370](#4370)) ([82588c1](82588c1))
* **Upload:** adds `onFileClick` event ([#4365](#4365)) ([c5abd0e](c5abd0e))
* **Upload:** adds support for async `onFileDelete` ([#4351](#4351)) ([f41e42d](f41e42d))
* **Value.Upload:** add `onFileClick` event ([#4367](#4367)) ([56e9caf](56e9caf))

### 🐛 Bug Fixes

* **Dropdown:** enhance height calcilation and add support for strict `direction="bottom"` usage, including when used in a Dialog component ([#4368](#4368)) ([32b7b5b](32b7b5b))
* **Field.Upload:** handling of multiple async file uploads ([#4360](#4360)) ([5cb1518](5cb1518))
* **Forms:** avoid unnecessary rerenders in Form.Handler ([#4363](#4363)) ([7de5e49](7de5e49)), closes [#4357](#4357)
@tujoworker
Copy link
Member Author

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

@langz langz langz approved these changes

Assignees
No one assigned
Labels
released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tujoworker @andlbrei @langz