Conditionally show default value fields

Assigned to me intentionally, don't want anyone to replicate work that has already been done. Once the preliminary work from Github is ported over this can switch to a regular unassigned issue.

Did a little bit of research on how we could implement this. For most fields, this is pretty straight forward. The outlier here would be any kind of field type that uses radio buttons, because you would ideally want one of the options be selected by default. If we check for a default value recursively, we can make get the "Set default value" checkbox be checked by default. I'm wondering if for these type of fields, it should be possible to remove the default value at all. 🤔

Getting a list of failing tests that I can work against.

The last submitted patch, 6: 3347296-6-wip.patch, failed testing. View results →

The last submitted patch, 8: 3347296-8.patch, failed testing. View results →

This should be 🟢

Verified this on Drupal10.1.x with a standard install.

Added a default value to the tags field of the Article content type
Applied patch
Edited tagged field again
Verified Set default is checked and default field is shown.

Added a new text field
Set default is unchecked and I don't see the default field.

@lauriii just showed me the lasted version and it's a clear UX improvement, so +1 to this changes.

On a throttled connection to ok 3G, there's a noticeable chunk of time where the default value checkbox is unchecked but the field is visible. Here's a youtube video of it happening. Is there a way to strategically use a bit of additoinal js and perhaps the no-js class to reduce the chances of this being disruptive?

Two other tiny things

1. +++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
   @@ -132,12 +135,74 @@ public function form(array $form, FormStateInterface $form_state) {
   +   * A function to check if element contains elements with #required.
   

   is it more accurate to say if it contains elements with #required set to TRUE? This will not return true on ['#required'] = FALSE (though arguably that existing is unlikely).

2. +++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
   @@ -132,12 +135,74 @@ public function form(array $form, FormStateInterface $form_state) {
   +      if (isset($element[$child]['#default_value']) && $element[$child]['#default_value']) {
   

   Could this wind up not returning true if $element[$child]['#default_value'] is set to have a default value of FALSE (which could be a valid default value).

Addressed #15.1

I have uploaded the patch.
Addressed point 15.2.
Please review.

The last submitted patch, 18: 3347296-18.patch, failed testing. View results →

The test failure looks unrelated, but #13 isn't addressed (either via code change or explanation) so this shouldn't be set back to Needs Review.
Lets base any additional changes on #9 Because

#16 Makes a change to a comment that gets it closer to accurate, but the string is still grammatically incorrect. Nothing lost there.

#17 instructs us to ignore this

#18 changes a conditional to reject default values that are FALSE. In #15.2 I point out the change is needed because FALSE could be a legitimate default value so the condition should be met.

This should address feedback from #15.

+++ b/core/modules/field_ui/tests/src/Unit/FieldConfigEditFormTest.php
@@ -0,0 +1,108 @@
+   * Provides test cases with required and optional elements.

Looks like this comment needs to be updated. Will do in next iteration, or could be fixed on commit too probably.

Retested #22 following same steps in #11

#15 appears to be addressed

#22 seems to add additional test coverage which is always good.

But what's the thoughts on #13?

Also moving to NW for #23 comment.

Researched #13; the main issue seems to be that loading JavaScript takes time when testing with the 3G throttle. This makes it hard to solve this because we have to also keep the non-js variation working, where we must always have the element visible. There might be ways to improve this in a heavily customized fashion but that likely wouldn't be sustainable. The heavily customized approach doesn't seem like a good approach especially given that this should be eventually improved by ✨ Use modals on the Manage Fields page Needs work .

Thanks for the explanation @laurii !

The explanation in #25 as to why #13 should not be within this issue scope makes sense. This was something I was interested in if feasible, but in this case I believe it's not worth adding maintenance-unfriendly and potentially fragile code to provide it. I wanted the option to be explored, and that was done successfully and the conclusion successfully justified not pursuing it.

Committed to 10.1.x. I don't plan to backport as it is a UI change.

Did not grant credit for patches with minor changes that were ultimately not useful as they were based on an incorrect understanding of what was required.

Automatically closed - issue fixed for 2 weeks with no activity.

It seems that this has accidentally broken node form comment settings for bundles that have comments configured as hidden. See 🐛 [regression] "Comments field is required" when creating content for types with a comment field configured as hidden Fixed .