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

Show new flash messages until discarded by user #10319

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Jan 25, 2023

What? Why?

We currently have two mechanisms to display flash messages. The old one through AngularJS and the new one with StimulusReflex.

The AngularJS directive showed flashes for 10 seconds. The StimulusReflex controller showed them only for 3 seconds. But any time based disappearance of error messages is problematic. There's important information in there and some error messages can be long. It's also possible that a request takes a while, the user leaves the computer and comes back later. If we hide the flash automatically then the user may have no idea what went wrong. They may even think that everything is fine and their order went through.

I removed the time-based removal of flash messages from the new StimulusReflex controller to address this problem. But I didn't touch the AngularJS directive because it will be removed anyway. There may also be many more messages that could be annoying if they didn't disappear, for example a simple "login successful".

I personally think that flash messages that are not important to keep, don't need to be shown in the first place. The best UX makes the success obvious on the page. And success should be assumed.

Relates to:

What should we test?

  • Visit split checkout page.
  • Don't fill in all shipping address fields.
  • Submit the form and observe the red error message at the top.
  • It should only disappear when you click the x button to close it.

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

We currently have two mechanisms to display flash messages. The old one
through AngularJS and the new one with StimulusReflex.

The AngularJS directive showed flashes for 10 seconds. The
StimulusReflex controller showed them only for 3 seconds. But any time
based disappearance of error messages is problematic. There's important
information in there and some error messages can be long. It's also
possible that a request takes a while, the user leaves the computer and
comes back later. If we hide the flash automatically then the user may
have no idea what went wrong. They may even think that everything is
fine and their order went through.

I removed the time-based removal of flash messages from the new
StimulusReflex controller to address this problem. But I didn't touch
the AngularJS directive because it will be removed anyway. There may
also be many more messages that could be annoying if they didn't
disappear, for example a simple "login successful".

I personally think that flash messages that are not important to keep,
don't need to be shown in the first place. The best UX makes the success
obvious on the page. And success should be assumed.
@mkllnk mkllnk self-assigned this Jan 25, 2023
@mkllnk mkllnk marked this pull request as ready for review January 25, 2023 05:00
@mkllnk mkllnk requested a review from jibees January 25, 2023 05:00
Copy link
Contributor

@jibees jibees left a comment

Choose a reason for hiding this comment

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

I think it's a pretty good idea. I don't exactly know if they're still flash messages then, but I'm happy with that.

Moving to Test Ready, one review is enough.

@filipefurtad0 filipefurtad0 self-assigned this Jan 25, 2023
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 25, 2023
@filipefurtad0
Copy link
Contributor

Hey @mkllnk ,

Nice improvement!

Now the flash-message indeed stays on-screen, till the user clicks the x:

image

(this also makes it much easier to take snapshots of the error messages, while testing 😁 )

Looks great! Merging.

@filipefurtad0 filipefurtad0 merged commit 65d8337 into openfoodfoundation:master Jan 25, 2023
@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 25, 2023
@mkllnk mkllnk deleted the show-flash-until-closed-by-user branch February 27, 2023 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants