- Issue created by @thomwilhelm
- 🇲🇾Malaysia jonloh
Just tested this and confirmed that 4.0.0-alpha2 has this issue. Downgraded to 4.0.0-alpha1 and it was fine.
- 🇲🇾Malaysia jonloh
It seems that the commit done in https://git.drupalcode.org/project/conditional_fields/-/commit/e1507398b... technically broke this functionality.
Remove that line of code and it works as it should for required fields that are not visible.
- 🇳🇱Netherlands zebda
I have the same problem. Is there a fix for this without downgrading?
- First commit to issue fork.
- @aneida opened merge request.
- 🇻🇳Vietnam Khoa Pham
conditional_fields/src/ConditionalFieldsFormHelper.php
Comment 1 line: // return;if (!empty($field_errors)) { $form_state_addition['errors'] = $field_errors; // return; }
- First commit to issue fork.
Hello, I would like to kindly confirm that the current version of Conditional Fields appears to attempt to validate hidden fields. Would it be possible to merge this update as soon as possible? Thank you.
- Status changed to Needs review
over 1 year ago 1:57pm 5 May 2023 - last update
over 1 year ago 130 pass - 🇮🇳India arisen Goa
Reviewed the patch and MR. Tested it on drupal 10 site.
The issue resolves as expected.But not sure if it's the right approach.
Is it required to make the text field required by default? We can keep the field as optional in the field and make it required in the dependency settings based on the selected value. Attached the screenshot for the same. - 🇮🇳India saurabh-2k17
I noticed that the change in ConditionalFieldsFormHelper.php is not affecting when we are using a reference field in a content type and that reference field has a hidden field which is required.
Example, for Basic content type, I have a reference taxonomy field. That taxonomy field has a bool field which displays a required text field. Even after making the above change, that hidden text field is still being required while adding a content.
- Status changed to Needs work
over 1 year ago 12:07am 19 May 2023 - 🇦🇺Australia thomwilhelm Sydney
OK so I've tested the patch in comment #15 and it fixes the issue on our site. However, I'm marking this as Needs work for a couple of reasons:
- I don't think the correct fix is commenting out a return statement. If it's not needed, it should be removed.
- So we have chance to respond to comments in #16 and #17.
In regards to #16, the previous module behaviour was that if a required field was hidden, then it wouldn't be required. I know you could leave it optional in the settings and then make required based on a condition, but I think for the scope of this issue we just need the previous behaviour back.
With regards to #17, is this something that was working in alpha1 and broke during the alpha2 release?
- 🇺🇸United States maddentim
Just so all understand the history, this return statement got introduced from this issue: https://www.drupal.org/project/conditional_fields/issues/3207751 🐛 URL validation of link field doesn't work Fixed
I am going to add this patch from #15 in my project for now so we can use the alpha3 relese, but long term I think the project probably needs to find a solution that does not just break the link validation issue again from 3207751.
- Assigned to jigish.addweb
- First commit to issue fork.
- last update
over 1 year ago 130 pass - @bojan_dev opened merge request.
- Status changed to Needs review
over 1 year ago 9:52am 31 August 2023 - last update
over 1 year ago 130 pass - 🇳🇱Netherlands bojan_dev
I have a different use case, but I do believe it's the same issue.
Steps to reproduce:
1. Target field: a date field (not required)
2. Controlled by: Text field (required if target field is not empty)
3. On saving a node while target field is empty, I was still getting a required validation error.After some debugging I found out that the related condition was using the
isset()
function which does not cover the case when the array keys do exist but the value is NULL.Fix available in: MR 29
- 🇮🇹Italy grimal
Same problem here,
after some debug i've fixed with the same solution of the patch #15, there is a wrong return statement pushed here ( https://www.drupal.org/project/conditional_fields/issues/3207751 🐛 URL validation of link field doesn't work Fixed ) to resolve an url validation issue but isn't a solution, just a bypass that introduce a lot of problems :) - 🇺🇸United States loze Los Angeles
This may be related:
When an inline entity form has a required field, and the parent form is hiding the entire ief form. I am getting errors that required fields are empty.
I would expect that if the ief field is hidden by conditional_fields it would not attempt to create an entity from the hidden ief field.
- last update
about 1 year ago 130 pass - 🇮🇹Italy grimal
I have a form that includes a required file field, which is only visible when a specific value is selected in a dropdown. Unexpectedly, this file field still triggers a validation error, even when it's hidden (already patched the module with #15) .
After some debugging, I discovered that, for some unknown reason, the same validation error is present in both the file field key and the submit key within the 'form_state' errors array (they share the same error object reference).
So if the same error is present in multiple fields is not removed.
I've resolved the issue by replacing the current 'array_diff_assoc()' comparison with an object reference comparison.
- Status changed to Needs work
about 1 year ago 8:12pm 17 October 2023 - 🇸🇰Slovakia coaston
In my case I have field marked as required.
I have added condtional field to make it hidden if taxonomy status is "hide".1. I have applied path #29 but it seems this solve other issue (remove duplicated error) and there are missing previous patches. So it is ok to solve additional issue but also previous path should be included.
2. I have applied MR29 (#25) but this one also did not fixed my case.
3. In my the problem was resolved with #15 as suggested in comment #27.
So it seems here are multiple issues and for all of them there are patches but someone needs to put them toggether and create one final patch or MR which can be pushed to prod.
But definitelly patch #15 should be included as well.
- 🇨🇦Canada robbdavis
#15 alone worked for me... for now.. we'll see if we still have issues and need #29
- 🇳🇬Nigeria chike Nigeria
None of the patches here is working for me in Drupal 10.2.0.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
First of all: Thanks for all the hard work in here! +1
But not sure if it's the right approach.
Exactly. #17/#18 indicate that it needs more discussion for a final task/patch on this. Apart from that please apply patches/merges always against latest dev and not against releases.
Final thoughts: from my understanding of this issue using/setting-up required fields which get hidden by CF and then should be ignored on save sounds a little bit like contradicting site building attempts and should not be considered to work out of the box. There is a certain contradiction in this setup. But I understand that users this wants this to be possible. But I consider this not a bug, but a feature request then.
- Issue was unassigned.
- 🇦🇺Australia thomwilhelm Sydney
Hey @dqd thanks for looking at this ticket!
using or setting-up a required field which gets hidden by CF and should be ignored then on save, sounds a little bit like contradicting site building attempts and should not be considered to work out of the box
This is not true at all and from my understanding is a main use case for this module, hence why people have noticed it's broken their sites. In fact this functionality you are describing is exactly what is shown on the modules project image. The field Operating System triggers a conditional field Other which is only required if it shown, otherwise it is not required.
https://www.drupal.org/files/images/conditional_fields_image.png →
I'd actually go one step further and say once fixed we should ensure this feature has tests written to make sure this functionality doesn't get broken.
- Status changed to Postponed: needs info
10 months ago 1:32am 4 March 2024 - 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
No it's not ;-) read my comment twice. And it is not what your image describes.
What is expected to happen
The field that is not visible is not required as it's not visible on the form. So I can submit the form (assuming all other fields have been completed).There are wrong assumptions in this description from the issue summary. If you want stop being a field required it is not enough to hide it, you also need to disable the required state ;-)
- 🇦🇺Australia thomwilhelm Sydney
Yes I'm aware you can still somehow achieve this result if you add two rules, one for the visibility, and one for the required state of the field. But in what case would a field be hidden, but still required?
The issue summary accurately describes how the module used to work, the release when this functionality changed, and steps to reproduce.
In fact in other issues there are commits from maintainers that confirm this is the way the module is intended to work. ie. once a field is not visible, it should not be required:
https://www.drupal.org/project/conditional_fields/issues/2983381 →
And in the 4.x version of js/conditional_fields.js you can clearly see when a field isn't visible, it should not be required.
https://git.drupalcode.org/project/conditional_fields/-/blob/4.x/js/cond...
So for me the issue summary is totally accurate. This is a clear bug not a feature request.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Thanks for taking the time to come back and elaborate on your thoughts. Very much appreciated and please consider following thoughts not as offense.
#38 But in what case would a field be hidden, but still required?
In the case, when a field needs to be filled but hidden and maybe made visible again on later state while for the overall setup it is mandatory that the field is filled, for example.
In fact in other issues there are commits from maintainers that confirm this is the way the module is intended to work. ie. once a field is not visible, it should not be required
And you read the maintainers and contributors comments wrong again. Especially the previous last comment in this issue, which follows some of my worries.
So for me the issue summary is totally accurate. This is a clear bug not a feature request.
Next example of reading comments the wrong way. I didn't say this issue is inaccurate, otherwise I would have closed it. And a bug is something crashing the site or throwing errors etc. To make something possible which wasn't possible before in this project is a Feature request. And Postponed only indicates that more thoughts need to shared before wasting the time and efforts of upcoming patch contributors who are willing to work on it. And as you can (maybe) see I am working hard on this project over my whole weekend now (over 200 issues!) and even committed patches for Drupal 7 versions while Drupal / EOL is around. But I am fine if you have ab idea how to solve this I woudl love to review a aptch of you on this.
Yes I'm aware you can still somehow achieve this result if you add two rules
OK, then my question again but with other words: why patching a workflow which is hardly opinionated and has already a workaround? And I didn't mentioned yet that we will have tons of issues afterwards when people complain that if they set up a field as required in the content type that they DO NOT want another contrib module override this state set up in core. This is a hard injection.
Again, postponed is just indicating maintainers will to further discuss this issue and if you are convinced this should happen in this way it is proposed here already, then please provide a working patch or MR which others can review. Otherwise I would kindly ask you to wait for other opinions on this here to not get too much offtopic in here. Thanks for understanding.
- 🇨🇦Canada star-szr
I was just debugging some of this code with a colleague and I have to say it seems the existing code seems to support the use case.
I don't want to stoke the flames here, but it just doesn't seem right to call this a feature request when there is so much code that appears to now be dead code (admittedly I have not tested every use case) thanks to the hastily-added
return
statement in 🐛 URL validation of link field doesn't work Fixedhttps://git.drupalcode.org/project/conditional_fields/-/blob/f988abcf081...
Of course
dependentValidate()
as well. - 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
I don't want to stoke the flames here
You don't. ... Don't you? - Don't worry. I asked for thoughts and I appreciate all of them. Thanks for taking the time! But ...
but it just doesn't seem right to call this a feature request when there is so much code that appears to now be dead code.
It would help the issue to point the code more precise. And why this code is dead in your eyes when/while we work on it? This issue is open for progress. Does this code has been there for several reasons or indicates the code that it has been added explicitly for this here to work already?
To call something a feature request does not stop anything(?) and does not depend on how much code partly is already there to make it happen. It only distinguishes from an existing and documented feature which is broken (bug) or a feature which would be nice to have in and (in this case maybe) has already most of the code it needs in there. Which would be great news.
(See also https://xkcd.com/1172, I don’t think this is that situation but I do think that breaking changes come in many forms.)
I don’t think this is that situation
I know this picture. And exactly: No it is absolutely not that situation, this is why I don't understand the mentioning. Seriously, I have no idea what this has to do with this issue where I - a maintainer of this project who is close before burn-out from the last weekend sprint with no sleep to help here to get the next release ready [I do not even use this module] - where I just try to move on productively. On the end it doesn't matter for the users how it is called. For the users it only matters that we get it done.
But we really shouldn't waste scroll band here for this back and forth. FR tagging helps maintainers to build release notes on new features and changes which are important to be aware of. Again, the wanted behavior here is not what everyone expects (explained above).
Let's start working on it!
- Status changed to Needs work
10 months ago 11:25pm 4 March 2024 - 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
So, finally, my questions which lead to postponed are not answered yet. So I try to answer my questions by myself to move on.
We have to take 2 different user expectations into account:
- Users who want a hidden field to give up its other states from field setup (like required).
- Users who want that a field no matter what it makes elsewhere never gives up its primary made setup (like being required)
Since I come from team and CRM software I know that setting up a field as required can be really mandatory for certain setups between content types and fields and can break the whole system badly if this will be turned off in other forms suddenly. Especially if not documented as a new feature ;-) (side kick)
My proposed solution would be to provide 2 hide options then in CF:
- hide only
- hide and disable being required.
This would be the most useful solution in my eyes to move on.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Apart from that I cleaned up and structured the issue summary a bit, don't know what all this hyphens with line breaks are all about.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Review on #29
- $errors = array_diff_assoc((array) $form_state->getErrors(), $untriggered_dependents_errors); + $errors = $form_state->getErrors(); + foreach($errors as $field_path => $error) { + foreach($untriggered_dependents_errors as $untriggered_dependents_error) { + if($untriggered_dependents_error === $error) { + unset($errors[$field_path]); + } + } + }
Not sure if this is the best approach to suppress errors for one of the both scenarios. It keeps the field as required in other situations than here under this conditions, which is good, but it circumvents the required state without warning. Maybe a good starting point for the second: "hide and ignore if being required.".
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
To prevent further polarization (we all agree that this needs to be fixed), I decided to turn it temporary into a task, since there are many many things to do. I will go back in the modules history and test extensively and come back to maybe turn it back into a bug then. But until then:
- This feature (and it doesn't matter if it existed all time before or just in between) needs documentation because of its huge intervention in field setup.
- It needs a second option so that we have "hidden only" and "hidden (disabling states like required)" to make users aware of there choice and consequences.
- As Thom already stated correctly it needs tests because this should not break in future.
And I will mark this for a release blocker since it is important to have a clear route on this for future releases.
Just a few last words regarding the previous discussion: I just want to make clear again that I only want to prevent a big mistake because the wanted behavior is a major intervention and can break sites depending on the required state very badly where users expect different behavior. A field setup as required should not be taken lightly. That's why my worries in here before. And I want to make clear again: that I very much appreciate all the thoughts, efforts, opinions, and token time to elaborate and to discuss this. Because I know very well how time and power it consumes. Imagine this from my standpoint on 100s of issues... And I am the last who do not empathize with the wishes of users. The opposite. I worked over 200 issues last weekend thru here to make this project final Drupal 10/11 ready and I do not even use this module nor do I get payed for my work. So you can be sure, that my intentions are of a genuine nature and if this used to work this way and was intended to work this way, then I am 100% fine with it. I just had to throw it against the wall to prevent light decisions. And the result is: we still cannot treat this issue as a usual bug fix, because (as I sad above) we need a second setup option to not break sites which are awaiting another behavior here.
I hope this comment makes it clearer...
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Edited issue summary accordingly...
- 🇨🇦Canada star-szr
I think the direction makes sense. I agree that the way it used to work might be called “overbearing” or similar. Thanks for all your work @dqd!
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Very much appreciated and motivating, thank you @star-szr. And: you're welcome.
- 🇳🇬Nigeria chike Nigeria
Thanks @dqd for all your work. When I tried the patches and they didn't work I reached out to a friend and he did this on the site: he made 3 conditions to solve the issue. Let me explain.
The control field is 'field_type_of_event'. If the value is 'Physical' then the target field 'field_venue' should be visible and if the value is 'Virtual' then the target field should be hidden. I had another condition to show another field 'field_online_event_platform' when 'field_type_of_event' has value 'Virtual'.
Initially I set one condition for 'visible' then I noticed that when it is hidden it was still required (the field was required) so the form won't submit. That was where the frustration started.
But I found out one other thing, if control field is set to 'Physical' and the target field showed up and I entered a value in 'field_venue' let's say 'Lagos' and then I switched the control field to 'Virtual' and 'field_online_event_platform' becomes visible and I entered a value 'Google Meet' and saved the form; then both 'Lagos' and 'Google Meet' are displayed on the page. So not only did I need to make the hidden field not-required, I also needed to make it empty cos user could have populated it with values before changing his mind to hide it.
So my friend made these three conditions on the site:
First he made 'field_venue' not required on the site and then he made the conditions.
1) If 'field_type_of_event' has value 'Physical' make 'field_venue' visible.
2) If 'field_type_of_event' has value 'Physical' make 'field_venue' required.
3) If 'field_type_of_event' has value 'Virtual' make 'field_venue' empty.
This is working perfectly.
I am not sure if this will help in the decision making for the way forward.
- 🇧🇷Brazil joaopauloc.dev
Got the same issue.
The changes works but I couldn't apply.
Attaching the new patch that I could apply to 4.0.0-alpha5 module version.
Thanks. - Status changed to Postponed: needs info
9 months ago 10:13pm 26 March 2024 - 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Got the same issue.
What do you refer to? The original report? The last comment?
The changes works but I couldn't apply.
Attaching the new patch that I could apply to 4.0.0-alpha5 module version.
Thanks.Sorry I have no clue what you are trying to say ... Couldn't apply? How can it "work" if you couldn't apply?
- 🇧🇷Brazil joaopauloc.dev
Hello @dqd, my sincere apologies for my last confusing and incomplete comment.
Let me explain better.I have a website using this module on the 4.0.0-alpha1 version.
Then I tried to upgrade to the next version 4.0.0-alpha2.
After this upgrade one of my content types presented the wrong behavior mentioned in this issue.I have field A which is a radio box with option 1 and option 2.
Also, I have field B that should be required and visible if option 1 is checked on field A
Then, field C is required and visible if field A has option 2 checked.The expected behavior stopped working after the upgrade from 4.0.0-alpha1 to 4.0.0-alpha2. So, I tried to update to the latest version 4.0.0-alpha5 but the issue persisted.
After my research, I found this issue. In my last comment when I said I couldn't apply, I meant that I couldn't apply the changes of the merge request 3344587-required-fields-that as patch in the 4.0.0-alpha5 version. I saw other code changes that made the patch fail. So I applied the same change to the 4.0.0-alpha5 version and the behavior to make the field required based on the radio box worked.
Hope I explained better this time. Sorry for the confusion.
Thanks. - Status changed to Needs review
9 months ago 9:36am 27 March 2024 - last update
9 months ago 130 pass - 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Thanks for coming back clarifying. Very much appreciated. +1
In short: you actually did a manual re-roll of the merge/patch to make it work in 4.0.0-alpha5, which In fact (the merge/patch) is just an added
return
. Glad that you was able to fix it for your scenario and thanks for taking the time to contribute.Still we need to make sure that your case is really part of this issue here. We still need testers for latest dev version to go on.
In your scenario for example the question is:
I have field A which is a radio box with option 1 and option 2.
Also, I have field B that should be required and visible if option 1 is checked on field AIs field B still required when it gets hidden by option 2?
Then, field C is required and visible if field A has option 2 checked.
Is field C still required when it gets hidden by turning back to option 1?
Both question contribute to the issue here but are not clear for me yet. In both cases you wouldn't be able to save if the required fields are empty (not filled).
- last update
9 months ago 120 pass, 2 fail - last update
9 months ago 131 pass - 🇧🇷Brazil joaopauloc.dev
Hello @dqd.
Exactly, the fields are still required after the submission. On the client side, the required status for both fields is updated and I'm able to submit the form.
But, I got an error message saying that field B is required but shouldn't be as I checked option 1.I added two patches, one with the test unit with the scenario that I mentioned and the test unit will not pass, and another patch with the fix and you can see the test unit passing.
The last submitted patch, 59: 3344587-required-fields-that-59.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.- 🇸🇰Slovakia coaston
Hi joaopauloc.dev,
Your patch 3344587-required-fields-that-59-with-fix.patch worked for me.
- 🇬🇧United Kingdom lexsoft London
@ joaopauloc.dev,
The patch does not work when used with inline_entity_form. If required field is hidden in inline_entity_form simple by conditional fields it would show up in form validation errors. Any advice on how to get it to work?
Thanks in advance.
- 🇬🇧United Kingdom lexsoft London
Steps to replicate:
On a standard Drupal 10.1.8 installation
Add a Content Reference Field to the Article Bundle:
In the Article bundle, add a new content reference field that points to the Basic Page bundle.
Set the 'Manage Form Display' for this field to 'Inline Entity Form - Simple'.Update the Basic Page Bundle:
Add a boolean field named 'field_boolean_test' to the Basic Page.
Ensure the body field on the Basic Page is set to be required.Configure Conditional Fields on the Basic Page:
Implement a conditional field setting where the 'body' field's visibility is controlled by 'field_boolean_test'.
Set the condition to make the body field visible only when 'field_boolean_test' is checked.Expected Behavior on the Article Creation Page:
You will see an inline entity form for a Basic Page embedded within the Article form. This form will include the checkbox ('field_boolean_test') and the required 'body' field.
When the checkbox is toggled, the 'body' field should hide or display based on the checkbox status.Encountered Issue:
If the checkbox is checked (hiding the body field), attempting to save the Article triggers a validation error due to the required status of the 'body' field. This only happens via inline entity form, if the same process is replicated on the Basic Page add it functions as it should.
- 🇺🇸United States jeffschuler Boulder, Colorado
Re-roll of #59 (with fix) for latest 4.x-dev.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Thanks @jeffschuler for keeping track on this (and the typo correction hah :) Let's move to MR's to speed up work in here.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Issue summary has #anchor points now to refer to.
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Does thoughts point 2 and tasks point 2 have been addressed in the latest patches?
- 🇫🇷France tostinni
We're facing the same problem and none of those patches seems to fix it (#59, #64 & MR26).
For our case we're using the Paragraphs stable form widget and we do have nested paragraphs.We tried setting the target field required => when it's displayed it's not required even if we add the required state next to the visible.
If we don't set it required in its config but setting the form state as required, it's still not required when visible.
Anyone has been able to make this work with nested paragraphs ?
We tried on alpha5 with patch from 🐛 Controlled-by fields inside a Paragraph don't work Needs work and also on latest dev. - 🇺🇸United States jeffschuler Boulder, Colorado
@tostinni: The patch in #64 on the latest 4.x-dev (and no other patches) is generally working for us inside nested paragraphs.
For example, we have a multi-value paragraph reference field that can reference a paragraph that has another multi-value paragraph reference field within. Inside those nested paragraphs there's a select field that has conditional fields visibility control over a few other fields; for various values of the select, other fields within the same nested paragraph are shown & hidden. Some of those fields are required (from within the paragraph type config). When those required fields are hidden (and empty), they do not stop the form from saving. When they are shown, they are required, as designed.
The exception to this is a required Media field within that nested paragraph. It does not interrupt the form from saving when it's hidden (good) but it also does not enforce its requirement when shown (bad). I brought this up in another issue #2902164 comment #140 🐛 Controlled-by fields inside a Paragraph don't work Needs work . I don't want to clutter this issue... only in case that's the problem you're experiencing.
Maybe you can describe your content structure and exactly what's not working?
- 🇫🇷France tostinni
@jeffschuler yes we have the same configuration and unfortunately it prevents saving the form.
I have restested it in an empty instance of D10.3.8 with the patch #64 and this set up :
- Article CT with reference to paragraphs:
field_ref_widgets
using "Paragraphs (stable)" form widget (but same error with - Creating a "Resources" paragraph with
field_ref_resource_items
pointing to a "Resource item" paragraph - Creating a "Resource item" paragraph with 3 fields
field_media_file
: an entity reference for media referencing the document Media typefield_link
: a link fieldfield_resource_type
: a select controlling which previous field will be displayed
- Assigning dependencies cf screenshot (I only put the visible part)
- Create a test content and seeing the error of the hidden widget required field
Is there anything else I can share or check to find out why this is happening ?
Thanks
- Article CT with reference to paragraphs:
- 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
Please let's focus on fixing this issue and let us move personal support into a seperate support request. Apart from that please help to answer the questions in #69 mandatory to move on. Thanks for understanding.