Problem/Motivation

HTML classes should be usually added in templates instead of preprocess functions.

Proposed resolution

Move the HTML classes from the preprocess function to the template. The classes should be added in a template radios.html.twig file.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 after-patch-claro-radios.png540.87 KBhash6
#10 3105583-10.patch1.14 KBkostyashupenko
#5 Screenshot 2020-01-15 at 3.16.12 PM.png1.19 MBsiddhant.bhosale
#2 3105583-2.patch1.13 KBhardik_patel_12

Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

hardik_patel_12 commented
StatusFileSize
new 3105583-2.patch1.13 KB

Kindly apply a new patch

hardik_patel_12’s picture

hardik_patel_12 commented
Assigned: hardik_patel_12 » Unassigned
Status: Needs work » Needs review
siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: Unassigned » siddhant.bhosale
siddhant.bhosale’s picture

siddhant.bhosale commented
StatusFileSize
new Screenshot 2020-01-15 at 3.16.12 PM.png1.19 MB

The patch looks good to me.
The class is being added from the radios.html.twig file.

Adding the screenshot where the added class on the radio is being highlighted.

siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: siddhant.bhosale » Unassigned
Status: Needs review » Reviewed & tested by the community
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll
hardik_patel_12’s picture

hardik_patel_12 commented

@lauriii , can you help here to understand that, what is the issue in template , what needs to be improve for radio template.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

There's nothing wrong with the patch except that it cannot be applied with the latest HEAD of core (due to conflict with #3105580: Move HTML classes from claro_preprocess_checkboxes to a template). Here's instructions for rerolling a patch: https://www.drupal.org/patch/reroll.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 3105583-10.patch1.14 KB
hash6’s picture

hash6 commented
Assigned: Unassigned » hash6
hash6’s picture

hash6 commented
Issue summary: View changes
StatusFileSize
new after-patch-claro-radios.png540.87 KB

Reviewed the patch and it adds the class correctly through the template.

hash6’s picture

hash6 commented
Assigned: hash6 » Unassigned
Status: Needs review » Reviewed & tested by the community
andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/themes/claro/templates/form/radios.html.twig
@@ -0,0 +1,13 @@
+<div{{ attributes.addClass('form-radios','form-boolean-group') }}>{{ children }}</div>

looks a space is missing after comma

  • lauriii committed cf8b8a4 on 9.0.x 
    Issue #3105583 by Hardik_Patel_12, kostyashupenko, hash6, siddhant....

  • lauriii committed eadd5ca on 8.9.x 
    Issue #3105583 by Hardik_Patel_12, kostyashupenko, hash6, siddhant....
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed cf8b8a4 and pushed to 9.0.x and 8.9.x. Thanks!

  • lauriii committed a473cdf on 9.0.x 
    Issue #3105583 follow-up by andypost, lauriii: Move HTML classes from...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

I was supposed to address #14 on commit but I forgot to stage that change before committing. Committed that in a follow-up commit a473cdf.

Status: Fixed » Closed (fixed)

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