Horizontal radio buttons, page transition and losing ui-btn-active #5111

Closed
SumoMobi opened this Issue Sep 29, 2012 · 9 comments

Comments

Projects
None yet
3 participants
@SumoMobi

Maybe this is by design, but I have to add the following event handler to my dialogs' page show event so that when the flow returns from another dialog, the original dialog shows the checked radio buttons as checked.

    $("div[data-role='dialog']").each(function ()
    {
        $(this).on('pageshow', function (evt, ui)
        {   //On page transition back to original page/dialog, horizontal radio boxes that are checked, do not look checked.
            $(this).find("input[type='radio']:checked").each(function ()
            {
                if ($(this).closest("fieldset").attr("data-type") == "horizontal")
                {  //A horizontal set of radio buttons.
                    $(this).closest("div").find("label[for]").addClass("ui-btn-active");
                }
            });
        });
    });

I tried class="ui-btn-active ui-state-persist" on the input tag, but that did not help.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Sep 29, 2012

Member

@SumoMobi

Please provide a simple test page that shows the issue. See our Contributing Guidelines. Thanks!

Member

jaspermdegroot commented Sep 29, 2012

@SumoMobi

Please provide a simple test page that shows the issue. See our Contributing Guidelines. Thanks!

@SumoMobi

This comment has been minimized.

Show comment
Hide comment
@SumoMobi

SumoMobi Sep 29, 2012

Hi Jasper

I inserted a simple issue demo page in the issue report.

Thank you.

Jean.

From: Jasper de Groot [mailto:notifications@github.com]
Sent: Saturday, September 29, 2012 12:22 AM
To: jquery/jquery-mobile
Cc: SumoMobi
Subject: Re: [jquery-mobile] Horizontal radio buttons, page transition and losing ui-btn-active (#5111)

@SumoMobi https://github.com/SumoMobi

Please provide a simple test page that shows the issue. See our Contributing Guidelines https://github.com/jquery/jquery-mobile/blob/master/CONTRIBUTING.md#issues . Thanks!


Reply to this email directly or view it on GitHub #5111 (comment) .

Description: Image removed by sender.

Hi Jasper

I inserted a simple issue demo page in the issue report.

Thank you.

Jean.

From: Jasper de Groot [mailto:notifications@github.com]
Sent: Saturday, September 29, 2012 12:22 AM
To: jquery/jquery-mobile
Cc: SumoMobi
Subject: Re: [jquery-mobile] Horizontal radio buttons, page transition and losing ui-btn-active (#5111)

@SumoMobi https://github.com/SumoMobi

Please provide a simple test page that shows the issue. See our Contributing Guidelines https://github.com/jquery/jquery-mobile/blob/master/CONTRIBUTING.md#issues . Thanks!


Reply to this email directly or view it on GitHub #5111 (comment) .

Description: Image removed by sender.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Sep 29, 2012

Member

@SumoMobi

Hi Jean,

Thank you, but could you please put the code in our JS Bin template (http://jsbin.com/uzaret/1/edit). When you paste the code in a post here on Github without using their flavored markdown it becomes one big mess as you can see.

Member

jaspermdegroot commented Sep 29, 2012

@SumoMobi

Hi Jean,

Thank you, but could you please put the code in our JS Bin template (http://jsbin.com/uzaret/1/edit). When you paste the code in a post here on Github without using their flavored markdown it becomes one big mess as you can see.

@SumoMobi

This comment has been minimized.

Show comment
Hide comment
@SumoMobi

SumoMobi Sep 29, 2012

Jasper, I would if I could. It would not let me log in. And without logging in, I do not see a Save button. Says also on psw reset that my email is invalid… Below is the static page. Run it. It takes you through the steps. Thanks.

<title></title>

<meta name="viewport" content="width=device-width, initial-scale=1" />

<link rel="stylesheet" href="http://code.jquery.com/mobile/1.2.0-rc.2/jquery.mobile-1.2.0-rc.2.min.css" />

<script src="http://code.jquery.com/jquery-1.8.1.min.js"></script>

<script src="http://code.jquery.com/mobile/1.2.0-rc.2/jquery.mobile-1.2.0-rc.2.min.js"></script>

<script type="text/ecmascript">

    function _sumo_ShowFirstDialog()

    {

        $.mobile.changePage($("#FirstDlg"));

    }

    function _sumo_ShowSecondDialog()

    {

        $.mobile.changePage($("#SecondDlg"));

    }

    function _sumo_AppendEventHandler()

    {

        $("div[data-role='dialog']").each(function ()

        {

            $(this).on('pageshow', function (evt, ui)

            {   //On page transition back to original page/dialog, horizontal checkboxes that are checked, do not look checked.

                $(this).find("input[type='radio']:checked").each(function ()

                {

                    if ($(this).closest("fieldset").attr("data-type") == "horizontal")

                    {

                        $(this).closest("div").find("label[for]").addClass("ui-btn-active");

                    }

                });

            });

        });

    }

</script>
<div id="ThePage" data-role="page">

    <div data-role="header">

        <h1>Show Horz Radio Btn Issue</h1>

    </div>

    <div data-role="content">

        <p>

            Step 1 : Tap on Show Dialog. Follow stepts in dialog.

        </p>

        <p>

            Step 2 : Tap Append Event Handler and repeat step 1. &nbsp;This time round all will be OK.

        </p>

        <p>

            Note that I am not dealing here with the navbar and removing the ui-btn-active class here as that will derail the illustration.

        </p>

    </div>



    <div data-role="footer">

        <div data-role="navbar">

            <ul id="Ul1" runat="server">

                <li><a onclick="_sumo_ShowFirstDialog();">Show First Dialog</a></li>

                <li><a onclick="_sumo_AppendEventHandler()">Append Event Handler</a></li>

            </ul>

        </div>

    </div>

</div>



<div id="FirstDlg" data-role="dialog">

    <div data-role="header">

        <h1>Show Horz Radio Btn Issue</h1>

    </div>

    <div data-role="content">

        <p>

            Step 1 : Check a horizontal and vertical checkbox, tap Show Second Dialog, close the dialog and see the issue.

        </p>

        <p>

            Step 2 : Close this dialog, tap Append Event Handler in the main page and repeat step 1. &nbsp;This time round all will be OK.

        </p>

            <div data-sumo="typeFilter_flag">

                <fieldset data-role="controlgroup" data-type="horizontal">

                    <input type="radio" name="FltrRad03" id="FltrDlg_status_none" checked="checked" data-sumo="filterNone" />

                    <label for="FltrDlg_status_none">No filter</label>



                    <input type="radio" name="FltrRad03" id="FltrDlg_status_set" />

                    <label for="FltrDlg_status_set">Filter</label>



                </fieldset>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_notstarted">Tasks that had not been started yet</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_notstarted" />

                </div>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_started">Tasks that had been started but not completed</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_started" />

                </div>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_notcompleted">Tasks that had not been completed (started or not)</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_notcompleted" />

                </div>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_completed">Completed tasks</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_completed" />

                </div>

            </div>

    </div>



    <div data-role="footer">

        <div data-role="navbar">

            <ul id="FltrDlg_NavBar" runat="server">

                <li><a onclick="_sumo_ShowSecondDialog();">Show Second Dialog</a></li>

            </ul>

        </div>

    </div>

</div>



<div id="SecondDlg" data-role="dialog">

    <div data-role="header">

        <h1>The Dialog</h1>

    </div>

    <div data-role="content">

        <p>After closing this dialog, the checked horizontal check boxes would have lost their styling.</p>

    </div>

    <div data-role="footer">

        <div data-role="navbar">

            <ul>

                <li><a href="#" data-rel="back">Go Back</a></li>

            </ul>

        </div>

    </div>

</div>

From: Jasper de Groot [mailto:notifications@github.com]
Sent: Saturday, September 29, 2012 3:57 PM
To: jquery/jquery-mobile
Cc: SumoMobi
Subject: Re: [jquery-mobile] Horizontal radio buttons, page transition and losing ui-btn-active (#5111)

@SumoMobi https://github.com/SumoMobi

Hi Jean,

Thank you, but could you please put the code in our JS Bin template (http://jsbin.com/uzaret/1/edit). When you paste the code in a post here on Github without using their flavored markdown it becomes one big mess as you can see.


Reply to this email directly or view it on GitHub #5111 (comment) .

Description: Image removed by sender.

Jasper, I would if I could. It would not let me log in. And without logging in, I do not see a Save button. Says also on psw reset that my email is invalid… Below is the static page. Run it. It takes you through the steps. Thanks.

<title></title>

<meta name="viewport" content="width=device-width, initial-scale=1" />

<link rel="stylesheet" href="http://code.jquery.com/mobile/1.2.0-rc.2/jquery.mobile-1.2.0-rc.2.min.css" />

<script src="http://code.jquery.com/jquery-1.8.1.min.js"></script>

<script src="http://code.jquery.com/mobile/1.2.0-rc.2/jquery.mobile-1.2.0-rc.2.min.js"></script>

<script type="text/ecmascript">

    function _sumo_ShowFirstDialog()

    {

        $.mobile.changePage($("#FirstDlg"));

    }

    function _sumo_ShowSecondDialog()

    {

        $.mobile.changePage($("#SecondDlg"));

    }

    function _sumo_AppendEventHandler()

    {

        $("div[data-role='dialog']").each(function ()

        {

            $(this).on('pageshow', function (evt, ui)

            {   //On page transition back to original page/dialog, horizontal checkboxes that are checked, do not look checked.

                $(this).find("input[type='radio']:checked").each(function ()

                {

                    if ($(this).closest("fieldset").attr("data-type") == "horizontal")

                    {

                        $(this).closest("div").find("label[for]").addClass("ui-btn-active");

                    }

                });

            });

        });

    }

</script>
<div id="ThePage" data-role="page">

    <div data-role="header">

        <h1>Show Horz Radio Btn Issue</h1>

    </div>

    <div data-role="content">

        <p>

            Step 1 : Tap on Show Dialog. Follow stepts in dialog.

        </p>

        <p>

            Step 2 : Tap Append Event Handler and repeat step 1. &nbsp;This time round all will be OK.

        </p>

        <p>

            Note that I am not dealing here with the navbar and removing the ui-btn-active class here as that will derail the illustration.

        </p>

    </div>



    <div data-role="footer">

        <div data-role="navbar">

            <ul id="Ul1" runat="server">

                <li><a onclick="_sumo_ShowFirstDialog();">Show First Dialog</a></li>

                <li><a onclick="_sumo_AppendEventHandler()">Append Event Handler</a></li>

            </ul>

        </div>

    </div>

</div>



<div id="FirstDlg" data-role="dialog">

    <div data-role="header">

        <h1>Show Horz Radio Btn Issue</h1>

    </div>

    <div data-role="content">

        <p>

            Step 1 : Check a horizontal and vertical checkbox, tap Show Second Dialog, close the dialog and see the issue.

        </p>

        <p>

            Step 2 : Close this dialog, tap Append Event Handler in the main page and repeat step 1. &nbsp;This time round all will be OK.

        </p>

            <div data-sumo="typeFilter_flag">

                <fieldset data-role="controlgroup" data-type="horizontal">

                    <input type="radio" name="FltrRad03" id="FltrDlg_status_none" checked="checked" data-sumo="filterNone" />

                    <label for="FltrDlg_status_none">No filter</label>



                    <input type="radio" name="FltrRad03" id="FltrDlg_status_set" />

                    <label for="FltrDlg_status_set">Filter</label>



                </fieldset>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_notstarted">Tasks that had not been started yet</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_notstarted" />

                </div>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_started">Tasks that had been started but not completed</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_started" />

                </div>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_notcompleted">Tasks that had not been completed (started or not)</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_notcompleted" />

                </div>

                <div data-role="fieldcontain">

                    <label for="FltrDlg_status_completed">Completed tasks</label>

                    <input type="radio" name="FltrRad04" id="FltrDlg_status_completed" />

                </div>

            </div>

    </div>



    <div data-role="footer">

        <div data-role="navbar">

            <ul id="FltrDlg_NavBar" runat="server">

                <li><a onclick="_sumo_ShowSecondDialog();">Show Second Dialog</a></li>

            </ul>

        </div>

    </div>

</div>



<div id="SecondDlg" data-role="dialog">

    <div data-role="header">

        <h1>The Dialog</h1>

    </div>

    <div data-role="content">

        <p>After closing this dialog, the checked horizontal check boxes would have lost their styling.</p>

    </div>

    <div data-role="footer">

        <div data-role="navbar">

            <ul>

                <li><a href="#" data-rel="back">Go Back</a></li>

            </ul>

        </div>

    </div>

</div>

From: Jasper de Groot [mailto:notifications@github.com]
Sent: Saturday, September 29, 2012 3:57 PM
To: jquery/jquery-mobile
Cc: SumoMobi
Subject: Re: [jquery-mobile] Horizontal radio buttons, page transition and losing ui-btn-active (#5111)

@SumoMobi https://github.com/SumoMobi

Hi Jean,

Thank you, but could you please put the code in our JS Bin template (http://jsbin.com/uzaret/1/edit). When you paste the code in a post here on Github without using their flavored markdown it becomes one big mess as you can see.


Reply to this email directly or view it on GitHub #5111 (comment) .

Description: Image removed by sender.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Sep 30, 2012

Member

@SumoMobi

You don't need to log in. You can just start editing and the url updates to a new version and it auto-saves after each change you make.

No problem, I copied your code in a JS Bin: http://jsbin.com/uzaret/12/edit

Member

jaspermdegroot commented Sep 30, 2012

@SumoMobi

You don't need to log in. You can just start editing and the url updates to a new version and it auto-saves after each change you make.

No problem, I copied your code in a JS Bin: http://jsbin.com/uzaret/12/edit

@ghost ghost assigned jaspermdegroot Sep 30, 2012

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Sep 30, 2012

Member

New test page that contains all form elements that are affected by this issue: http://jsbin.com/enetay/4

The issue is caused by this line in dialog.js. Not only .ui-slider-bg should be excluded, but also .ui-slider-label-a, .ui-radio-on, .ui-checkbox-on, .ui-selectmenu-list .ui-btn.
For performance it would probably be best to not exclude all those classes but narrow the scope first:

$( this ).find( ".ui-btn." + $.mobile.activeBtnClass ).not( ".ui-radio-on, .ui-checkbox-on, .ui-selectmenu-list .ui-btn" ).removeClass( $.mobile.activeBtnClass );

[Updated the code snippet above, because I made a mistake in the original post]

This is not an issue with regular pages. I was wondering why we added code in dialog.js to remove the active button class while we also take care of this in navigation.js. It was added as fix for #1839 (active state isn't removed from cancel/close button). I removed the code from dialog.js and tested, but didn't see that issue.
So an even better solution would be to just remove this code.

Member

jaspermdegroot commented Sep 30, 2012

New test page that contains all form elements that are affected by this issue: http://jsbin.com/enetay/4

The issue is caused by this line in dialog.js. Not only .ui-slider-bg should be excluded, but also .ui-slider-label-a, .ui-radio-on, .ui-checkbox-on, .ui-selectmenu-list .ui-btn.
For performance it would probably be best to not exclude all those classes but narrow the scope first:

$( this ).find( ".ui-btn." + $.mobile.activeBtnClass ).not( ".ui-radio-on, .ui-checkbox-on, .ui-selectmenu-list .ui-btn" ).removeClass( $.mobile.activeBtnClass );

[Updated the code snippet above, because I made a mistake in the original post]

This is not an issue with regular pages. I was wondering why we added code in dialog.js to remove the active button class while we also take care of this in navigation.js. It was added as fix for #1839 (active state isn't removed from cancel/close button). I removed the code from dialog.js and tested, but didn't see that issue.
So an even better solution would be to just remove this code.

@toddparker

This comment has been minimized.

Show comment
Hide comment
@toddparker

toddparker Oct 1, 2012

Contributor

Interesting. I'm not sure of the history but this dialog code may have pre-dated the nav code. I thinking we test this thoroughly and there don't seem to be regressions, Id remove it post-1.2

Contributor

toddparker commented Oct 1, 2012

Interesting. I'm not sure of the history but this dialog code may have pre-dated the nav code. I thinking we test this thoroughly and there don't seem to be regressions, Id remove it post-1.2

@SumoMobi

This comment has been minimized.

Show comment
Hide comment
@SumoMobi

SumoMobi Oct 1, 2012

Thank you Jasper.

There is more to it… If after checking a different radio button (via JS) that is on a previously shown dialog (in the call hierarchy) and then return to that dialog (similar to what my example was doing), the originally checked radio button will be styled as checked (and not the newly checked button).

Here is my workaround in case it is useful to you:

    $("div[data-role='dialog']").each(function ()

    {

        $(this).on('pageshow', function (evt, ui)

        {   //On page transition back to original dialog, horizontal radio buttons that are checked, do not look checked.

            //And on vertical radio buttons they look checked when they are not.

            $(this).find("input[type='radio']").each(function ()

            {

                if ($(this).prop("checked"))

                {

                    if ($(this).closest("fieldset").attr("data-type") == "horizontal")

                    {

                        $(this).closest("div").find("label[for]").addClass("ui-btn-active");

                    }

                }

                else

                {

                    if ($(this).closest("fieldset").attr("data-type") != "horizontal")

                    {   //Vertical ones still looks checked when they are not.

                        $(this).closest("div").find("label[for]").removeClass("ui-radio-on").addClass("ui-radio-off").find("span .ui-icon-radio-on").removeClass("ui-icon-radio-on").addClass("ui-icon-radio-off");

                    }

                }

            });

        });

    });

Thank you.

From: Jasper de Groot [mailto:notifications@github.com]
Sent: Sunday, September 30, 2012 4:33 PM
To: jquery/jquery-mobile
Cc: SumoMobi
Subject: Re: [jquery-mobile] Horizontal radio buttons, page transition and losing ui-btn-active (#5111)

New test page that contains all form elements that are affected by this issue: http://jsbin.com/enetay/4

The issue is caused by this line https://github.com/jquery/jquery-mobile/blob/master/js/widgets/dialog.js#L71 in dialog.js. Not only .ui-slider-bg should be excluded, but also .ui-slider-bg, .ui-slider-label-a, .ui-radio-on, .ui-checkbox-on, .ui-selectmenu-list .ui-btn.
For performance it would probably be best to not exclude all those classes but narrow the scope first:

$( this ).find( ".ui-btn ." + $.mobile.activeBtnClass ).not( ".ui-selectmenu-list .ui-btn" ).removeClass( $.mobile.activeBtnClass );

This is not an issue with regular pages. I was wondering why we added code in dialog.js to remove the active button class while we also take care of this in navigation.js. It was added as fix for #1839 #1839 (active state isn't removed from cancel/close button). I removed the code from dialog.js and tested, but didn't see that issue.
So an even better solution would be to just remove it.


Reply to this email directly or view it on GitHub #5111 (comment) .

Description: Image removed by sender.

SumoMobi commented Oct 1, 2012

Thank you Jasper.

There is more to it… If after checking a different radio button (via JS) that is on a previously shown dialog (in the call hierarchy) and then return to that dialog (similar to what my example was doing), the originally checked radio button will be styled as checked (and not the newly checked button).

Here is my workaround in case it is useful to you:

    $("div[data-role='dialog']").each(function ()

    {

        $(this).on('pageshow', function (evt, ui)

        {   //On page transition back to original dialog, horizontal radio buttons that are checked, do not look checked.

            //And on vertical radio buttons they look checked when they are not.

            $(this).find("input[type='radio']").each(function ()

            {

                if ($(this).prop("checked"))

                {

                    if ($(this).closest("fieldset").attr("data-type") == "horizontal")

                    {

                        $(this).closest("div").find("label[for]").addClass("ui-btn-active");

                    }

                }

                else

                {

                    if ($(this).closest("fieldset").attr("data-type") != "horizontal")

                    {   //Vertical ones still looks checked when they are not.

                        $(this).closest("div").find("label[for]").removeClass("ui-radio-on").addClass("ui-radio-off").find("span .ui-icon-radio-on").removeClass("ui-icon-radio-on").addClass("ui-icon-radio-off");

                    }

                }

            });

        });

    });

Thank you.

From: Jasper de Groot [mailto:notifications@github.com]
Sent: Sunday, September 30, 2012 4:33 PM
To: jquery/jquery-mobile
Cc: SumoMobi
Subject: Re: [jquery-mobile] Horizontal radio buttons, page transition and losing ui-btn-active (#5111)

New test page that contains all form elements that are affected by this issue: http://jsbin.com/enetay/4

The issue is caused by this line https://github.com/jquery/jquery-mobile/blob/master/js/widgets/dialog.js#L71 in dialog.js. Not only .ui-slider-bg should be excluded, but also .ui-slider-bg, .ui-slider-label-a, .ui-radio-on, .ui-checkbox-on, .ui-selectmenu-list .ui-btn.
For performance it would probably be best to not exclude all those classes but narrow the scope first:

$( this ).find( ".ui-btn ." + $.mobile.activeBtnClass ).not( ".ui-selectmenu-list .ui-btn" ).removeClass( $.mobile.activeBtnClass );

This is not an issue with regular pages. I was wondering why we added code in dialog.js to remove the active button class while we also take care of this in navigation.js. It was added as fix for #1839 #1839 (active state isn't removed from cancel/close button). I removed the code from dialog.js and tested, but didn't see that issue.
So an even better solution would be to just remove it.


Reply to this email directly or view it on GitHub #5111 (comment) .

Description: Image removed by sender.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Oct 1, 2012

Member

@toddparker

I mentioned the simple solution (excluding those classes) as well because I was thinking we could land that in 1.1.2 and 1.2.1. For 1.3 we are going to work on nav refactor and active state of buttons so that seems a good moment to see if we can remove this code.

@SumoMobi

Thanks for posting a workaround but let's establish the issue first. I created another test page: http://jsbin.com/enetay/7
This page uses JQM latest JS including the fix I mentioned in my previous comment.
On this test page I set the checked attribute of the checkboxes and radio buttons with JS. I don't see a problem.

Member

jaspermdegroot commented Oct 1, 2012

@toddparker

I mentioned the simple solution (excluding those classes) as well because I was thinking we could land that in 1.1.2 and 1.2.1. For 1.3 we are going to work on nav refactor and active state of buttons so that seems a good moment to see if we can remove this code.

@SumoMobi

Thanks for posting a workaround but let's establish the issue first. I created another test page: http://jsbin.com/enetay/7
This page uses JQM latest JS including the fix I mentioned in my previous comment.
On this test page I set the checked attribute of the checkboxes and radio buttons with JS. I don't see a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment