silverstripe-cms
silverstripe-cms copied to clipboard
silverstripe
Validation messages for hidden tabs
Please see related spike first: https://github.com/silverstripeltd/product-issues/issues/262
Overview
If page validation fails and the error is on a hidden tab it is unclear for content authors what the issue is. For example: a page validator might require that a meta title is required, the field for the meta title is in the "Settings" tab, when a content author saves the page from the "Main Content" tab there is a popup with a validation error message such as "Validation Error" but it isn't clear where the error is, which tab the error might be in. When a content author views the "Settings" tab it is clear that the error is for the meta title field.
Acceptance Criteria
- [ ] When a validation error happens, I can clearly identify the field, even if it's not in the currently active tab
- [ ] Validation errors for all fields are shown in the tab they relate to
- [ ] All validation errors are displayed for fields with validation errors
- [ ] Tabs indicate whether they have validation errors (including nested tabs)
- [ ] Any validation indicators disappear if I resubmit the form with valid input
- [ ] Validation errors in the central place can be dismissed
- [ ] When a validation error occurs in a nested tab, the validation error shows on the toplevel tab and all nested tabs on the "path"
- [ ] This works in React and Entwine
Notes
- Works for Pages, ModelAdmin, AssetAdmin, Security
- For AssetAdmin, this should work for client-side errors as well
- Dismissing the validation errors does not have to be permanent
- Causing new client-side validation errors should only show these in-place (not in a global warning)
- Track under ORB-175
Designs
Out of Scope
- Toast error message
- Styling changes to validation on fields
We experienced this recently with a required field inside a toggled composite field as well I believe.
Hey @frankmullenger, is there a way we can reproduce this on the demo site? (https://demo.silverstripe.org/) Or is it based on gridfield?
I was wondering if you could please give an example of when a meta title field would be in the "Settings" tab?
Within Pages, there is a notification that stops you from navigating away but we do notice with the metadata section that there should be a validation message.
Hi @sachajudd, meta title was just an example I borrowed from our current bespoke project to illustrate the problem really. If we add a required field to a validator and the field is not visible when the error is triggered (it might be "hidden" in a background CMS tab or closed composite field?) then there isn't an indication for content authors where the error might be. Hope that clarifies.
Right, I see! 🙂 I'll look into this more over the next week and see if I can come up with some designs.
cc @clarkepaul
Instead of using popovers for validation errors, we thought using alerts would be more appropriate. The alert would indicate where the error is occurring. A nice to have concept would be to add an icon to the specific errored tab (see "Attention" icon).
Popovers would then only be used for what is currently called "Internal errors" but we'd re-word to suit different system errors (see screenshot).
NOTE See issue description for updated designs
"validation error on [tab name] or [field name]" seems a bit weird - every field will be in a tab, should it be "and" rather than "or"? I think the error count next to the tab is a good solution, and probably means you don't need to mention which tab a field is in, as long as you can link to it from the validation error message (opening the right tab).
How would this look like when you have ten validation errors on different fields? We're obviously trying to avoid this in core UIs, but this is a framework, so there'll be implementations that can cause that many errors in one submission.
In terms of wording, ideally we find something that doesn't need to vary between "item created" and "item updated".
We opted to use (!) on the tab instead of numbers to simplify the UI (and a bit nicer than a tab growing and shrinking when there is more than 10 errored fields.
@chillu do you think its possible to have a link to the actual erroring field from the alert? we weren't sure as to what is possible from a technical prospective but ideally we can link to the field (and if not then the tab as a fallback).
True that we can improve the messaging, "There is an validation error on x field" would be sufficient.
I think it's possible, just a bit finnicky. We'd have to implement it twice for Entwine and React. Assuming we can do that, can you come up with a design that incorporates a list of validation errors, rather than just one of them?
Hey @chillu, here is an example of errors in multiple tabs. Let me know what you think 🙂 ~Design can be found here: https://invis.io/6EGYSGTZD8G#/291645219_Multiple_Validation_Errors OLD LINK~
NOTE See issue description for updated designs
OK, that'll be a pretty long red box when you have a dozen validation errors. Which might not be as rare as it might sound, given that those validation errors should also be triggered before submission. So if you fill out fields on the first tab only, ignoring other tabs with required fields, and hit "save", you might get a really long red box with lots of redundant info. Can we use the actual validation message here?
Example:
[field1 label] is required
[field2 label] is required
[field3 label] needs to be a number
No need to update the designs, just let me know if you're OK with us using the text like that
@chillu Is that a pattern being used in validation messages already? [label]... is required.
I like the idea if it can read as a sentence otherwise it'll need to be something like: Page name: This field is required Meta data: Please remove invalid characters
Yes the notification could be long but didn't want to over complicate the UI either with expand and collapse, we can provide additional designs for this if you think it can be done at the same time (it just isn't default Bootstrap). It would be something like showing the first 5 errors then having a "show more" to expand fully.
Based on last team conversation... As a MVP (if there are complications in creating a notification which leads users to different tabs) we can create a generic notification saying something like "(!) Validation error, please correct field errors", using the icon on the tab to indicate which tab has the validation error.
Errors could be across multiple tabs like in the file edit panel (edit and permissions), so the alert would need to be seen on all tabs with which the item actions control.
@clarkepaul Yep, that's consistent with the ACs. Once we're at that point, it'll be pretty easy to also add a list of validation errors though, so that won't make the card much easier. The spread comes from doing this work effectively twice, once in Entwine for Pages, and once in React for Assets
Would it be fair to say the impact rating of this has gone down as the people it was created for found workarounds @frankmullenger ?
@clarkepaul we don't have a suitable workaround for this currently, there is a notification of a validation error in a popover but no indication of what tab the error might be on.
I was doing some snooping around in legacy JavaScript the other day and I think I saw some code which is supposed to auto open a tab which has validation errors in it. Will report back.
Edit: here's the code I was thinking of - it's supposed to ensure the first field with a validation error is shown. That won't help when there are validation errors in multiple tabs though, which this issue could still resolve.
https://github.com/silverstripe/silverstripe-admin/blob/1.3.1/client/src/legacy/LeftAndMain.EditForm.js#L111-L136
I think the use of the ss-tabset class is lighter than it was in SS3, which is also causing other issues like https://github.com/silverstripe/silverstripe-admin/issues/911.
Related, there's a nasty bug to look out for with this: CMS users lose all block content when attempting to save page with validation errors
Some quick notes of other things related to this that currently are not captured in the description:
- There would be separate implementations for Entwine (pages, modeladmin, security) and React (asset-admin)
- There's no AC for this working for tabs contained in inline-editable content blocks. Are these linkable?
- I'm guessing we don't need to worry about non-inline-editable blocks (and other gridfield items with tabs) as they'll require their own save action and validation check?
I think this is where those errors get populated. https://github.com/silverstripe/silverstripe-framework/blob/a712000404526a7d58fefee73bc7eae71011ea59/src/Forms/Form.php#L485-L499
I'm thinking we can either:
- update tabs so they can read the errors within the field they contain.
- update
Form::loadMessagesFrom()so it sets a flag on the tab as well as the individual field.
