- Issue created by @Rory Downes
- 🇬🇧United Kingdom Rory Downes
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.
- last update
7 months ago 29,676 pass 21:31 20:34 Running- last update
7 months ago 29,676 pass - last update
7 months ago 29,676 pass - last update
7 months ago 29,676 pass - last update
7 months ago 29,676 pass - last update
7 months ago 29,676 pass - Status changed to Needs review
7 months ago 5:17am 6 November 2023 - Status changed to Needs work
7 months ago 2:38pm 6 November 2023 - 🇺🇸United States smustgrave
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.
- last update
7 months ago Patch Failed to Apply - 🇫🇮Finland Sir-Arturio
Taking this on. (Hello from Drupal Contribution Sprints Helsinki!)
- 🇫🇮Finland Sir-Arturio
Was able to commit the patch to the merge request(?) branch. Had problems setting up the contrib environment. Maybe next time!
- 🇬🇧United Kingdom Rory Downes
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
- First commit to issue fork.
- Status changed to Needs review
7 months ago 4:12pm 13 November 2023 - Status changed to Needs work
7 months ago 4:15pm 13 November 2023 - 🇺🇸United States smustgrave
Was previously tagged for tests which are still needed.
- Assigned to Akhil Babu
- Issue was unassigned.
- Status changed to Needs review
6 months ago 10:11am 15 December 2023 - 🇮🇳India Akhil Babu Chengannur
Added tests as per #5. Not sure why spell check is failing
- Merge request !5822Issues/3396669: has-error class not applied to media fields → (Open) created by Akhil Babu
- 🇮🇳India Akhil Babu Chengannur
Akhil Babu → changed the visibility of the branch 3396669-has-error-2 to hidden.
- Status changed to Needs work
6 months ago 1:30pm 15 December 2023 - 🇺🇸United States smustgrave
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
- Status changed to Needs review
6 months ago 12:08pm 19 December 2023 - 🇮🇳India Akhil Babu Chengannur
Thanks @smustgrave. Rebasig fixed the pipeline.
Moving to needs review - Status changed to Needs work
6 months ago 4:01pm 19 December 2023 - 🇺🇸United States smustgrave
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.
- 🇮🇳India Gauravvv Delhi, India
Added before and after patch screenshot in the issue summary
Before patch
After patch
- Status changed to Needs review
5 months ago 10:11am 20 December 2023 - 🇮🇳India Akhil Babu Chengannur
Thanks @smustgrave ,@Gauravvvv . The title was indeed incorrect. Updated the title and issue summary.
Moving back to needs review. - 🇮🇳India Sandeep_k New Delhi
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:- Install Media module
- Create a Content type & add a media field, make this field mandatory.
- Go to content & add new content> Leave media field and save to reproduce. (Shared before results)
- Download this patch and Apply.
- 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.
- Status changed to Needs work
5 months ago 2:25pm 20 December 2023 - 🇺🇸United States smustgrave
Then the test can be removed as it's not testing anything with this issue.
- Status changed to Needs review
5 months ago 3:33am 21 December 2023 - Status changed to RTBC
5 months ago 2:37pm 21 December 2023 - last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - First commit to issue fork.
- 🇳🇿New Zealand quietone New Zealand
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. - last update
5 months ago 29,722 pass - Status changed to Needs work
5 months ago 8:41am 2 January 2024 - Status changed to Needs review
5 months ago 4:04am 3 January 2024 - Status changed to RTBC
5 months ago 5:52am 3 January 2024 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
- last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - last update
5 months ago 29,722 pass - Status changed to Needs work
3 months ago 11:33am 19 February 2024