Skip to content
This repository

Fixes #3496 - Styling of added checkbox incorrect when in controlgroup #4299

merged 1 commit into from about 2 years ago

4 participants

Jasper de Groot Anne-Gaelle Colom Scott Jehl avilaviad
Jasper de Groot

Issue: When a new controlgroup item (button, selectmenu, checkbox/radio) is dynamically injected, the controlgroup corner styling can't be refreshed.
Cause: The controlgroup function doesn't consider situations where the controlgroup element is already styled as a controlgroup.
Fix: Added classes to the list of classes that have to be removed before corner classes getting assigned (again).

Passed all unit tests. Live test page:

Fixes: #3496

Jasper de Groot

To do after this got pulled:

Add information to docs about how to refresh a controlgroup.
Note the difference with refreshing listview and other 'real' widgets: $('#mycontrolgroup').controlgroup(); versus $('#mylist').listview('refresh');

@agcolom could you help with this? Thanks!

Anne-Gaelle Colom
agcolom commented

@uGoMobi Hi Jasper, yes the demo looks really great. I was talking with @toddparker just last week about adding some examples on dynamically generated content as I believe the docs are not great on that at the moment and your demo page looks fantastic for that. What I'd like to do is to have the code above each example, and have other elements as well that are not in a control group. Can I/we reuse your code for that? Do you want to work together on that or do you want me to do it?



Jasper de Groot

hi Anne,

Sure, sounds like a good plan. We can do this together. My suggestion is that you point out what kind of examples and where they need to go (on excisting pages or new ones). Then I will create the examples with the code above it. After that you could add introduction text where needed. Ok?


BTW - This will most probably solve #4297

Anne-Gaelle Colom
agcolom commented

Hi Jasper,

That sounds perfect. I'll start on it tomorrow,


Scott Jehl

This change looks fine to me. Nice work, @uGoMobi . Merging now.

Scott Jehl scottjehl merged commit a21eb90 into from
Scott Jehl scottjehl closed this

This fix is working perfectly if you try to add checkbox to the current visible page,
However, there's a bug within this fix -
when you try to dynamically add checkbox to pageA from pageB the new checkbox style isn't look as part of the controlgroup.


Jasper de Groot

@avilaviad - Can you open a new issue and provide a simple test page that illustrate the issue? Here is a template you can use
If you think the bug is caused by the changes made by merging this pull request you can refer to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 1 addition and 1 deletion. Show diff stats Hide diff stats

  1. +1 1  js/
2  js/
@@ -10,7 +10,7 @@ define( [ "jquery", "./" ], function( $ ) {
10 10
11 11 $.fn.controlgroup = function( options ) {
12 12 function flipClasses( els, flCorners ) {
13   - els.removeClass( "ui-btn-corner-all ui-shadow" )
  13 + els.removeClass( "ui-btn-corner-all ui-corner-top ui-corner-bottom ui-corner-left ui-corner-right ui-controlgroup-last ui-shadow" )
14 14 .eq( 0 ).addClass( flCorners[ 0 ] )
15 15 .end()
16 16 .last().addClass( flCorners[ 1 ] ).addClass( "ui-controlgroup-last" );

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.