Currently the summary generated from inline_errors_template has no template. Lets add one
Do we need one for form-item__error-message? Technically it runs through the form_element.html but wasn't super clear by the original post and certainly don't want to lose any of the desired request.
Add a template for the summary.
Do we need a new template for the error??
NA
N/A
N/A
Inline error messages are currently using their own custom markup and CSS selectors (in other form elements) and using absolutely no theming mechanism in fieldsets.
They all should be using something similar to: http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Render/Eleme...
Note that it should be using #theme and not #type (which will use any existing stored messages).
It should also use a theme hook suggestion so it can be specifically targeted when needed (such as in a base theme like Bootstrap so it can append the alert-sm class): '#theme' => 'status_messages__inline_form_errors'.
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
jrockowitz commentedComment #3
markhalliwellI'm tempted to say that this is actually a core issue and a fatal flaw of the Inline Form Errors module that was pushed through. It simply passes error messages but doesn't actually use any existing theming mechanisms to display them properly (i.e. setting the variable to a render array that, at the very least, mimics error status messages and uses a theme hook suggestion for the "inline" bit).
See: http://cgit.drupalcode.org/drupal/tree/core/modules/inline_form_errors/i...
Comment #4
jrockowitz commentedYou are right core does include some styling for form element errors via .form-item--error-message and no error styling for fieldsets.
At the same time, the Bootstrap theme overrides the form-element.html.twig template to include custom error styles and the issue is that fieldset.html.twig is not being overridden.
Comment #5
markhalliwellThis is a core issue and oversight.
These error messages are currently using their own custom markup and CSS selectors (in other form elements) and using absolutely no theming mechanism in fieldsets.
They all should be using something similar to: http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Render/Eleme...
Note that it should be using
#themeand not#type(which will use any existing stored messages).It should also use a theme hook suggestion so it can be specifically targeted when needed (such as in a base theme like Bootstrap so it can append the
alert-smclass):'#theme' => 'status_messages__inline_form_errors'.Comment #6
msankhala commentedComment #7
markhalliwellThe IS needs work since it's not for Bootstrap specifically.
Comment #8
markhalliwellComment #13
pameeela commentedUpdated IS based on #5
Comment #14
pameeela commentedComment #20
joseph.olstadthe fieldset.twig.html still doesn't have the twig changes proposed by @jrockowitz
I've added this fieldset.twig.html in my theme but core should have that change suggested by @jrockowitz
Comment #24
smustgrave commentedCame here from #2880011: Add a #disable_inline_form_errors_summary property to disable the Inline Form Errors summary where it was suggested in #69 to add a summary template. So going to give it a shot to help inline_form_errors get polished to be included in the standard profile.
Comment #26
smustgrave commentedDisclosure used AI to help write the unit test since this was a bug and didn't know what to include.
The javascript errors randomly failing has skyrocketed this weekend. But could use some eyes
Comment #27
dcam commentedI'm sorry my idea wasn't working out the way I suggested. I left some thoughts on the MR.
Comment #28
smustgrave commentedI also definitely like the markup being in the template too. We could try and use item-list template but would have to update tests to pass. Also not opposed to.
Comment #29
nicxvan commentedI think we would use item-list template since it is a list.
Comment #30
smustgrave commentedOkay will just need alter the render array, which shouldn't be a problem.
Comment #31
smustgrave commentedWas able to get the template working but did have to edit the PathAliasTest because of some weird whitespace.
Even asked claude if something could be done in the template and the suggestion was to add {{- ' ' -}} which felt wrong.
And obviously had to refactor FormErrorHandlerTest because the list now gets ran through a template.
Comment #32
dcam commentedThis looks great. Thank you for continuing to work on it. The only difference between the rendered HTML from the new template and the old render array is the addition of a
<span>around the error count list heading.I've never added a template to Core before, but I assume it needs a change record to announce that the new template is available.
Comment #33
smustgrave commentedCR added
Comment #34
dcam commentedLet's send it up.
Comment #35
joseph.olstadOnly 8 years, it can wait a few more yet :P
Aren't there many important deprecations to work on? Moooahahaha