Inline form errors for accessibility and UX

Just in case people have not seen these yet, there are two modules in contrib that attempt to do this.

http://drupal.org/project/inline_messages
http://drupal.org/project/ife

I have not yet tried these out, but they both have D7 branches.

oops .. uploaded the patch twice instead of the screenshot

The last submitted patch, 1493324.patch, failed testing.

The last submitted patch, 1493324.patch, failed testing.

The last submitted patch, 1493324.patch, failed testing.

The last submitted patch, 1493324.patch, failed testing.

Here is an update on the current patch and the remaining issues:

• We should really be checking if an element has an error long before we get to the theme layer (right now I'm checking in theme_form_element) but I'm not sure where the right place might be. I suppose a preprocess function would be better, but still not good. Ideas?
• I have not tested this at all if a single element has more than one error.
• The link in the top error message does not work in overlay at presentThere is a bug in overlay when you click an anchor link after you have already clicked one. See #1542472: Clicking on multiple anchor links while in overlay causes a page refresh potentially causing form data to be lost
• Currently I am adding "messages" and "error" classes to the form element but that is not correct. Should I add these class definitions to system.css? If not, where? Fixed
• A from that redirects the user on submit even if an error occurs is not handled properly. For an example, try using the search block without entering any keywords. You should be directed to /search/node and see this error: Please enter some keywords. Since we are no longer using drupal_set_message at the time of the error is discovered (via form_set_error) the error is no longer stored in a session early enough. Should be fixed by this patch.
• I would like to add "next/prev error" anchor links below each element that has an error. This should be relatively easy once we figure out the issues above.

The last submitted patch, 1493324.patch, failed testing.

@Bojhan: I added the icon because the error and the title looked confusingly similar

In other news, the current patch has issues with checkboxes/radios ... both the group and the individual form elements get the error state. Wont have time to look until later this week.

@mgifford, My feeling is that we should add both the ARIA and HTML5 markup to indicate required fields. However, I believe that is a separate issue and we should not try to tackle it in this issue. I know the markup for required fields introduces some complexity about the ability to use the "Cancel" or "Previous Step" buttons in multi-step forms, so it should definitely be a separate issue that we address carefully.

I clearly have not had a lot of time to try and plow through some of the roadblocks I've come across already ... I still plan on working on this, but I dont want anyone to wait on me if they want to take a swing so I'm unassigning myself

Linking to related issue about validation of complex form elements #179932: Required radios/checkboxes are not validated

@bleen18 - are these types of errors all tied to compound forms that probably should be in a fieldset anyways? #504962: Provide accessible labels (was "Use fieldset and legend") for compound form elements

I'd just like to know how to repeat that nasty error you got. Not opposed to the #hide-errors. Something we might be able to talk about on the weekend at the a11y code sprint.

This patch:

• Adds a simple error message (drupal_set_message) with links to the individual errors
• Adds the detailed error message to each individual element that has an error
• Introduces a new FAPI property to all elements called #hide_errors. When TRUE, the inline error is not displayed with the element. This might need to be thought out further but it is needed in some form to get radios & checkboxes to work
• Next/Prev error navigation at the bottom of each element

Still to do:

• Tests
• Think through the ramifications of the #hide_errors property
• Documentation (after patch is committed)

Important Notes:

• The attached foo module alters the admin/config/system/site-information form for easier testing
• I'm setting this to needs review to see how badly the tests are failing.
• It would be great to get some feedback and get some folks to apply this patch and start really testing.

@bleen18 Thanks for keeping this issue moving, I really love to get this in soon so we can usability test it.

I don't really like that during this thread, we tend to randomly add functionality. It's taken quite a while to reach concencus on some of the fundamental parts, lets get those in first. A quick review;

1 ) Lets lose the icons, per field - I never liked the icon, and it only adds a whole lot of clutter on the page.
2 ) The error should be next to the label, not above.
3) The next/previous is to me, functionality that hasn't been agreed upon - so lets keep that out of the patch.

I honestly, don't see how the fieldset is adding anything. If I am color blind, I have no idea what a "gray" box around a field communicates. As some who can see this color, it adds a whole lot of unnecessary clutter. I thought we just needed some signaling, like a "!" in front of every error.

The links in the error-messages area aren't very easy to use if you're a sighted keyboard-only user.

An example scenario: visit /admin/people/create, and trigger an error by providing a username which is aleady in use (e.g. admin). When the form is returned with errors, try to correct them using only the keyboard...

1. I use the skip-to-main-content link (TAB, ENTER).
2. Next, I TAB through the error messages until the "username" link has focus, and follow it by pressing ENTER.
3. The page shifts, and now the Username text input is at the top of the view-port. So far, so good.
4. However, I don't see any indication of focus. If I start typing some letters, nothing happens. Clearly the text input hasn't been brought into focus.
5. So I press TAB
6. What the heck? Now the email address text input has focus. That's not the input I was trying to reach.
7. So now I have to go backwards through the focus order. I press SHIFT + TAB, and finally arrive at the username input.

(This test was using Firefox 13 on Linux + KDE.)

Currently the error-message links use the id attribute of the elements as their href. Using the link changes your place in the tabindex order, but it doesn't actually focus the input.

Suggestions:

• Instead of using the id attribute of the target input, we could generate an id attribute for the associated element. Assuming the label precedes the input, TAB should then focus the correct input. However, it's probably unwise to assume the label always precedes the input.
• Alternatively, we could generate an id for the form-item wrapper div, and target that. The desired input would then come next in the tabbing order. I think this would be more reliable.

Another weird behaviour I sometimes observed: after following the shortcut to the username field, the page shifted in the viewport, and the username input was concealed beneath the toolbar, so even if it did gain focus, you wouldn't be able to see what you were typing.

Well if you really want that kind of link inside error messages, why not a link "return to top" instead of next/previous? You don't have to scroll to the top.

That said prev/next feel way to "invasive" to put by default in core.

I agree with Bohjan. It seems focus gets lost and we are not maintaining focus on the primary issues at hand.

This issue is about improving the accessibility and usability of form errors. I appreciate that there was a lot of work done prior to my getting involved in this issue and I don't mean to minimize that but at the end of the day we all want an improved experience, right? That said, I don't believe there was any consensus around either of these two issues in the wiki for this issue.

1) regarding the position of the error next to the label: Evan Donovan made this comment, http://groups.drupal.org/node/209513#comment-710049, where he made the identical point I am making: the error should be on a separate line. That comment was not discussed or responded to. No consensus was had...

2) the next/prev navigation: there was no consensus around this because it was never discussed. In the wiki the only example of a "very long form" is a form that has only 7 fields. This is minuscule. There was no discussion at all about how this would work with a form that has dozens of fields. I maintain several modules that have forms like this (DFP and Dart for instance) and without the next/prev nav (or some other solution) I would argue that what we have now is more usable (though definitely not as accessible). Disclaimer: both of those modules use v tabs heavily to improve the UX of the large forms but even still there are individual tabs with far more than 7 fields

I understand that there was discussion ahead of time on this issue and I have been involved in several d.o. issues where someone has come along and tried to derail a consensus but I have also been involved in issues like that where someone has come along and derailed the consensus because something important had not been considered. The reason I am being stubborn here (you can look back at other posts of mine and see that I am never stubborn like this) is because it is this exact UX problem that my users suffer and that I want to see fixed (errors on very long forms).

Consensus is good. Consensus is important. But a good consensus should not prevent us from making good adjustments. If you don't think these are good improvements, please make those arguments and I will gladly listen but don't just dismiss them because no one brought them up 4 months ago.

Final note: this is my last plea... If people still think we should just do exactly what is in the original post without any changes at all I will not argue after this. Although I've always though that stepping up and actually bringing the code held more weight in this community...

Ok .. these are some discussion points I can work with :). Lets see if we cant go through them and either come to a general agreement why these links should stay, change or go...

1) Yes, you are correct. these links are most useful for a form with several errors. If there is only one error, no links would exist and if there are only two, the first one would only have the "next" and the last would only have the "prev". I agree that in a majority of cases no one would see any of these links in which case we're both happy :) In the rarer case of multiple form errors however, that is when these links are most useful

2) I've tried writing a response to this one a couple of times and so far I have (internally) vehemently agreed with you and vehemently disagreed ... It just feels like if we can make it easier we should. One thing I will say is that if we ultimately decide to nix these next/prev links perhaps we should keep them for keyboard users the way that the "skip to" links work - I'm not quite ready to concede the point yet, but it's worth thinking about.

3) This one I dont understand. You briefly mentioned fieldsets earlier in #29 (I think) and I forgot to ask about it then, but as it stands there are no fieldsets being added anywhere. Can you elaborate.

4) My argument here is (again) it only takes up that extra real estate when it is useful ... and scrolling on a mobile device (an already crappy experience) is exactly the time I think this functionality is most useful.

Does the fact that these nav elements only appear when there are multiple errors help alleviate your concerns? Also, a disclaimer: I barely gave any thought to the positioning/styling of those next/prev links so if there are suggestions on that front that might help to settle this that could work too.

Thanks for talking this out with me ... I'm sure that we'll come to a good conclusion at the end. In the mean time I'll try and spend some time this weekend on the issues raised in #30 & #31 as this is all moot if those bugs dont get squashed.

We seem to assume we're going to have a form with lots of errors here, I agree with bojhan that for the typical use-case this is not true.

Showing form validation errors is an UX fail in and of itself. It means there is no client-side validation (HTML5 can do that to some extend) and that the descriptions/labels were confusing, not explicit enough.

While I understand your point bleen18, I think making the HTML5 integration of form validation would be a better way of addressing this issue because if you can reduce the number of errors to begin with, this simplify the problem. When validation error do happen the agreed upon way of dealing with it might not be the best, but it's good enough. What's important is that contrib is still able to tweak it and add prev/next buttons to form errors.

Regarding the prev/next links discussed in 38 and 39, my feeling is that they are more harm than help. They add a lot of UI clutter/overwhelm without delivering much help to users. At the top of the page there is a summary of the errors with a link to each erroneous field. Any user who is having trouble finding which fields have errors can return to the top of the page and use those links. Scrolling back to the top of the page is a quick gesture on a trackpad or a 1-finger touch at the top of an iPhone/iPad screen.

@bleen18, I do want to thank you for your passion to make this UI good and your willingness to explain your arguments and work through this decision together.

Ok folks ... I need some guidance:

This patch does 3 things:

• Adds a function (which is ugly, kludgy and needs to die a fiery death) called form_element_get_title(). This function takes a form element and returns the title, or it searches the element's children until it finds a title. This solves one of the issues brought up in #30 about relying on title. This solution is crappy because it might choose a pretty random title that is not so helpful; we could still end up with no title if the element nor any of its children has a title; to add this ridiculous function makes my brain unhappy. So far the only viable solution I can think of for this issue is to make #title a requirement for all form elements but this would be a HUGE change and probably not a very welcomed one. YOUR FEEDBACK HERE
• Adds the #hide_errors property to the password/confirmation form. This highlights my concerns (raised in #40) about the fragility of this new FAPI property. We will now need to be on the lookout for form errors that may get duplicated and add #hide_errors to that form element with no good way to find all the occurrences in core where this might be an issue. This will also effect contrib down the road. YOUR FEEDBACK HERE
• Removes the next/prev links

Requiring title would work great but I like your suggestion of "error in this text field" as a fallback.

As for your point about hide errors...

• This effects radios as well as checkboxes as well as the passwords as well as any complex custom form elements that contrib has... This is why it makes me nervous, but I can't think of a good alternative
• Setting parent[errors] does not seem possible as far as I can tell because the children are checked for errors before the parent. If you have thoughts on how that suggestion might work I'd love to hear it ...

More feedback welcome.....

We likely want to limit this additional processing to non-programmed forms.

I'm not sure what you mean by this... can you elaborate?

Need to think about this idea some more, but FWIW, I think that this should rather read #error_use_parent or similar. #hide_errors is absolutely misleading.

Are you mulling over the idea of using a property like this? or just the name of the property (which I agree needs changing)? I dont want to keep going too far down this rabbit hole if this solution is ultimately a dead end.

re: #49:

i) I meant that the specific call to form_display_errors() I referenced should be wrapped in a if (!$form_state['programmed']) condition.

btw, it looks like you can skip the else, because if the initial if condition is positive, then the call to drupal_redirect_form() will end the request.

ii) I meant the name of the #property only.

Overall, I need to spare some time to give this patch some more thought though. However, it would still be a good idea to get these known issues out of the way already.

oops ... forgot to attach patch

Looking at @yatil's js code now - https://github.com/yatil/accessible-form-js

Probably not something we can draw on, but interesting model.

Could an exception be made for search?

@bleen18 Are there other examples where where we might need form validation done there?

search_box_form_submit() is the exact definition of broken code. ;) I wasn't aware we have that in core. Let's create a separate issue to fix that (but check for a possibly existing first, please).

@sun can this patch proceed on it's own now that there is a separate issue for search_box_form_submit()? Also, I'd like some ideas on how to better tag @bleen18's new patch. Not sure if a WTF tag is the best way to get folks to look at and review this issue.

I'd just like to see that we can't have better inline form error handling in D8 at the moment and want to see that we address these. We could mark it as postponed possibly, but not sure how related it needs to be.

Need to use soft-tabs, your editor is inserting tab characters.

#1789768: search_box_form_submit() improperly calls form_set_error() Kindly needs a careful review, please. =)

The last submitted patch, drupal8.form-error-inline.66.patch, failed testing.

#66: drupal8.form-error-inline.66.patch queued for re-testing.

re #68: ... to test I used firebug/chrome to remove the "required" attribute from the input manually after the page had loaded. Not ideal, but Drupal doesnt know the difference and it stops HTML5 validation from pestering you

Can someone re-roll @sun's patch from October? There's more progress now on #1789768: search_box_form_submit() improperly calls form_set_error()

The last submitted patch, drupal8.form-error-inline.74.patch, failed testing.

The last submitted patch, drupal8.form-error-inline.76.patch, failed testing.

The last submitted patch, drupal8.form-error-inline.76.patch, failed testing.