Love this module, but we're losing users on errors caught in validation checks. If a field fails a validation check -- for example, by putting alphabetic characters in a numeric field -- and that field is on one of the back tabs, when the user saves the form, they see an error message referencing the field, but it's not clear where to go to find the field in error.

If we could highlight tabs containing fields with errors, it'd be much easier for users to see where they need to go to fix the errors.

I don't know JavaScript, otherwise I'd offer up a patch. Poking through the code, it looks like this could be done by:

- capturing what fields are on which tabs in vertical_tabs_form_alter,
- in theme_vertical_tabs(), using form_get_errors() to get a list of active errors on the form,
- using that list in theme_vertical_tabs() to change styles.

This is somewhat related to http://drupal.org/node/363279, which proposes putting red asterisks on tabs containing required fields. But highlighting tabs goes a step further to cover all kinds of form errors, not just required fields.

Does this make sense? How can I help?

CommentFileSizeAuthor
#4 highlight_errors_vertical_tabs.patch996 bytesdigi24

Comments

digi24’s picture

digi24 commented
Version: 6.x-1.0-beta2 » 6.x-1.x-dev

Yes, I need this too.

I am right now looking at these patches #280627: Add "error" classes to tabs with errored fields and switch to problem tabs maybe we can mimic the functionality.

digi24’s picture

digi24 commented
Assigned: Unassigned » digi24

I have made a patch, it's basically working, just need one or two days to make sure it does not create other problems before I upload it.

crea’s picture

crea commented

subscribing

digi24’s picture

digi24 commented
Status: Needs work » Needs review
StatusFileSize
new highlight_errors_vertical_tabs.patch996 bytes

Patch is attached. I shamelessly borrowed most of the logic from #363279: Add a user-defined "required" indicator to tabs containing required fields. I hope it works, as I have only tested it on an installation that had some other patches and functions included, shouldn't be hard to fix though.

Currently the css attributes are set directly to the element style, it would be nicer to have them inside a css class. The appropriate js is already included and commented out. Just switch it and add class to vertical_tabs.css (and please tell me, which classname actually worked, I always had the problem that it was partly overriden by some other parts)

crea’s picture

crea commented
Status: Active » Needs work

I'm testing it now.
Please rework the patch: remove inline styles and leave only "error" class as you have in commented line.
As for theming, something like this will work:

ul.vertical-tabs-list li.error {
  /* background-color: #FFEEEE; */
  background-image: url(images/messages-error.png);
  background-position: 180px 5px;
  background-repeat: no-repeat;
}

Basically it's theming of "error" messages ('images/messages-error.png' is just icon of error message in my Zen subtheme).
Note that I commented background-color out, cause it may mess up "selected tab" theming. Probably a border will be needed too, but imo too many red color on a page sucks (red color is very aggressive), so I decided to leave icon only.

digi24’s picture

digi24 commented

Thanks crea, I will provide a patch soon, however I personally do not like it that much, as it could be irritating when also using a red asterisk to mark the required fields.

I still have the css on my to do. The reason why I did not implement it in the first place, were problems with the red border. I was not able to create a full border by just using a css-class. Still have to investigate, whether it was just a theme-related issue or a general problem.

ptoly’s picture

ptoly commented
Status: Needs review » Needs work

subscribing

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Closed (duplicate)

Duplicate of #607752: Highlight vertical tabs when they have elements with errors. Feature requests must be fixed in core first.

crea’s picture

crea commented

This sucks for someone like me, who doesn't care about core much. Very sad.
So is it ok to create fork if noone implements that in core ?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

You can write a freaking patch for core instead of forking.

Don't care about core? Give me a break. You wouldn't be able to use Vertical Tabs without core.

crea’s picture

crea commented

I mean D7 will be useful for me like in 2 years. D6 was released long ago and still there are too many issues, core included. Having general "D7 fix -> D6 backport" policy is reasonable, but VT is contrib, and is much easier to fix. So you can have young/unexperienced developer to fix it (like we have here digi24), but it can be totally discouraging for him to hear "go make core patch". We all know core development is whole different story of much higher complexity.