Problem/Motivation

If a single checkbox/radio is required, when submitting the form, there is an error (visible in the global error message), but there is no inline error message on the related input, and no error style applied on it (should be red with a border on the left).

This problem only occurs on single checkbox/radio : when it's multiple options, there is an error message on the radio/checkbox group, and a global error style as well.

Proposed resolution

I've realized this is caused by the conditions on $variables['element']['#delta'] in ui_suite_dsfr_preprocess_form_element() : this variable doesn't exist, so "add_error" is always false.
My suggestion would be to use $variables['element']['#error_no_message'] instead, which seems to fit exactly that purpose (in the reverse way).

CommentFileSizeAuthor
#2 checkbox-error-fix-3396957.patch1016 bytestguerineau

Issue fork ui_suite_dsfr-3396957

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mh_nichts created an issue. See original summary.

tguerineau’s picture

tguerineau commented
Status: Active » Needs review
StatusFileSize
new checkbox-error-fix-3396957.patch1016 bytes

Hi,

I've looked into the issue regarding the single checkbox/radio error class/message and have identified the root cause.
I've attached a patch that implements this fix. Here's what I've changed:

1- Replaced the condition checking for #delta with #error_no_message to determine if an error should be added.
2- Adjusted the logic for setting the $error_class variable based on this new condition.

Please test the patch. Looking forward to feedback!

pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau

thanks, I will have a look

pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Needs review » Needs work

Looks great. Can you do a git MR instead of a patch?

tguerineau’s picture

tguerineau commented
Status: Needs work » Needs review

Certainly!

I've created a Merge Request (MR) with the changes.

Let me know if there's anything else needed. Thank you!

pdureau’s picture

pdureau commented
Status: Needs review » Needs work

I see your commit: https://git.drupalcode.org/issue/ui_suite_dsfr-3396957/-/commit/b654066b...

It is a great and I want to merge it, but I don't find the MR (and I don't know how to create it myself. I guess you have to create it).

pdureau’s picture

pdureau commented
Title: Single checkbox/radio error class/message » [beta5] Single checkbox/radio error class/message

tguerineau opened merge request !40

tguerineau’s picture

tguerineau commented
Status: Needs work » Needs review

Thank you for your feedback!

My apologies for the oversight. I've now created the MR. You can review and merge it. Appreciate your patience!

  • pdureau committed 55df713d on 1.0.x authored by tguerineau 
    Issue #3396957 by tguerineau: Single checkbox/radio error class/message
pdureau’s picture

pdureau commented
Status: Needs review » Fixed

Merged, thanks

mh_nichts’s picture

mh_nichts
she/her
Primary language French
commented

Hello,

Thanks to both of you for the quick actions !

However I'm afraid there is a typo in the fix, on line 217 : the condition should be on $variables['add_error'] instead of !$variables['add_error'] (= the error class should be added only if "add_error" is true).

pdureau’s picture

pdureau commented
Status: Fixed » Needs work

Thanks a lot.

Tom'as can you have a look? I guess it is possible to add a commit in the MR.

tguerineau’s picture

tguerineau commented

Hi @pdureau and @mh_nichts,

Thank you for the catch! I've reviewed the code and you're correct about the error class condition.

I've made the necessary update and pushed the commit.
But I've observed that my latest commit addressing the error class condition is present in the repository on the original branch, which confirms the push was successful. However, this update isn't reflected on the changes page linked from the issue.

Could anyone clarify if I should create a new merge request for this fix, or if there is another step I should take?

Appreciate your help on the next steps.

pdureau’s picture

pdureau commented

Hello,

Could anyone clarify if I should create a new merge request for this fix, or if there is another step I should take?

I am more familiar with the "vanilla" gitlab workflow than the Drupal one, so I was bealieving you can add a new commit in your already merged but still available MR: https://git.drupalcode.org/project/ui_suite_dsfr/-/merge_requests/40

Anyway, if you can"t do that, no need for a new issue. You can create a new fork or a new branch in the actual issue.

tguerineau opened merge request !47

tguerineau’s picture

tguerineau commented
Status: Needs work » Needs review

Hi @pdureau,

Following up on the recent discussion, I've created a new branch 3396957-Single-checkbox-radio-error with the updated fix where I've removed the ! operator as suggested. I've pushed this branch and created a new merge request to address the typo mentioned earlier.

Here's the link to the new merge request for review.

I look forward to any reviews or comments.

  • pdureau committed 405062c9 on 1.0.x authored by tguerineau 
    Issue #3396957 by tguerineau, mh_nichts: Single checkbox/radio error...
pdureau’s picture

pdureau commented
Status: Needs review » Fixed

Merged. Credit given to both tguerineau & mh_nichts.

pdureau’s picture

pdureau commented

Status: Fixed » Closed (fixed)

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