Correct vertical tab does not focus on form validation

+1

Would have bet we had something for it already.

The patch is a little too specific. I would use something like

// get the errors on the page, return the form and the specific field that has an error.
const error = document.querySelectorAll(':invalid');
// get the vertical tab object this field is in
$(error[error.length -1]).closest('[data-vertical-tabs-panes]').data('verticalTab')
  .tabShow()

With the appropriate tests all around this to make sure the field is in a vertical tab.

Original patch didn't grab the tab (was failing on the .length check). Modified a bit and is now working as expected.

I've tried to test this in latest D8.9.20 and D9.3.5, but did not see any change. Any vertical tab that is not the default/focused that has required fields does not get any focus whatsoever. Things are made worse by the fact that the default/native browser errors do not trigger any validation error summary at the top of the page, so it appears as though as clicking the submit button does nothing (when in fact the native validation error is triggered "in the background", but not shown because the vtab that contains the field is not the one that's focused).

Syntax error in drupalci.yml

Fix + the test from #21 which should hopefully pass now.

Adding some tags to make this more popular

#22 can't be apply on Drupal 10

Even if i improve #22, it does not work for Drupal 10 (I am not sure to even understand what does #22 suppose to do).

If someone want to unlock validation, i improve #7 for Drupal 10 and add support for textarea.

But i don't think it's the right direction to take.
It seems to specific (and it won't work if form element is not input, select or textarea).

Anyway, enjoy the improvement

By the way, here's the improvements for #22 if it can help.

But it's weird.
Most of the code was already in the core and like es6 support is gone, the patch is just about moving a behavior (which does not work at all)

@arnaud-brugnon #22 provides intentionally failing tests that prove the problem exists and #24 gets those tests to pass, which proves the solution is effective. That should at least suggest the current solution is effective enough to not require a complete rewrite. Your patches completely remove the test coverage, which is not at all productive.

If someone want to unlock validation, i improve #7 for Drupal 10 and add support for textarea.

There's no need to target specific element types. There are selectors added via Drupal's Form API that can be targeted.

Patch #7 is a solution based on the assumption that Drupal uses HTML5 form validation. It would not work at all for server side validation as the logic is triggered on submit before any of that server side validation is run.

But it's weird.
Most of the code was already in the core and like es6 support is gone, the patch is just about moving a behavior

No.es6 files are no longer needed in Drupal 10 as the supported browsers all support modern syntax. Yes, this patch is about moving a behavior that successfully addressed an issue with CKEditor 5 - it's being moved so all Vertical tabs can benefit from the fix, not just those used in CKEditor 5 config, hence the @todo Remove when https://www.drupal.org/project/drupal/issues/2911932 lands. comment in the CKEditor5 JS.

(which does not work at all)

The test coverage provides objective proof that there was a problem and that patch #22 fixes it. Perhaps there are use cases that have not yet been accounted for, and details regarding how to reproduce those conditions would be helpful.

Okay.
Thanks for all the explanation.

#30 works like a charm

WCAG tagging.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Let me know if I'm testing this wrong

Installed Drupal 10.1.x with a standard profile
Installed field_group module
In the article content type setup a tabs group with 2 tabs A and B.
Moved a required field into the B tab and a non required field into the A tab
Created an article not filling in the required field in tab B
When I click save tab B is displayed with the required field in red showing as required.

I've deleted my comment as I figured out that my problem was problem with field_group tabs

original

Thank you for explanation and information #38. I didn't realize field_group was the problem but it seems to be. This issue fixed it for us. Even though it results in resolving "Please fill out this field" errors first then core errors afterward, it should work fine.

re #37 testing should never require installing a contrib module like field_group. Any issue fixed in core should be reproduceable with just core, otherwise its an issue that should be filed with the contrib module. In this case it is a confirmed problem.

To reproduce, you need to have a field inside a vertical tab that can fail validation. Usually, required and similarly validated fields are not inside tabsets in core

The easiest way to reproduce IMO is apply 2911932-30-test-only.patch from #30, which already reproduces the problem for the purposes of testing. Enable theform_test module (must have

$settings['extension_discovery_scan_tests'] = TRUE;

).

Then go to form-test/group-vertical-tabs and enter "bad" into the input in "Second group element", go to an different tab, then submit to trigger an error.
It not working === it does not switch to second tab, where the error is

Problem Happening
https://youtu.be/kYnLIIWCQes

Problem Fixed
https://youtu.be/Ydr06gybiFI

Re #37

on 9.5.7 patches not applying. Both vertical and horizontal tabs have this issue. We won't be able to use tabs at this point in time.

That makes sense as patches are built for the most recent dev branch, which is currently 10.1.x. This can probably be made into a 9.x patch pretty easily - if you have success with that perhaps upload it here for users who may also need it.

Thanks for the clarity @bnjmnm and the youtube links!

Did as you suggested and used the form_test module for testing.
Patch works great!

Committed 8f9e0bd and pushed to 10.1.x. Thanks!

Triggered 10.0.x tests for backport.

Cherry-picked to 10.0.x. Still need a separate patch for 9.5.x backport.

I have attached the re-rolled patch for 9.5.x

Reroll seems fine.

Seems random

Committed 5b7e07a and pushed to 9.5.x. Thanks!

I'm using a module that makes the log message field in the 'information' tab of drupal core required ( https://www.drupal.org/project/require_revision_log_message ) and ran into this issue. Was hoping that this patch would address the issue but it does not seem to. If anyone has the capacity to assist that would be appreciated