Ensure visibility of invalid fields (JS)

Hey maxilein,

I ran into this issue today and after much reading and sole searching I have rolled a patch that might help. Only works for tabs currently but the idea is to scribe to the invalid event and then show that tab before the HTML5 validation takes place. This way the browser can focus the element and avoid the 'An invalid form control with name='xxx' is not focusable.'

Only tested in chrome.

Aaron

Ok first time I have added a patch with a new file, trying again.

Unfortunately this gets complicated once you start nesting tabs / field groups - including something as simple as having a core details element (which some field widgets do, e.g. media_browser) within a tab. I attach a patch that works to some degree, but to be honest, a more general solution is needed, as each kind of tab / field group has their own way of opening/closing.

This patch has been made against HEAD. It only handles vertical/horizontal tabs and standard Details elements, so accordions will not necessarily work correctly. It also loops over all invalid items, so if there are multiple tabs, each with an invalid element, only the last one will get shown. (Once that's been filled in and then the form is re-validated on clicking Submit, the previous tab will get shown.)

So consider this some basic improvement, but some more serious thought/work needs doing really to resolve this in a more general way.

Re-rolled against current HEAD.

Re-rolled against current HEAD.

Couldn't get the patch to work for my horizontal tabs. Made a few changes to get it to work.

I have updated the patch to fix a few errors, and added support for details elements. This whole thing is a bit of a mess and desperately needs to be refactored (and JS be put in the right places) but for now lets focus on getting them all working.
Updates:

• Fixed syntax issue with the new JS in field_group.libraries
• Fixed a problem where only vertical tabs were working
• Added basic support for nested tabs
• Added support for details groups
• Added support for submission validation errors - displays with errors after the form has been submitted

To do:

• Add the rest of the field group elements
• Consolidate/refactor into a single, simpler form validator for field groups
• Find the proper place for the validation JS
• Set javascript to properly on load on forms
• Add tests

I took a look at ChristianEsbensen's patch and didn't see anything missing from my patch there, feel free to re-review between the two.

Patch #9 works fine on 8.0-1.0-rc6 version

Related core issue #2346773: Details form element should open when there are errors on child elements

@webflo Thanks for the reference.
Did the issue #2346773: Details form element should open when there are errors on child elements fixed means the patch in this issue queue will no longer be necessary ?

Lets see what the testbot says.

#9 works for vertical tabs, agreed that the solution needs to be more generic.

Regarding the issue at #2346773, as of 8.2.7, that patch does not resolve the issue. I attempted to back-port the patch, but there were dependencies outside of the scope of that patch that I could not track down. That patch will be backported to 8.3—once it is, we can assess if the issue has been resolved and this may then be unnecessary.

That being, for immediate purposes, the patch in #9 works for now.

Would be nice if this would work for details elements inside paragraphs, also looking forward for a more generic solution.

I'm running 8.3.x and #2346773: Details form element should open when there are errors on child elements (which has been committed to 8.3) did not resolve this issue. The patch in #2787179-9: Ensure visibility of invalid fields (JS) improves the situation, but not fully:

• it doesn't always open the fieldgroup. I suspect that this may be an edge case related to my site's use of the Chosen module for selectboxes.
• it doesn't show the HTML5 pop-up "Please fill out these fields". I'm not sure if this one is an edge case that is specific to my configuration, or more general.

The patch above might be good enough to at least get something working out there that we can improve upon.

Tested on 8.3.x

https://www.drupal.org/node/2787179#comment-11877050

Works for tabs, doesn't work for accordion items.

patch #9 doesn't actually work for our vertical tab configuration, I'm pretty sure it's because the JS is assuming there is a <details> element which may or may not exist.

It might be better to take a step back and think of a more extensible approach than having a different validation library for each type of container type. It seems like you have a few different types of parents that all need to be flagged, and then opened in various ways (or maybe not)

Right now you have two different invalid listeners that will traverse up to find the parent. Instead maybe we could:

- In the plugin handler give a generic class to all container types that have required fields, (right now the class is specific to tab or details)
- On 'invalid' just traverse up for that generic class, and then you might need only one if-statement to check how to open it, or trigger another event?
- Or maybe make each required field aware (through a class) of what the tab or details containers hiding it are so 'oninvalid' might be much easier logic

I hope that made sense at all, I think it's worth discussing a bit more while it's in beta.

I have improved Patch #9, it was failing because of the following reason: I was editing a node with 2 tabs. Both tabs had text field which were required, when I had an error in both tabs it would open the last tab. The problem is that the HTML 5 error would occur in the first tab and thus not be visible for the content editor.

I am certain that this isn't the way to fix the bigger issue and as #21 said we should look further for a more generic solution.

For me patch #23 works on normal fields (username, password) but does not work on address field which we have in user profile :/

Patch 23 does not work in the following scenario for me:

(1) add a non-required image field with a required alt text
(2) put that field in a tab group which is closed by default
(3) try to save a node which has that group

I suspect the patch at #23 only works for certain types of tabs (the ./src/Plugin/field_group/FieldGroupFormatter/Tabs.php and ./src/Plugin/field_group/FieldGroupFormatter/Details.php formatters).

In my case I am using the ./src/Plugin/field_group/FieldGroupFormatter/Tab.php (Tab singular, not Tabs plural) formatter, in which case the code in patch #23 was not being loaded at all.

I have built upon tab 23 by adding a new js file for Tab formatters, see the file "interdiff-do-not-test" for what has changed. The entire patch is enclosed also.

I know @alberto56 fixed something else just for info #26 still does not work with field address

@mkolar thanks for checking; let's set this back to needs work then.

+1

Note that the direction #1797438 HTML5 validation is preventing form submit and not fully accessible is headed is to remove HTML5 validation.

Because of #3011584: trying to get a patch in a script: Access to this page has been denied I can no longer get this patch via curl in a Dockerfile, I have uploaded the patch to Github so if you need it and run into an error via curl, you can run:

curl -O https://gist.githubusercontent.com/alberto56/22540d014f709cda9538032ff1e05ad0/raw/a920149ea7d1bd4a62396466f705d0383a593894/field_group-8.x-1.x-2787179-26.diff

For info here's my attempt of a very simple patch on the 3.x branch, just for the Tabs : #2894351-5: Tabs with invalid input are not focused

Bumping to major, because this relates to WCAG success criterion 3.3.1 - Error Identification at level A.

Are the approaches in these core issues relevant for Field Group module?

• #2346773: Details form element should open when there are errors on child elements - fixed in D8.3 in February 2017.
• #2848507: Indicate that grouping elements have child element errors for ux and a11y

The latest patch posted here (#23) seems to have been included in the latest release 3.0 RC1.

Hello folks!

We too love our users and thus need a solution to this problem of properly highlighting HTML5 validation errors :) Our case is lots of vertical tabs (used on many content types) with some nested fieldsets within each tab. I agree with @mkolar that field address widget wouldn't work with the patch #26. I made a small iteration on top of #26. Unfortunately, #26 wouldn't apply cleanly against latest dev snapshot, so I cannot show you guys an interdiff.txt, but I literally just changed the following within ./field_group/js/field_group.tabs_validation.js

from

            fieldGroupTabsOpen($(e.target).parents('details:not(:visible), details.horizontal-tab-hidden, details.vertical-tab-hidden'));

to

            $(e.target).parents('details:not(:visible), details.horizontal-tab-hidden, details.vertical-tab-hidden').each(function() {
              fieldGroupTabsOpen($(this));
            });

The problem was that if you have nested <details> tags, then the $(e.target).parents() selector would match a collection of jQuery DOM objects (instead of a single element) and the fieldGroupTabsOpen() expects it to be a single object. Just iterating over the matched collection is enough to get by the problem.

Moreover, I do confirm it works on address field widget (and on multiple other forms that we have with heavy use of vertical tabs via field_group.module).

I like where this is going! The patch looks good and does what it should.

Setting to needs work for the tests.

Hi all. The latest path didn't work for me so I refactored it and made it work :)

I have took the patch from #36 and I wrote a functional javascript test for it.
I cover the tabs validation for vertical+horizontal with required fields in both places with a collapsible detail element to check if it is opened accordingly. Please note that the patch will fail on 8.x-3.x-dev due to a different issue, you can see it on the module's page.

patch #40 worked for me.
Thank you so much @joevagyok

The patch on #40 works fine and adds the required tests for the added functionality.

1. +++ b/js/field_group.tab_validation.js
   @@ -0,0 +1,22 @@
   +      $('#edit-submit').on('click', openTabsWithInvalidFields);
   

   We cannot assume an ID here. We should rely on the invalid event.

2. +++ b/js/field_group.tabs_validation.js
   @@ -0,0 +1,34 @@
   +            $inputs.off('invalid', onInvalid);
   

   If we want to do this, we need to namespace the event. Something like invalid.field_group.

3. +++ b/js/field_group.tab_validation.js
   @@ -0,0 +1,22 @@
   +      var openTabsWithInvalidFields = function() {
   

   We should use the context parameter and once library to avoid applying the behaviour multiple times.

4. +++ b/js/field_group.tabs_validation.js
   @@ -0,0 +1,34 @@
   +    attach: function () {
   

   Same here.

5. +++ b/js/field_group.tabs_validation.js
   @@ -0,0 +1,34 @@
   +            } else {
   

   We could use an else if here to make it a bit more readable.

6. +++ b/tests/src/FunctionalJavascript/EntityFormTest.php
   @@ -0,0 +1,219 @@
   +        'label' => 'Tab 1',
   

   This should be Tab 2.

7. +++ b/tests/src/FunctionalJavascript/EntityFormTest.php
   @@ -0,0 +1,219 @@
   +    $this->assertSession()->fieldExists('field_test');
   

   This method returns the field if found. We can use that in the next line to retrieve the xpath instead of using the machine name.

8. +++ b/tests/src/FunctionalJavascript/EntityFormTest.php
   @@ -0,0 +1,219 @@
   +    // Assert that the field_test is not visible because it's in the first tab.
   

   Same as before.

Also as stated in #9, some field group elements are not supported yet so we cannot mark the issue as RTBC. Maybe we can rescope this issue and fix the main ones and have followups for the remaining.

Reroll of the patch from #40 against current dev.

@gease, thanks a lot. Could you throw an interdiff as well with your patch, please?

@joevagyok, it's a reroll of patch from #40 against recent dev version, because the latter didn't apply any more. There's no interdiff then.

Ah right, I see. Thanks :)

I was able to create interdiff between 40 and 45 patch.
Here i am uploading one.

Thanks for providing the interdiff @tapaswini sahoo. You have removed the 2 patches in subjct from visibility so I put a bit of order with the patch files now for more clarity.

Thanks for the work on this! When using #2787179-45: Ensure visibility of invalid fields (JS) I run into situations where tabs groups are emtpy for anonymous users.

Steps to reproduce

1. Add tabs group + 2 tab elements
2. Require fields inside the second tab tab
3. Set the first tab as open by default
4. Login as anonymous and see that inside the second tab the fields are hidden (in one setup also the first tab)

As you can see in the screenshot the open="open" attribute is missing. I don't see any errors in the console.

I ran into a similar issue as #51 but did not have to be anonymous.
Similarly I had a group of type tabs with multiple groups of type tab nested within.
There were multiple required fields in the second tab that wasn't opened by default;
specifically, there were two fields that use fieldsets and one that didn't.
If I went to open the previously mentioned tab, all the fields were hidden.

I am not sure that the attached is the solution but for now it seems to address my issue.

Updating to handle invalid multiselect fields. A quick test looks like it may also handle multiple tab levels as outlined in #52.

I faced on hidden tab contents (for second tab and above) when using #53 on radix theme (seven was behaving properly). I wasn't expecting that issue using #52.

No reroll necessary, my bad. ✌️

Tested with Tabs, Accordions & Details – works very well.

The only issue I could notice was that Details Sidebar groups don't expand to show the browser's validation message so I guess .field-group-details-sidebar should also be considered in the JS.

The closed required tab issue from #51 and #52 is solved by patch #52 but not by the latest patch #53, because it was an update of patch#45 instead of patch#52.

So we need a new patch based on #52 which also addresses issue #53.

I have seen in my project with #53 that openTabsWithInvalidFields() function gets called when any ajax is triggered.
That resulted in it hopping to different tab when ajax get triggered, so I removed the line that calls openTabsWithInvalidFields() when the form is first loaded

edit: see #59 for correct patch file

Edit #58: My bad I see I have submitted the wrong file.

#59 works great but as mentioned in #56 the sidebar is not taken into account. The following patch updates to add the sidebar in.

I also updated so that if the sidebar is closed we show it again before throwing the error.

NOTE: I do still see an issue when you have a collapsed fieldset with required fields in it. The node form validation applies and opens the fieldset from core after save. But this happens after you pass other inline validations so you think everything is solid and then you get the form validation upon save.

#40 Worked for me. Thank you

The latest patches have visibility issues for horizontal tabs. Tested 60 and 59 without luck. After reading some comments thought to jump to #52 and this particular patch appears to consistently address my needs for horizontal tabbed content building. The con is it doesn't include fixes for the sidebar menus. Definitely needs finesse to be working consistently for both scenarios still.

Patch #40 works for me.
Thank you @joevagyok !

Patch against latest develop (version 3.2) based on the patch #52 has been attached.

I've reworked JS approach from #52. Hope it is more efficient.

Tested #67 and works fine. Thank you @sinn!

For me and my situation (multiple vertical tabs inside one horizontal tab of many horizontal tabs) patch #67 isn't working.

Slight optimization of patch #67.

Drupal Core has moved away from using $this-xpath() in tests, since they aren't always testing what we expect to test, see #3220255: Convert assertions involving use of xpath on links to WebAssert.

Also: On my local Windows install both time $this-xpath() is used in the new test, it fails.
This works for me locally and should also work on TestBot. Let's see...

Ok, final alteration of the excellent patch #67, chosen some (hopefully) better wording of the extracted helper function name.
(Sorry for the noise)

I think this already a massive improvement as is, especially with the addition of a test.
So a big thanks to all involved!

It would be nice if this test (and of course also the fix would consider) the use-case of vertical tabs inside horizontal tabs and vice-versa, but I'm totally OK with handling that in a follow-up issue.

Patch from #72 worked for me, thanks.

Found the solution for my use-case and I think it's a better, more generic approach: Don't use click(), but set the correct attribute to "open".
This prevents, as in my use case, that an already correctly opened tab was closed because of a click() on it.

All other changes are moving the test to the correct namespace and some (hopefully) better naming/documentation.

Here's a fix for Gin Theme tabs.

Forget about the patch #75, here's the final one for Gin Theme tabs.

For what it's worth, manual testing on horizontal tabs on the Seven theme on Drupal 9.2.6 seems to indicate that the patch in #74 does the trick.

I was hoping to come here to advocate that this patch be committed, but I think there's some fundamental problems that need to be resolved first, and not to do with the actual code:

1. There are patches on this issue, but there's also an issue fork. If I follow this correctly, there is not actually any commits on the fork??? https://git.drupalcode.org/project/field_group/-/merge_requests/6/diffs
2. @lucasgrecco can you include an interdiff so that we can see what you've changed?

To update the submit button classname instead of element ID

#74 Work for me with Adminimal Theme

Is this also meant to work with inline_form_errors? I tested #74 on a site with field group tabs. On validation, when there is an error in a field inside a tab that is not currently opened, the link to the field does not work. It only works for the currently opened tab.

Using Field Group 8.x-3.2 and Gin 8.x-3.0-alpha37 with a tabs group in the node form, got this issue.

Only patch #77 worked fine for me (thanks for that). #76 didn't work at all, and #74 kind of worked; the closed tab (with the required field) appeared but the previously opened tab also remains open.

Not sure how a patch for two (perhaps more) different themes could be worked out. Would we need to involve theme developers, in my case from Gin?

Today I discovered that patch #77 causes a different issue.

Take for example a form with 3 tabs: "One" (which is open by default), "Two" and "Three". There is a field "Image" inside the tab "Two", that uses the Media Library widget (which opens a modal window). If you click the tab "Two" to edit the "Image" field, then you open the modal, the active tab will automatically be changed to the first tab with invalid fields ("One" or "Three") as soon as the modal is closed.

Shouldn't this behavior be restricted to the submit event handler?

#74 works for my needs, but I would argue that the validation for collapsed details elements should be applied to core so that this issue is resolved for any details elements, not just ones built by field groups.

I have added Core issue 3333481 to port the collapse solution of #74 to core which ends up working for both custom / core details elements as well as field groups.

Updating the patch of #74 to use the new once library (see change record), so that patch is compatible with D10.
Also fixed two 2 deprecation warnings.

This issue appears to be trying to solve the same as this issue https://www.drupal.org/project/field_group/issues/2969051 .. I don't think we need two issues open trying to solve the same thing, unless i've missed some difference?

This patch from https://www.drupal.org/project/field_group/issues/2969051#comment-15022271 in that issue appears to solve the issue as far as I can see from my testing with 9.5.x.

We were previously using patch 60 from this issue on a project which at some point in the last year and a half seemed to stop working in all circumstances. Patch 99 (from comment #100) in the other issue (2969501) is working nicely though.

Patch #85 works for me for fields that are in both inside and outside tabs. But the patches on #2969051: Fix HTML5 validation prevents submission in tabs don't work for fields inside tabs.

Note that issue #2894351 also try to fix this bug. Patch 26 works pretty well with Seven/Claro/Gin admin theme

source: https://www.drupal.org/project/field_group/issues/2894351#comment-14394498

Adding related issue mentioned on #86. Please test the patch on #85.

Can confirm that patch on #85 works for me on Drupal 10.1.8 and unpublished_node_permissions 1.4.0

Patch #85 works fine for us. Moving to RTBC.

I have added a required field in tab , but if tab is close than the validation is not working for the required field and getting error in console. i have applied this patch #85 , and this working fine for me. +RTBC

The MR is empty, could someone please create a working MR from the RTBC'd patch?

We have the full HTML required field & I am not able to set the focus. I have tried the latest patch #85 also.

We are using the D10 version.

Please provide some insights.

@Anybody Done, patch #85 already uploaded to module issue fork, also updated latest code from 8.x-3.x. So now MR is updated.

@nikky171091 Are you using field_group version 8.x-3.4?

@rcodina thank you! Sadly tests are failing currently.

The tests seem to be fixed in #3430494: Automated Drupal 11 compatibility fixes for field_group - Fixed PHPUnit on GitlabCI, in particular with the changes in file contrib/field_group_migrate/tests/src/Functional/MigrateUiFieldGroupTest.php, see:
https://git.drupalcode.org/project/field_group/-/merge_requests/49/diffs...
maybe, @Anybody, it might be worth taking a closer look at this other MR first (MR!49) so phpunit tests could get fixed and run for other tickets.

I would recommend getting the ESLint job fixed first in #3448470: GitlabCI: Fix ESLINT validation errors (MR!54), before getting this MR approved:
it seems it would introduce quite a few linting errors, mostly in the JS files.

Otherwise, @rcodina, you could try cherry-picking a commit from another MR, I've used to pass the tests, see :
https://git.drupalcode.org/project/field_group/-/merge_requests/47/diffs...
Or simply download the file locally and try running the phpunit tests, then commit the file and re-run the tests.

Hope that helps moving the ticket forward.
Thanks in advance for your feedback!

patch for issue #2969051 works (even for the fields inside the tabs comment #87)

This is the patch to the latest 8.x-3.6 stable version

@Anybody All tests and checks of the MR are green except the following one reported by phpstan which reproduces in 4 files of the module. This 4 files haven't been modified by current MR:

Class Drupal\field_group\Element\HtmlElement extends deprecated class  
         Drupal\Core\Render\Element\RenderElement:                              
         in drupal:10.3.0 and is removed from drupal:12.0.0. Use                
           \Drupal\Core\Render\Element\RenderElementBase instead.    

So, given this is not related to current issue and that core requirements of Field Group should be changed to solve it, I think this should be addressed in a new issue.

Please ensure that this won't conflict with #1797438: HTML5 validation is preventing form submit and not fully accessible

Tests look good, I think we're almost close to the finish line here?

@rcodina said in #87:

Patch #85 works for me for fields that are in both inside and outside tabs. But the patches on #2969051: HTML5 Validation Prevents Submission in Tabs don't work for fields inside tabs.

Both this issue and #2969051: Fix HTML5 validation prevents submission in tabs, seem to fix a very similar issue (if not the same).

The fixes here seem to be more sophisticated, though. I'd say, we merge the MR opened here (once tested) and see if anything is left to do in
#2969051: Fix HTML5 validation prevents submission in tabs.

@Anybody, I think you meant to not conflict with #2969051: Fix HTML5 validation prevents submission in tabs instead of #1797438: HTML5 validation is preventing form submit and not fully accessible ?

Right!

Works like a charm, great work everyone! 🤩

I was working on re-rolling #2969051 but saw the latest comment on that ticket and came to see if this fix resolves my problem.

I have three tabs all with required fields. The first tab is the default shown tab so no issue there. When I try to submit with the 2nd tab fields not completed I am correctly brought to the tab and field. However, if I fill that in and submit again, I am not pushed to the 3rd tab and instead see the server side validation.

Here is the video of this interaction, https://www.drupal.org/files/issues/2024-09-09/field_group_3_tabs.mov

I'll note that this was the exact same behavior I experienced on #2969051.

Thank you, @robpowell!

Could you create a follow-up issue regarding this problem, with the information you provided, targeting 4.x?

Done! https://www.drupal.org/project/field_group/issues/3473361

@robpowell, thanks!

Thanks for working on this. Ended up here coming from #2969051: Fix HTML5 validation prevents submission in tabs

I noticed that the MR target changed from 8.x-3.x to 4.x and as result, non of the changes here was committed to 8.x-3.x. Is the plan to only fix this in 4.x? If so, I am a little concerned, as 4.x is only in alpha1 and site policies will probably block them from upgrading to an early alpha, if alpha at all and I guess most field_group installations is on 8.x-3.x branch. Can you elaborate on what the plan is. Sorry if I missed this in any of the comments above.

@grevil You missed to give me credit. I helped to set up the MR and fix MR pipeline errors.

That's incorrect. See https://www.drupal.org/u/rcodina/issue-credits/948488.

@grevil You are right. I just only looked the git commit comment in #108 (I expected a comment like "Issue #2787179 by..."). Sorry!