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

[D8][UX] Add inline form errors #1040

Open
jenlampton opened this issue Jun 27, 2015 · 33 comments · May be fixed by backdrop/backdrop#4334
Open

[D8][UX] Add inline form errors #1040

jenlampton opened this issue Jun 27, 2015 · 33 comments · May be fixed by backdrop/backdrop#4334

Comments

@jenlampton
Copy link
Member

jenlampton commented Jun 27, 2015

This is part of #378 as it is now a feature included in D8 core (d.org issue):

screen shot 2018-10-28 at 10 16 15 am

This was something that tested really well on the D8 UX study, and we should consider adding to Backdrop as well.

User interface changes

  • Form validation error messages are displayed below the respective form fields that have triggered them.
  • A brief indication of the number of errors, and a link to each error is displayed in the messages area at the top of the page.

Contrib modules


~~ Advocate for this issue: @klonos ~~

@klonos
Copy link
Member

klonos commented Jun 28, 2015

Definitely yes 👍

@Dinornis
Copy link

Dinornis commented May 6, 2017

👍 Yes please.

@klonos klonos added this to the 1.8.0 milestone May 6, 2017
@klonos
Copy link
Member

klonos commented May 6, 2017

I say lets add this to the list of 1.8 targets. I'd absolutely love it if we could get this + #2430 by then.

@jenlampton
Copy link
Member Author

jenlampton commented May 6, 2017 via email

@klonos
Copy link
Member

klonos commented May 6, 2017

😞 ...really sad face if we'll have to wait until 2.x for these two UX issues (especially since we have not even set a date for wen we'll start working on Backdrop v2).

I know that they are both marked as feature requests and that they would introduce some considerable UI changes, but these changes are to solve some long-standing UX issues; not only in Backdrop, but since D7. Both address a similar issue: the message that we are trying to convey to the user on actions they need to take ("validation error in certain fields"/"changes not saved yet") is hidden from the user's view and they either have to scroll up/down to see what's wrong, or they simply leave the form without saving it and then wonder why their changes were not applied (in the case of #2430).

@jenlampton
Copy link
Member Author

jenlampton commented May 6, 2017 via email

@klonos
Copy link
Member

klonos commented May 6, 2017

Fair enough. I wish that we had the experimental features thing in core then. That way we could get reference and the inline messages modules in. People could enable and use them if they want, but they'd be disabled by default. Isn't there a way to do it in a non API-breaking fashion?

@jenlampton
Copy link
Member Author

jenlampton commented May 6, 2017

I wish that we had the experimental features thing in core then. That way we could get reference and the inline messages modules in.

I must admit, I only see disadvantages to having experimental features in core. As contrib projects, they would be updated and improved much faster. Plus, we wouldn't have the pressure of needing to get everything right on the first try (or maintain a legacy of bad choices). But I digress...

For something like inline form errors, that shouldn't even be a module (or something you can turn on or off). Building in extra code to make a UX improvement enable-able seems like a non-ideal solution for core.

As we've been adding modules to core, not all of them remain modules. Token became part of system (i think), pathauto part of path, and entity_view_modes part of Field UI. The same would likely need to be done for ife (or whatever solution we choose).

Isn't there a way to do it in a non API-breaking fashion?

Maybe? But someone would need to look into it to decide if "a non API-breaking fashion" is a good enough solution for core, or if that would be dangerous in any way, or too much of a hack.

@bd0bd
Copy link

bd0bd commented May 7, 2017

My dear developers, could you create a small module for webform inline errors? Or port any module from Drupal (Inline Form Errors, Clientside Validation). Currently I am forced sometimes to use Drupal ONLY because of this option.

@jenlampton
Copy link
Member Author

jenlampton commented May 7, 2017

@bd0bd Would you please create an issue in https://github.com/backdrop-ops/contrib/issues requesting that one of these modules be ported? Once there, please add the tag Port request and that way developers will know that you need it :)

@daggerhart
Copy link

daggerhart commented May 7, 2017

I have an approach to this problem. It's not perfect, but provides a direction that could be taken to provide inline form errors without modifying core APIs. I wasn't sure how to best organize it throughout core as a PR, so I wrote it as a module for now. I agree with @jenlampton that idealy it is not a module.

https://github.com/daggerhart/inline_form_errors/tree/form-validate

It does 3 main things

  1. It adds a #validate callback to all forms. That callback uses form_get_errors() to collect the existing errors.
  2. It then recurses through the form to find the appropriate element that relates to each error, and stores some additional information (#id and #title) about that element in a static array for later use.
  3. On hook_preprocess_layout, it intercepts form error messages from the $_SESSION['messages'] array, converts them into a single error message with anchor links to each field.

This is a little tricky because I wanted the validate callback to run very late, and the preprocess_layout function to run very early. I ended up using a high module weight and theme_registry_alter so that it could execute its hooks at their appropriate time, but I think these tricks could be avoided if it were to become a part of core.

A screenshot of an error-filled form is on the project readme. If this is something we want to explore further as a core feature, I'm happy to rewrite as a PR to core.

@klonos
Copy link
Member

klonos commented May 7, 2017

@daggerhart care to file an application to become a member of backdrop-contrib? That way, you could get that module in contrib-land, and once there is a release, people will be able to install it via the project browser 😉

@daggerhart
Copy link

Just offering one approach that could get inline form errors into core, but I have other ideas as well. I probably should have placed this functionality into core and submitted as a PR. I don't mean for this above repo to be considered a contributed module.

Happy to become a contrib maintainer once I have an idea for something I want to maintain indefinitely, but this isn't that ;). I'll withhold further random-idea-posting on this issue for now. Most of my ideas are probably better hashed out in gitter/other chat before throwing them online.

@jenlampton
Copy link
Member Author

jenlampton commented May 7, 2017

@daggerhart please don't hold back on random-idea-posting, this is a fantastic start! :)

I like your idea of adding a #validate handler. That's a nice clean way we could add something new that may not require adding a module to do so... I wonder if there's a way we could add the #validate to all form elements (instead of all forms) and handle each with ajax, so that we wouldn't need to mess around with layout preprocessing to get and set the inline errors...

@klonos
Copy link
Member

klonos commented May 7, 2017

@daggerhart please don't hold back on random-idea-posting, this is a fantastic start! :)

I second that. It might be long till we get something in core, but if in contrib, people can start using it now. Besides, brainstorming is one of the best ways to solve problems.

I like your idea of adding a #validate handler.

...not an expert dev, but what I like with this idea is that it is an API addition and these are easier to sneak into core 😄 ...we recently added a #help element in #2546

@jenlampton
Copy link
Member Author

jenlampton commented May 7, 2017

@klonos we already have #validate in core, we're just brainstorming ways we might start using it for this purpose.

@daggerhart
Copy link

daggerhart commented May 8, 2017

Back with another approach as a branch named "preprocess-only" in my repo --- https://github.com/daggerhart/inline_form_errors/tree/preprocess-only

This approach accomplishes very similar results, but is better in a number of ways.

  1. It works on the field-level, so theoretically it is more ajax-friendly
  2. It uses the core backdrop_get/set_messages() mechanisms for error message storage, so theoretically these messages are just as reliable as any other.
  3. It only uses 2 functions to make everything happen. theme_preprocess_form_element() and theme_preprocess_status_messages()

It works like this:

  1. In theme_preprocess_form_element() it detects if the current form element has an error within the form_get_errors() array. If it does, it sets a message using a custom $type. backdrop_set_message( $error_data, 'inline_form_errors');
  2. In theme_preprocess_status_messages() we get those messages, and convert them into a single message of the $type 'error', with anchor/jump-links to individual fields. backdrop_get_messages('inline_form_errors');

I'm sure it has issues, just throwing out more ideas.

@jenlampton
Copy link
Member Author

We're two weeks away from code freeze for 1.8, and with no code here yet to review or revise it's not likely this feature will get done in time. Bumping to 1.9.

@domaingood
Copy link
Contributor

+1 Yes please.

@klonos
Copy link
Member

klonos commented Oct 12, 2021

I liked this design (from https://drupalcontributions.opensocial.site):

image

image

@klonos
Copy link
Member

klonos commented Oct 12, 2021

I find the validation error summary message at the top of the form really useful, as it provides an overview of what needs to be addressed. The anchor links that auto-scroll to the specific fields is also a UX+, but once you have scrolled to the first validation error, the summary is now outside your viewport. I would like us to also try to solve that UX issue; perhaps by adding "next error" links?

@klonos
Copy link
Member

klonos commented Oct 12, 2021

This is such a small code change for such a big UX gain. I think that we should aim to get it in for 1.21.0, and I'm advocating for it.

@quicksketch
Copy link
Member

I moved this into 1.21.0 just so we can discuss it in weekly meetings.

@jenlampton
Copy link
Member Author

@bugfolder mentioned that Civi uses https://pear.php.net/package/HTML_QuickForm/docs/latest/ for their inline form errors. Might be worth looking into to see how they did it.

@klonos
Copy link
Member

klonos commented Dec 2, 2021

I've temporarily removed my advocacy for this issue, and moved it to #5348 instead. We need that (and #5346) in first.

@herbdool
Copy link

herbdool commented Feb 11, 2023

I've got a client that did an accessibility audit of their site and got a recommendation to make errors inline with the fields. It did not say they needed be server or client-side generated so I started working on an IFE module port (it basically works). But I ended up testing out @daggerhart experimental module instead: https://github.com/daggerhart/inline_form_errors/tree/preprocess-only (the preprocess-only branch) because I think it might be more robust and didn't need all the settings (yet?)

A solution for core, I think, would probably be better to use something sooner during the element lifecycle rather than in the preprocess step. Not sure where though. And may require also adding aria-invalid, aria-errormessage as mentioned in #5672

@klonos klonos linked a pull request Feb 11, 2023 that will close this issue
@klonos
Copy link
Member

klonos commented Feb 11, 2023

Could it be as simple as this? (doesn't work with all forms, so needs further tweaking): backdrop/backdrop#4334

image

@herbdool
Copy link

@klonos I left a comment on your PR.

I've also cloned daggart's https://github.com/herbdool/inline_form_errors (using preprocess of element) since I think it's the best approach we've got right now. And I need it for a client. Despite the warning on the experimental module it works well enough (with a small bug fix).

I would even put it on backdrop-contrib as a first attempt, but there's no license info on the module. @daggerhart would you be willing to GPL your experimental module?

@klonos
Copy link
Member

klonos commented Feb 17, 2023

Thanks @herbdool 🙏🏼 ...would you expect things to live in a backdrop_preprocess_form_element() function then? (which would in turn live in core/includes/form.inc)

@klonos
Copy link
Member

klonos commented Nov 16, 2023

Noting that recent implementations in Drupal (at least in the Claro admin theme) also highlight the field labels when there's validation errors. A couple of examples:
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment