State required asterisk doesn't show

Hey @vqgr,

You need to make changes in states.es6.js as Drupal is now using ES6 for javascript development. Please refer https://www.drupal.org/node/2815083

Applied patch from #6. Patch applied without any error but the change is not working.When using state required, the asterisk is not displayed.
Screenshot attached.

The patch in #6 wouldn't apply since the line numbers are off - the states.es6.js file must have been updated since that patch was created. I've updated the patch with the current code. Thanks @vqgr for the original code.

Just found a bug in the code that was preventing the required classes from being removed when an element should have gone from required to optional. I'm attaching an updated patch.

tagging for badcamp 2018

The patch seems to resolve the issue of showing/hiding the asterisk when a form field's #states change.
In my case, I have a form_alter that handles #states changes of Paragraph form field subform elements when another field value is changed.

$element['subform']['field_text']['#states'] = [
  'required' => [
    ':input[name="field_video[0][subform][field_variation]"]' => ['value' => 'curate'],
  ],
];

In this example, in the field_video field, which is a an entity reference revisions/paragraph field on the node, when the field_variation field within it changes to Curated, the field_text is set to required, and when the value is Topic, the field_text is set to not required.

This is working properly, due to this patch, but it turns out there is another issue where now the node form is not submitting and is giving this Javascript error:

An invalid form control with name='field_video[0][subform][field_text][0][target_id]' is not focusable.

I assume it's some incompatibility with paragraphs fields like in the way I'm trying to do it.

Time to change the version to 8.7?

Rerolled patch in #13 for D8.6.13

Tagging for DrupalNorth 2019

The last patch won't work for me. When debugged, the problem is with:

var child = e.target.querySelector('label, legend');

e.target is input (or any other form element), so finding label/legend inside it fails. For me this improvement works:

var child = e.target.parentNode.querySelector('label, legend');

Attaching patch.

This issue solves the problem partially. There is still problem with '#type' => 'datetime' items. See issue https://www.drupal.org/project/drupal/issues/2419131. It covers all - asterix works for every #type.

Tagging for DrupalCamp Colorado 2019 (Sunday August 4)

Thank you! It works for me.

We should remove HTML attribute 'required="required"' from the too.

Added relevant tag

Reroll patch for 9.1 with #25 also implemented.

Using the bootstrap theme which adds a wrapper around the select element this code fails to add the mark on the label.

const child = e.target.parentNode.querySelector('label, legend');
const variable = child ? child.tagName.toLowerCase() : '';

I think it's more flexible to do it like this :

const $child = $(e.target).closest('.js-form-item').find('label, legend');
const variable = $child.length ? $child.first().prop('tagName').toLowerCase() : '';

Added a patch which covered the point by #30.

Tested #31, changes are visible
This can be moved to RTBC

Tested #31 but seems the patch is splitting up the old variable leaving some non-functional JS.

Created a patch from the one in 31 which also removes the 2 lines

'aria-required': 'true'
}).closest('.js-form-item, .js-form-wrapper').find(label);

This fixes console errors like Uncaught SyntaxError: Unexpected token ':'

@KevinVb please review as this is "Custom Commands Failed".

Thanks!

@djsagar
Patch was created on 9.1.3 and worked. It was first patched with #31 which broke ckeditor and entity browser when JS aggregation is applied.
If someone could test this in 9.2 it would be really appreciated but for now I can only say it works for me in 9.1.3

Update to Drupal 8.9 patch in #21. Some nested elements elements in webforms were not finding the .js-form-item wrapper with parentNode.

@KevinVb
It seems that states.js does not reflect changes in states.es6.js in your patch.

states.es6.js lines 690 and 691

const $child = $(e.target).closest('.js-form-item').find('label, legend');
const variable = $child.length ? $child.first().prop('tagName').toLowerCase() : '';

states.js lines 342 and 343

var child = e.target.parentNode.querySelector('label, legend');
var variable = child ? child.tagName.toLowerCase() : '';

Working on it

Adding patch as previous patch failed to apply.

Adding updated patch with an interdiff.

Thanks @yogeshmpawar for your work. The patch #42 seems good but in the patch #44 you changed :

.find(variable + '.js-form-required')
to
.find('variable.js-form-required')

which seems wrong to me. Is it intended ?

I just noticed that you're not using jQuery .closest() but Element.closest() method which is not available in IE.

Since Drupal 9 need to support IE I would recommend to use jQuery here.

Got some time to work on it. I used jQuery .closest() instead of Element.closest() to get it working on IE. And reverted back
.find('variable.js-form-required')
to
.find(variable + '.js-form-required')

There are still issues with the latest patches:

1. Multi-value fields do not display an asterisk when required via states.
2. Date fields have issues applying the asterisks properly.

I decided to attempt a slightly new approach in the states.js code which I have provided a patch for.

Basically the changes to states.js from the previous patch(es) are:

1. I have simplified the discovery of the label to apply the asterisk to into a single (chained) lookup instead of having it divided it into multiple parts ($child / variable / $variable).
2. The search for the form wrapper now only includes "visible" labels by ensuring the element does not contain the .form-no-label class.
3. The querySelector() part is now broken up to easily identify the various types of label elements known to core.
4. Fixed searching for legend to instead be legend > span in order to prevent certain issues where double asterisks were occurring.
5. Added support for multi-value labels.
6. Added support for date / datetime labels.

const $target = $(e.target);
const $label = $(e.target
  .closest('.js-form-item:not(.form-no-label), .js-form-wrapper:not(.form-no-label)')
  .querySelector(
    // Standard field labels.
    'label'
    // Labels within a fieldset.
    + ', legend > span'
    // Labels for multi-value fields.
    + ', th.field-label > .label'
    // Labels for date / datetime fields.
    + ', h4.label'
  )
);

This will result in the asterisk being placed on the closest "visible" label and not just the closest label it has found.

This helps for multi-value fields as the element/widget labels are often intentionally hidden (by WidgetBase::formMultipleElements()) so that we don't duplicate the field name for every delta. This new logic which will skip hidden labels will ensure that multi-value fields can now show the asterisk when required by states.

This also fixes other issues in fields with multiple sub-elements, such as formatted text fields, URL fields, etc.

This patch also includes fixes to formatted text fields by ensuring the .form-no-label is placed on its wrapper.

As for date fields, they currently have their own issues with how they are themed, this patch does somewhat help with the asterisks, but is not completely successful. This patch does however future-proof itself as the Issue 3078334 issue, when it's merged in, combined with this patch will fix *most* asterisk issues on date fields.

Multi-value fields before this patch:

Multi-value fields after this patch:

Formatted text fields before this patch:

Formatted text fields after this patch:

Date fields before this patch:

Date fields after this patch (plus latest patch from Issue 3078334):

Uploaded a new "before" image for the date fields in order to update the previous comment.

Psatch doesn't seem to apply cleanly to 9.5, so I've rerolled.

try to fix ccf in #55

The reroll in #55 clearly went wrong, we get notices and it is not working at all.
Same for #56, which is not clear what it is trying to accomplish.

Here is a proper reroll of #52 for 9.5.x.

Ah got it, #56 was attempting to fix the eslint errors.
This hopefully should work.

edit: Ok \Drupal\KernelTests\Core\Theme\ConfirmClassyCopiesTest tests are failing since text-format-wrapper.html.twig got changed here which changes the hash. This patch changes Stable and Stable9. Is that allowed? I was under the impression that it wasn't... I also notice that not all text-format-wrapper.html.twig files were changed (e.g: claro, bartik, etc).

It would be great if we could write some tests for this, just to make sure that this continues to work in future 😇

Needs reroll as in 10.0.x branch *.es6.js files exists anymore

Patch for 10.1.x.

Still needs test coverage so this is not ready for review yet.

Rerolled patch 54 to resolve warning "Undefined index: title_display in template_preprocess_text_format_wrapper".

Patch doesn't seem to apply cleanly to 9.5, so I've rerolled.

Sorry about my failing patch in #52 not sure why it was not applying when I submitted it.

It seems that many of the subsequent re-rolls have introduced issues or omitted patch changes that were present in #52 that are no longer present in the latest #65.

I have provided updated patch files for D8, D9, and D10/11 (10-1-x currently applies to both versions).

Patches 8.9.x and 9.5.x provide the same changes.

The main differences between 9.5.x and 10.1.x:

1. Removes the states.es6.js which no longer exists.
2. Removes the bartik template changes as the theme no longer exists.
3. Removes the classy template changes as the theme no longer exists.
4. Removes the seven template changes as the theme no longer exists.
5. Removes the stable template changes as the theme no longer exists.
6. Alters the new claro template to be changed in the same fashion as other templates.
7. Alters the new olivero template to be changed in the same fashion as other templates.
8. Alters the new stable9 template to be changed in the same fashion as other templates.
9. Alters the new starterkit_theme template to be changed in the same fashion as other templates.

Besides the re-rolling only a single change has been made to states.js which is to add the lines:

// Labels within a summary.
+ ', details > summary'

edit: Ok \Drupal\KernelTests\Core\Theme\ConfirmClassyCopiesTest tests are failing since text-format-wrapper.html.twig got changed here which changes the hash. This patch changes Stable and Stable9. Is that allowed? I was under the impression that it wasn't... I also notice that not all text-format-wrapper.html.twig files were changed (e.g: claro, bartik, etc).

I am not sure about this, I may need someone with more knowledge as to what is allowed to be changed when it comes to base theme templates.

Regardless whether it's allowed or not, one would need to make the same changes to overridden templates in their own themes as the title_display variable should be provided to all text-format-wrapper.html.twig templates, the theme will need to ensure that the value is utilize to add the form-no-label class or not.

To clarify on these template changes, these are only to fix asterisks on formatted text widgets, so if this needs to be cut from the issue then it's not the end of the world and can be manually fixed on a site-by-site basis.

Created MR, took the patch from #67 (2912092-67-10-1-x.patch), fixed eslint, and one twig missing commas.
Attaching static patch for composer.

Tests still needed. I had a look but we cannot simply replicate in \Drupal\FunctionalJavascriptTests\Core\Form\JavascriptStatesTest, this needs fields on an entity form for every scenario (e.g.: multi-value fields, date fields,...). Also media library fields if possible, which is why I need this patch.

This solved the issue for me in 10.5.2, for simple fields, however, entity reference fields don't show the *, I think the changes from #3342118: When an autocomplete entity reference is required via #states, no required marker is added should be incorporated here
Here's a patch for composer workflows.

MR updated to include the parts from #3342118: When an autocomplete entity reference is required via #states, no required marker is added , credit should go to @penyaskito, @edmoreta and @leoenriquezp I think.

Also attaching a #2912092 + #3342118 patch for composer workflows for 10.5.2

I think this work in #72 adds the required asterisk, but doesn't actually enforce it. So users can submit without filling in the field.

Or maybe that's expected. I know it says this in the docs:

Since states are driven by JavaScript only, it is important to understand that all states are applied on presentation only, none of the states force any server-side logic, and that they will not be applied for site visitors without JavaScript support.

I think this work in #72 adds the required asterisk, but doesn't actually enforce it.

Have not tested, but in the MR I do see JS that is meant to add the required attribute to the HTML element. In which case, if it's working, browser HTML5 validation should enforce required fields to be entered, unless the form is set to novalidate.

11.3.1

I rebased the existing MR !12713.

If we search for form-required in core we can find all places that we want to actually target.
I don't think the current approach is the best though because it tries to match all those and hardcodes all selectors in states.js, while every theme can produce a different markup (template overrides etc).

I think it would be way more reliable to add an arbitrary class, say js-form-item-label to all those places (preprocess and twig). This way we can easily target that class in JS.
The downside being that all contrib and custom themes would need to add it as well ideally, but we can maintain backward compatibility by keeping the existing label selector as fallback, if we don't find our class.
This should cover all cases I believe (standard label, fieldset, details, etc) but in case it doesn't cover a specific use case, states.js uses state:* events on DOM document which allows themes and modules to react to state events.

So I created a new MR !14909 with that approach, any opinions/thoughts are welcome.

Attaching both MR snapshots for composer.

#MR12713 is failing with Drupal 11.2.10 as this particular file https://git.drupalcode.org/project/drupal/-/merge_requests/12713/diffs#d... is not present in the Drupal 11.2.10

I have rerolled the patch to be compatible with D11.2.10

I suggest focusing on MR !14909, following up on #78.
Diagnosis and fix assisted by Claude Code below.

MR !12713

Unlikely to be accepted as it stands. It has no test coverage, asked for in #60 and #64 back in 2022. It hardcodes label positions that only describe core's own markup. And it regresses: closest() can return null and querySelector() is called on it unguarded.

TypeError: Cannot read properties of null (reading 'querySelector')

Repro: a field with '#title_display' => 'invisible' and a #states required, not inside a fieldset. The .form-no-label exclusion from #52 is deliberate, but nothing handles it matching nothing. The throw precedes the required attribute being set, so the field never becomes required. That case works on main today.

The filter changes are the same exclusion again: text-format-wrapper is a js-form-item with no label of its own, so it has to be taught form-no-label too. Every label-less wrapper in core and contrib needs the same treatment.

MR !14909

Each template marks its own label with js-form-label, which states.js looks for, falling back to label[for=...], so nothing depends on markup structure. JavascriptStatesTest covers the fieldset legend and details summary, and fails without the JS change, so contrary to #70 those two do not need an entity form.

The tradeoff from #78 stands: a theme overriding these templates gets no marker on legends or summaries until it adds the class, and plain labels keep working through the fallback. The frontend BC policy allows this ("We will change the markup and CSS that core generates for user interface and theme improvements").

Correction to my previous comment: testing MR !14909 against a real site turned up two cases it missed. I will fix those shortly and leave this in Needs work for feedback.
It cannot move to review with both MRs open and I would rather not force that decision here.