has-error class not applied to media fields

Aditi Saraf → made their first commit to this issue’s fork.

The proposed resolution was wrong as it turns out. This class is being applied correctly. It needed some changes to fieldset and form styling to make sure that such a field has the star when mandatory and goes red when the has-error class is added by existing code.

I attach a patch to make these changes that worked for the site where I spotted this issue.

Thank you for reporting. Moving to 11.x as the current dev branch.

Could we have a simple assert to verify the classname is appearing correctly.

Taking this on. (Hello from Drupal Contribution Sprints Helsinki!)

Was able to commit the patch to the merge request(?) branch. Had problems setting up the contrib environment. Maybe next time!

This has been moved to the 11.x branch as that is the current dev branch. However, my patch failed to apply against 11.x and I have not had time to fix that

Was previously tagged for tests which are still needed.

Added tests as per #5. Not sure why spell check is failing

Akhil Babu → changed the visibility of the branch 3396669-has-error-2 to hidden.

May need a rebase but other issues don’t seem to be having it so something that needs to be looked at before it can be merged

Thanks @smustgrave. Rebasig fixed the pipeline.
Moving to needs review

before/after screenshots should be included in Issue summary.

Tests pass without the change so either the test is off or the title is wrong, is the class not being added or is the styling not being added? Based on the answer will need a title/issue summary update to be clear.

Screenshots should be added either way.

Added before and after patch screenshot in the issue summary

Before patch

After patch

Thanks @smustgrave ,@Gauravvvv . The title was indeed incorrect. Updated the title and issue summary.
Moving back to needs review.

Verified and tested patch MR !5371 mergeable on Drupal version- 11.0-dev. The patch was applied successfully and looks good to me.
Testing Steps:

1. Install Media module
2. Create a Content type & add a media field, make this field mandatory.
3. Go to content & add new content> Leave media field and save to reproduce. (Shared before results)
4. Download this patch and Apply.
5. Go back to Add Content and Reverify.

Testing Results:
Now the media field is getting highlighted, sharing after patch results.

We can move this ticket to RTBC.

Then the test can be removed as it's not testing anything with this issue.

I'm triaging RTBC issues → . I read the IS and skimmed comments. The IS explains what this is about and there are before/after screenshots. All very helpful! I didn't find any unanswered questions.

This changes the UI so needs to be tagged for usability. I am adding the tag.

I tested this today on Drupal 11.x, standard install, using the steps in the issue summary. My results agree with what is in the issue summary.

There is a failing test, Drupal\Tests\media\FunctionalJavascript\MediaSourceOEmbedVideoTest so I pressed 'rebase' in Gitlab which will start the tests again.

Merge blocked needs to be rebased

Rebased MR

I tested on Drupal 11.x changes are done as mentioned in the Issue summary now the Media fields are highlighted during validation errors.

Attached screenshot for reference.

Thanks

couple questions

unable to reproduce the issue please check the attachment.

Steps followed
Create a content type with a mandatory media field
Create a new item of content of this type but fail to add a media item.
Click on save and see that the error is reported at the top but that the field is not highlighted in a red border with red label text as other field types.