Joomla! Issue Tracker | Joomla! CMS #33176 - [4.0] Fixes radio fields layout

? ?

Fixed in Code Base
20 Apr 2021
Medium
Build: 4.0-dev
# 33176
Diff
YatharthVyas:form-radio-btn-fix

Pending

User tests: Successful: Unsuccessful:

YatharthVyas
17 Apr 2021

Pull Request for Issue #33144

Summary of Changes

This PR changes the CSS class and styling applied to the input type=radio's label. Refer to

joomla-cms/layouts/joomla/form/field/radio/buttons.php

Line 49 in 2c30264

$btnClass = $isBtnGroup ? 'btn btn-outline-secondary' : 'form-check';

Bootstrap suggests that you organize the radio options as:

<div class="form-check">
  <input class="form-check-input" type="radio" name="exampleRadios" id="exampleRadios1" value="option1" checked>
  <label class="form-check-label" for="exampleRadios1">
    Default radio
  </label>
</div>
<div class="form-check">
  <input class="form-check-input" type="radio" name="exampleRadios" id="exampleRadios2" value="option2">
  <label class="form-check-label" for="exampleRadios2">
    Second default radio
  </label>
</div>

This PR changes the form-check class applied to each option's <label> to form-check-label and encloses each option's input-label pair in a <div class="form-check">

Testing Instructions

Change the XML form for any plugin's admin console page to include a radio field like: (similar to the example in Joomla 4 docs)

<field
  name="search_archived"
  type="radio"
  class="switcher"
  label="LABEL_NAME"
  default="0"
>
  <option value="1">JYES</option>
  <option value="0">JNO</option>
</field>

Note that there is NO layout="joomla.form.field.radio.switcher" part.
Please also test for class="btn-group" and class="btn-group-yesno"

For example, in the screenshots below I've removed line 37 from plugins/content/vote/vote.xml to achieve the desired testing condition

joomla-cms/plugins/content/vote/vote.xml

Line 37 in 2c30264

layout="joomla.form.field.radio.switcher"

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

`no class`

`btn-group`

`btn-group-yesno`

Documentation Changes Required

layout="joomla.form.field.radio.switcher" should be mentioned in the Joomla 4 Plugin Tutorial Page's XML Section for making radio fields with 2 options.

69ce6cb 17 Apr 2021

Removed extra styling applied to label

YatharthVyas - open - 17 Apr 2021

YatharthVyas - change - 17 Apr 2021

Status

New

⇒

Pending

joomla-cms-bot - change - 17 Apr 2021

Category

⇒

Layout

rjharishabh - comment - 18 Apr 2021

I have tested this item ✅ successfully on 69ce6cb

particthistle - test_item - 18 Apr 2021 - Tested successfully

particthistle - comment - 18 Apr 2021

I have tested this item ✅ successfully on 69ce6cb

Works as described.

Nice work @YatharthVyas - is that your first PR that's going to be merged for Joomla? If so, welcome on board and I look forward to seeing more soon.

Documentation Required label should be added: The layout parameter should be included in the documentation if it's not there as that should become best practice for extension developers going forward to give a consistent UX in the backend.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

particthistle - comment - 18 Apr 2021

RTC.

@richard67 not sure what's happened there with the first test by @rjharishabh not triggering JTracker properly?

rjharishabh - comment - 18 Apr 2021

RTC.

@richard67 not sure what's happened there with the first test by @rjharishabh not triggering JTracker properly?

@particthistle commenting from github

rjharishabh - comment - 18 Apr 2021

I have tested this item ✅ successfully on 69ce6cb

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

rjharishabh - test_item - 18 Apr 2021 - Tested successfully

YatharthVyas - comment - 18 Apr 2021

@particthistle @rjharishabh, Thanks for testing!

Nice work @YatharthVyas - is that your first PR that's going to be merged for Joomla? If so, welcome on board and I look forward to seeing more soon.

It's my second one that's about to be merged. I'm glad that you found it helpful and thanks for the kind words! I hope to contribute more

Fedik - comment - 18 Apr 2021

Sorry, the fix is incorrect.

This PR removes the extra styling applied to each option's <label>

You have removed possibility to set a class per option, that may come from $option->class

Need to check what exactly cause the issue, which class or which style, to make a proper fix.

~~I can give you a tip: Look in to if ($isBtnYesNo) section, it not used in Joomla 4, the issue may come from there.~~
My tip is wrong sorry :)

YatharthVyas - comment - 18 Apr 2021

Sorry, the fix is incorrect.

This PR removes the extra styling applied to each option's <label>

You have removed possibility to set a class per option, that may come from $option->class

Need to check what exactly cause the issue, which class or which style, to make a proper fix.

~~I can give you a tip: Look in to if ($isBtnYesNo) section, it not used in Joomla 4, the issue may come from there.~~
My tip is wrong sorry :)

The class is still being applied to the checkbox, it's just the label that is not re-using it again. I believe if you want to allow the users to add classes to both the option and label separately then there should be 2 different parameters for them. Doing so would require changes in the XML schema.

Also, the Joomla 3 docs (Link) say that the class is only used to make 'nice coloured buttons' so it feels like the style must only be applied on the buttons, no?

Please correct me if I'm wrong and thanks for the feedback!

Fedik - comment - 18 Apr 2021

The class is still being applied to the checkbox

That is correct, if $option->class exists then it should be applied.

The issue comes from class .form-check, which applied to label, that is incorrect.
Bootstrap doc: https://getbootstrap.com/docs/5.0/forms/checks-radios/#default-stacked

That looks like the markup of radiobutons may need to review, to fit the doc.
You can try to look in to the checboxes layout joomla.form.field.checkboxes, there seems correct markup

Or maybe someone from frontender can give you a tip.
@ciar4n do you have a tip here? ;)

brianteeman - comment - 18 Apr 2021

There is something not right with this PR. You have removed $optionClass from the label but that would appear to have been the only place it was used so you should also have removed all the code that created it as well.

be7ff55 18 Apr 2021

makes options bootstrap friendly and reverts $optionClass

YatharthVyas - change - 18 Apr 2021

Labels

Added: ?

YatharthVyas - comment - 18 Apr 2021

Summary of modifications:

Added the class="<?php echo trim($optionClass . ' ' . $style); ?>" back to the label
And made the layout bootstrap friendly
thanks @Fedik for the help!

Screenshots after the fix:

btn-group

btn-yes-no

Regular

layout="joomla.form.field.radio.switcher"

7b6486b 18 Apr 2021

fix indentation

YatharthVyas - change - 18 Apr 2021

The description was changed

YatharthVyas - edited - 18 Apr 2021

YatharthVyas - change - 18 Apr 2021

The description was changed

YatharthVyas - edited - 18 Apr 2021

YatharthVyas - change - 18 Apr 2021

The description was changed

YatharthVyas - edited - 18 Apr 2021

particthistle - test_item - 18 Apr 2021 - Tested successfully

particthistle - comment - 18 Apr 2021

I have tested this item ✅ successfully on 7b6486b

Tested again. Changes now do what the changes do, even more correctly than earlier in the day.

All four options tested.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

rjharishabh - test_item - 18 Apr 2021 - Tested successfully

rjharishabh - comment - 18 Apr 2021

I have tested this item ✅ successfully on 7b6486b

All four options tested.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

richard67 - change - 18 Apr 2021

Status

Pending

⇒

Ready to Commit

richard67 - comment - 18 Apr 2021

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

YatharthVyas - change - 18 Apr 2021

Title

…

[4.0] ~~Removes extra styling applied to label for radio fields~~

[4.0] Fixes radio fields layout

YatharthVyas - edited - 18 Apr 2021

Quy - comment - 18 Apr 2021

The styling/markup for button group is not correct. See https://getbootstrap.com/docs/5.0/components/button-group/

richard67 - comment - 18 Apr 2021

@Quy So I shall remove "RTC" and add "Updates Requested"?

Quy - comment - 18 Apr 2021

Yes please.

richard67 - change - 18 Apr 2021

Status

Ready to Commit

⇒

Pending

richard67 - comment - 18 Apr 2021

Back to pending, see previous comments.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

richard67 - comment - 18 Apr 2021

@Quy What doesn't match in particular?

Quy - comment - 18 Apr 2021

No spacing between buttons.

e60a68e 18 Apr 2021

fix html for .btn-group

YatharthVyas - change - 18 Apr 2021

Labels

Added: ?

YatharthVyas - comment - 18 Apr 2021

No spacing between buttons.

Thanks, fixed.

<div class="btn-group radio">
   <input class="btn-check" type="radio" id="jform_params_show_total_votes0" name="jform[params][show_total_votes]" value="0" checked="checked">
   <label for="jform_params_show_total_votes0" class="btn btn-outline-secondary"> Hide </label>
   <input class="btn-check" type="radio" id="jform_params_show_total_votes1" name="jform[params][show_total_votes]" value="1" >
   <label for="jform_params_show_total_votes1" class="btn btn-outline-secondary">Show </label>
</div>

For `btn-group`

For `btn-group-yesno`

ab64693 18 Apr 2021

fix spacing for btn-group-yesno

YatharthVyas - change - 18 Apr 2021

Labels

Added: ?
Removed: ?

f58aaf3 18 Apr 2021

Update layouts/joomla/form/field/radio/buttons.php

Quy - change - 18 Apr 2021

Labels

Removed: ?

particthistle - test_item - 19 Apr 2021 - Tested successfully

particthistle - comment - 19 Apr 2021

I have tested this item ✅ successfully on f58aaf3

I did think the button groups didn't quite look right last test... that explains why now.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

chmst - test_item - 19 Apr 2021 - Tested successfully

chmst - comment - 19 Apr 2021

I have tested this item ✅ successfully on f58aaf3

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

richard67 - change - 19 Apr 2021

Status

Pending

⇒

Ready to Commit

richard67 - comment - 19 Apr 2021

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33176.}

rdeutz - close - 20 Apr 2021

rdeutz - merge - 20 Apr 2021

rdeutz - change - 20 Apr 2021

Status	Ready to Commit	⇒	Fixed in Code Base
Closed_Date	0000-00-00 00:00:00	⇒	2021-04-20 06:51:14
Closed_By		⇒	rdeutz
Labels	Added: ?

YatharthVyas - comment - 20 Apr 2021

Thank you for merging. Could you please add the Documentation Changes label for

layout="joomla.form.field.radio.switcher" should be mentioned in the Joomla 4 Plugin Tutorial Page's XML Section for making radio fields with 2 options.

richard67 - comment - 20 Apr 2021

@YatharthVyas But in your description in section "Documentation Changes Required" you wrote:

None, if the PR is merged.

YatharthVyas - comment - 20 Apr 2021

Apologies but it was later that I realized that class="switcher" (the current class attribute mentioned in Joomla 4 Plugin Docs XML section) isn't a valid attribute. It should be one of the following :

layout="joomla.form.field.radio.switcher"
class="btn-group"
class="btn-group-yesno"

I forgot the edit and correct this part.

YatharthVyas - change - 20 Apr 2021

The description was changed

YatharthVyas - edited - 20 Apr 2021

regularlabs - comment - 3 May 2021

This PR removes the ability to set custom class names.
It is now not able to do something like:

<field name="..." type="radio"
        class="btn-group btn-group-yesno my-own-class">

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#33176] - [4.0] Fixes radio fields layout

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

`no class`

`btn-group`

`btn-group-yesno`

Documentation Changes Required

Summary of modifications:

Screenshots after the fix:

btn-group

btn-yes-no

Regular

layout="joomla.form.field.radio.switcher"

For `btn-group`

For `btn-group-yesno`

Add a Comment

[#33176] - [4.0] Fixes radio fields layout

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

no class

btn-group

btn-group-yesno

Documentation Changes Required

Summary of modifications:

Screenshots after the fix:

btn-group

btn-yes-no

Regular

layout="joomla.form.field.radio.switcher"

For btn-group

For btn-group-yesno

Add a Comment

`no class`

`btn-group`

`btn-group-yesno`

For `btn-group`

For `btn-group-yesno`