- last update
about 2 years ago 17 pass - 🇧🇪Belgium DuneBL
This patch is nice and working well except if we are adding fields in an empty group in the hook
form_alter
.
I am using the following function to add $form[$field_name] to a group (id=$group_name):function addToGroup(array &$form, string $field_name, string $group_name) { /** @var EntityFormDisplay $form_display */ $form_display = $form['#process'][1][0]; $form[$field_name]['#group'] = $group_name; $field_group = $form_display->getThirdPartySettings('field_group')[$group_name]; $field_group['children'][] = $field_name; $form_display->setThirdPartySetting('field_group', $group_name, $field_group); } }
If I add $form['field_to_add'] in a group which is empty but which have the option "
Display element also when empty
" checked; then the group is not displayed.To summarize, there are 2 issues:
1-The patch doesn't take into account the optionDisplay element also when empty
2-The patch doesn't take into account any element added to the group in theform_alter
hook - 🇫🇷France prudloff Lille
The patch works correcly on initial load, but it does not seems to work correctly when detecting a change.
We have a fieldset containing 3 fields that can be shown/hidden depending on when another checkbox field is checked (we use conditional_fields for this).
The field group is initially hidden but if I make the fields visible then hide them, the field group is not hidden and is displayed empty.This seems to happen because when
Drupal.FieldGroup.hideGroupIfEmpty()
evaluate thedisplay
property of the fields, the last does not havenone
yet. So it might be some kind of race condition that makes it check the field too soon.
If I do something like this, then it works correctly:diff --git a/formatters/fieldset/fieldset.js b/formatters/fieldset/fieldset.js index 7349be4..3a2f052 100644 --- a/formatters/fieldset/fieldset.js +++ b/formatters/fieldset/fieldset.js @@ -34,7 +34,9 @@ if ($(e.target).hasClass('field-group-child-field')) { let $group = $(e.target).closest('.field-group-fieldset'); if ($group.length !== 0) { - Drupal.FieldGroup.hideGroupIfEmpty($group, null, true); + setTimeout(function () { + Drupal.FieldGroup.hideGroupIfEmpty($group, null, true); + }, 500); } } });
(This is obviously not the way to the fix the problem, but it helps demonstrate it.)
- Status changed to Needs work
8 months ago 9:43am 21 November 2024 - 🇧🇷Brazil karen-kramek
Hello everyone,
Re-holl of patch field_group-2978747-40.patch (8.x-3.0), now covering the module version 8.x-3.6. - 🇪🇸Spain pakmanlh Barcelona
Unfortunately I can confirm the behavior described by #42 working only initially.
- 🇩🇪Germany tgauges
Would
MutationObserver
be an acceptable solution for #42? One would need to observesubtree
,childList
, andattributes
.I will create a new branch in the issue fork based on
2978747-hide-group-if
. - 🇩🇪Germany tgauges
You can take a look at https://git.drupalcode.org/issue/field_group-2978747/-/tree/2978747-hide-group-with-mutation-observer for my solution for #42.
There don't seem to be any tests implemented for this issue?
- 🇨🇦Canada tame4tex
tame4tex → changed the visibility of the branch 8.x-3.x to hidden.
- 🇨🇦Canada tame4tex
tame4tex → changed the visibility of the branch 4.x to hidden.
- Merge request !116Issue #2978747: Hide field groups when all children hidden via js states → (Open) created by tame4tex
- 🇨🇦Canada tame4tex
Not only is there an issue with the code only working initially, I also experienced issues with it not working with nested groups. To fix these issues I implemented a different approach.
We need to ensure that when the
state:visible
event is fired we not only set the visibility of the closest field group but we continue up the DOM to set the visibility of parent field groups. This is easier to handle with onestate:visible
event listener in thefield_group.js
than one for each field group type.Because each field group type might need to do different things when its children are all hidden vs when a child is visible, once we have determined the current visibility of the field group children we need to hand off what is done to the field group to the js file for the different types.
I have updated the Issue Summary with my proposed resolution. Given the significant changes to the previous version and the merge conflict issues in updating the old solution branches, I have added a new 2978747-hide-group-4.x and opened a MR.
I have also added preliminary testing for this bug.
@tgauges: Interesting concept with the MutationObserver, although I am wondering if it is more than what is needed in this instance? Given the new approach, what does it do better?
- 🇪🇸Spain pakmanlh Barcelona
I can confirm the last / new approach from 2978747-hide-group-4.x works fine. thank you!
- 🇩🇪Germany tgauges
@tame4tex MutationObserver is a catch-all solution which also can work if the DOM is mutated by something other than the Drupal Form API
#states
property.In my opinion you are correct: It is probably more than what is needed. I think it's enough to react to changes caused by the
#states
property. - 🇨🇦Canada tame4tex
I have pushed a couple of fixes to the MR due to issues encountered.
`Display element also when empty` setting was not being respected. I have now added a test for this and fixed the bug by adding a css class
if this setting is enabled and then checking for that class in the logic.I also encountered a weird bug in Safari 18.x where it was ignoring the style attribute on the element being hidden/shown by states and instead determining the display value to be "none" when it was actually "block". I have adjusted the code to specifically check the style attribute on the element first and only get the computed style if the style attribute display value is empty.
I have switched the status back to Needs Review due to these changes.