When a tab contains a field that the browser invalidates before submission, the tab containing that field should be focused so it's not a mystery as to why the form can't be submitted. The attached patch should check on form submit if inputs are valid, and if not set the focused tab.

Command icon Show commands

Start 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:

Comments

blake.thompson created an issue. See original summary.

dddbbb’s picture

Thanks for the patch, this is working nicely for me!

Drupal 8.5.1
PHP 7.1.15

blake.thompson’s picture

Version: 8.x-1.x-dev » 8.x-3.x-dev

Patch still seems to apply to 8.x-3.x and is still an issue.

dddbbb’s picture

Status: Needs review » Reviewed & tested by the community
guillaumeduveau’s picture

Priority: Minor » Normal
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new602 bytes

Hi,

This patch supposes you have a "details" group which we do not need with tabs. Basically to have tabs you need 1 field_group of "tabs" type and inside you need some field_groups of "tab" type.

This patch does not work at all on my configuration which is the one I just described.

I'm proposing another patch that should be more generic. Could you test it ?

kris77’s picture

Thanks @blake.thompson,
your patch seems to work.

I use field_group 8.x-3.x-dev with TABS vertical group and TAB item

Thanks.

partdigital’s picture

StatusFileSize
new612 bytes

This patch applied however the javascript wasn't firing with horizontal tabs with workbench moderation enabled. I've re-rolled the patch, it should work with and without workbench moderation now.

andrewmacpherson’s picture

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?

Note that more than one tab panel may contain an error, but only one tab panel may be in view at a time. #2848507: Indicate that grouping elements have child element errors for ux and a11y covers that scenario for core vertical tabs.

partdigital’s picture

StatusFileSize
new626 bytes

I ran into some js misfires when using Media with Entity browser. Updating the patch.

mahtab_alam’s picture

StatusFileSize
new962 bytes
partdigital’s picture

rreiss’s picture

#9 Works for me.
Thanks :)

rreiss’s picture

I have some issues with IE and Edge, the fieldset is being closed (collapsed) on submit and another click on the submit button expands the fieldset again.
Didn't look into it yet, but does it happens to all of you as well?

grimreaper’s picture

Hello,

Using patch from comment 9, it was not working for me on 8.x-3.0-beta1 because vertical tabs the selector is a little bit different.

vertical:
'.' + direction + '-tabs__pane'

horizontal:
'.' + direction + '-tabs-pane'

I haven't tested if it is the same markup on the dev version maybe it has evolved between the 8.x-3.0-beta1 and the last dev version.

grimreaper’s picture

It seems that the file has not been transfered...

kalyanik’s picture

chris matthews’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
Related issues: +#2787179: Ensure visibility of invalid fields (JS)

The 2 year old patch in #15 needs a reroll.

megha_kundar’s picture

Status: Needs work » Needs review
StatusFileSize
new870 bytes
chris matthews’s picture

Issue tags: -Needs reroll
narres’s picture

#18 is working fine!
Tested against field_group-8.x-3.1

kris77’s picture

#18 is working for me too without problem.

Apply patch in 8.x-3.1.

Thanks @Megha_kundar.

dddbbb’s picture

Patch in #18 is working for me.

opi’s picture

StatusFileSize
new870 bytes

#18 doesn't work when using Gin admin theme, because the submit button is not in a #edit-actions wrapper (#edit-actions--2 instead).

I rerolled the #18 patch with a more generic selector for the submit button. Works well for me then, when using horizontal tabs with Gin in a node form.

steinmb’s picture

Anyone tested this on Claro?

antoniya’s picture

Status: Needs review » Needs work

Tested patch in #23 with Drupal 9.3.3 & Field Group 3.2 and it worked for my case (vertical tabs in custom theme).

@steinmb this patch won't work when using Claro (or Gin as it is based on it) because it uses '__item' in its BEM classes instead of '__pane' – at least that is the case with vertical tabs. I wonder if it makes sense to use a less theme/direction specific selector like .field-group-tab – this will cover both vertical & horizontal tabs and also work with both Claro & Gin without introducing any regressions in Seven.

antoniya’s picture

Status: Needs work » Needs review
StatusFileSize
new659 bytes
new898 bytes

Here's a patch with my suggestion in #25. Interdiff attached.

Tested so far with vertical & horizontal tabs using Seven, Claro & Gin.

antoniya’s picture

Status: Needs review » Needs work

Since 'context' can contain lots of forms and there can be lots of input.form-submit, the code in the patch will execute way too often. I don't see any form ID in 'group_info' that would help with adding specificity to the selector. A different approach might be needed to move forward with this issue.

edyuenyw’s picture

Thanks @opi for #23. This works for me for the default 'Seven' theme.
Passed manual testing.

opi’s picture

Status: Needs work » Reviewed & tested by the community

Thanks @Antoniya for your more generic patch #26, works like a charm !

rcodina’s picture

Status: Reviewed & tested by the community » Needs work

The patch on #26 breaks tests.

kufliievskyi’s picture

@rcodina I have also noticed the issue in the tests/src/Functional/MigrateUiFieldGroupTest.php file while executing the tests.
The error I saw is this one:

1) Drupal\Tests\field_group_migrate\Functional\MigrateUiFieldGroupTest::testFieldGroupMigrate
Undefined array key "Drupal\sqlite\Driver\Database\sqlite"

/var/www/html/web/modules/contrib/field_group/contrib/field_group_migrate/tests/src/Functional/MigrateUiFieldGroupTest.php:135
/var/www/html/web/modules/contrib/field_group/contrib/field_group_migrate/tests/src/Functional/MigrateUiFieldGroupTest.php:157
/var/www/html/web/modules/contrib/field_group/contrib/field_group_migrate/tests/src/Functional/MigrateUiFieldGroupTest.php:59
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

But it seems to be not related to the fix and issue. In order to pass the tests I have just added

$class_name = substr(strrchr($driver, '\\'), 1);
$form = $drivers[$class_name]->getFormOptions($connection_options);

Instead of this code at line 135 in field_group/contrib/field_group_migrate/tests/src/Functional/MigrateUiFieldGroupTest.php
:

$form = $drivers[$driver]->getFormOptions($connection_options);

@rcodina Could you please specify the error you see in the tests? Or could probably someone else test it also?

The patch on #26 works well for me.

nadim hossain’s picture

StatusFileSize
new922 bytes

Re-rolled the patch 23 against the new release 8.x-3.6

kufliievskyi’s picture

Version: 8.x-3.x-dev » 8.x-3.6
StatusFileSize
new680 bytes

Re-rolled the patch 26 against the new release(3.6), branch 8.x-3.x.
Works on Drupal 10.3.0 with Gin admin theme.

rjg’s picture

#33 is working for me on Drupal core 10.3.2 and field_group 3.6.0 with horizontal tabs.

juandels3 made their first commit to this issue’s fork.

juandels3’s picture

Status: Needs work » Needs review

Patch #33 working for Drupal 10.3 with version 8.x-3.6 module. I have created the merge request and it is ready for review.

anybody’s picture

Version: 8.x-3.6 » 4.x-dev

Please use MR's!

anybody’s picture

Status: Needs review » Postponed (maintainer needs more info)

Please check if this issue still exists in 4.x after #2787179: Ensure visibility of invalid fields (JS) has been merged now.

juandels3’s picture

Hi.
I confirm that this bug is not reproduced in version 4.x-dev, so it is already fixed from this version onwards. Thanks!

anybody’s picture

Status: Postponed (maintainer needs more info) » Closed (duplicate)

Thanks @juandels3 so let's close this closed as duplicate! :)

clemorphy’s picture

I use the patch from the MR !71 on the version 3.6, but I found an issue with the submit button being targeted with not enough specificity.
I have an entity browser button on one of my tabs, and as soon as I click it, it goes back to my first tab.
The selector in the patch is not specific enough :
$(context).find('input.form-submit')

The entity browser also has the form-submit class, and a click on this button triggers the check for invalid fields.

This selector could be more suitable:
$(context).find('input.form-submit.button--primary')

Or :
$(context).find('input.form-submit').not('.entity-browser-processed')

I'm not sure if this applies to the 4.x-dev version, as it seems to be treated in a different way.