Make WordPress Core

Opened 9 years ago

Closed 7 years ago

#30951 closed defect (bug) (invalid)

Customizer: visible values (choices) not correct after setting it through javascript

Reported by: katazina's profile katazina Owned by: westonruter's profile westonruter
Milestone: Priority: normal
Severity: normal Version: 4.1
Component: Customize Keywords: has-patch reporter-feedback
Focuses: javascript Cc:

Description (last modified by westonruter)

I have one template which is highly customizable and I created "subtemplates" from that by javascript using the wp.customize api.

After I upgraded to v4.1, I found out, that now the customizer is capable to refresh the values if I change it from javascript and I don't have to reload the customizer page.

But, it has some small issues, including:

radio buttons don't get checked correctly. I can say, they are not checked at all, after changed one subtemplate to another.

I try to explain:

I added new setting:

$wp_customize->add_setting('title_align', array(
        'default' => 0,
        'sanitize_callback' => 'absint'
));
$wp_customize->add_control('title_align', array(
        'label'   => 'Title alignment',
        'settings'=> 'title_align',
        'section' => 'content_sidebar_title',
        'type'    => 'radio',
        'choices' => array(
                0 => 'Left',
                1 => 'Center',
                2 => 'Right',
        )
));

I attached an image, how it lookes like.

With javascript I'm changing the settings (values):

(function($)
{
    $(document).ready(function ()
    {
        var rTSelect = $('input[name="ready_templates"]');
        var api = wp.customize;
                
        rTSelect.click(function()
        {
            var template = $(this).val();
            switch(template)
            {
                case "orange": template_orange(); break;
                case "green": template_green(); break;
                default: template_default(); break;
            }
        });

        var template_default = function() {
            api.instance('title_align').set(1); //center
        };

        var template_orange = function() {
            api.instance('title_align').set(0); //left
        };

        var template_green = function() {
            api.instance('title_align').set(2); //right
        };
    }
});

Now I select the orange template after the green template was selected and saved, and I see, that the "Left" labeled checkbox is not selected. Instead the "Right" labeled checkbox what is selected (checked).

It is only visually incorrect, because if I click on the save button, the data is saved correctly.
But, it will be visually correct, only if I reload the customizer page.

Attachments (2)

wp-customizer-radio-buttons.jpg (3.5 KB) - added by katazina 9 years ago.
setting with radio button
30951.diff (484 bytes) - added by westonruter 9 years ago.
Always cast setting value to string in comparison with radio input value, since the latter will always be a string in the DOM.

Download all attachments as: .zip

Change History (12)

@katazina
9 years ago

setting with radio button

#1 @westonruter
9 years ago

The problem with the checkboxes is that you need to use strings for the setting values. So you should create the control with choices like this:

<?php
// ...
$wp_customize->add_control('title_align', array(
        'label'   => 'Title alignment',
        'settings'=> 'title_align',
        'section' => 'content_sidebar_title',
        'type'    => 'radio',
        'choices' => array(
                '0' => 'Left',
                '1' => 'Center',
                '2' => 'Right',
        )
));

And then you can set a choice via:

api.instance('title_align').set( '1' ); //center

The reason for this is that the radio's synchronizer uses === when comparing the radio's value with updated setting:

api.Element.synchronizer.radio = {
        update: function( to ) {
                this.element.filter( function() {
                        return this.value === to;
                }).prop( 'checked', true );
        },
        refresh: function() {
                return this.element.filter( ':checked' ).val();
        }
};

And in the DOM, an input's value property is always casted to a string, and number can never be identical === with a string ( (1 === '1') === false ).

This being the case, we could patch Core to explicitly cast the new value to a string, since it will always be compared with a string value anyway:

  • src/wp-includes/js/customize-base.js

    diff --git src/wp-includes/js/customize-base.js src/wp-includes/js/customize-base.js
    index 5ffa924..2dc4c8e 100644
    window.wp = window.wp || {}; 
    491491        api.Element.synchronizer.radio = {
    492492                update: function( to ) {
    493493                        this.element.filter( function() {
    494                                 return this.value === to;
     494                                return this.value === String( to );
    495495                        }).prop( 'checked', true );
    496496                },
    497497                refresh: function() {

@westonruter
9 years ago

Always cast setting value to string in comparison with radio input value, since the latter will always be a string in the DOM.

#2 @westonruter
9 years ago

  • Description modified (diff)
  • Keywords has-patch added

Please file a separate ticket for the other issue you reported, which I've removed from the description. It is surely a different issue, probably something specific with the color picker control.

#3 @westonruter
9 years ago

And actually, it would be much better if you used the values 'left', 'center', and 'right' as opposed to the numbers 0, 1, and 2 respectively anyway.

#4 @katazina
9 years ago

comment:1 Thanks, its works like a charm in that way :)
comment:2 I will, I'm testing it a little bit more
comment:3 I know :)

#5 @katazina
9 years ago

With checkboxes if I write it as string, than doesn't work, just with numeric values.
<input type="checkbox" ... />

api.instance('separation').set(0); //works
api.instance('separation').set('0'); //not working

Which should be right?

#6 @westonruter
9 years ago

Please share add_control code.

#7 follow-up: @katazina
9 years ago

$wp_customize->add_setting('image_align_center', array(
    'default'   => 0,
    'sanitize_callback' => 'absint'
));
$wp_customize->add_control('image_align_center', array(
    'label'      => 'Position pictures to the center',
    'section'    => 'cbtc_theme_options',
    'settings'   => 'image_align_center',
    'type'       => 'checkbox'
));

I tried it with:

$wp_customize->add_setting('image_align_center', array(
    'default'   => '0',
    'sanitize_callback' => 'absint'
));

It's maybe, because I called the sanitize_callback function with absint?
But I wrote that for radio buttons also, and there string were what accepted.

#8 @afercia
8 years ago

  • Owner set to westonruter
  • Status changed from new to assigned

@westonruter is this still relevant? I guess many things have changed in the last 12 months.

#9 in reply to: ↑ 7 @westonruter
8 years ago

  • Keywords reporter-feedback added

Replying to katazina:

It's maybe, because I called the sanitize_callback function with absint?
But I wrote that for radio buttons also, and there string were what accepted.

Looking at the Element synchronizer for checkboxes:

        api.Element.synchronizer.checkbox = {
                update: function( to ) {
                        this.element.prop( 'checked', to );
                },
                refresh: function() {
                        return this.element.prop( 'checked' );
                }
        };

Note that it is setting the checked property to the Setting's value. This means that the checkboxes are explicitly expecting the values to be true and false, not enumerated values. The radio button synchronizer looks like it will work with enumerated values, however. Isn't a radio button what you are after anyway?

#10 @westonruter
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from assigned to closed

Closing due to lack of reporter-feedback and apparently incorrect usage of checkbox when select is appropriate.

Note: See TracTickets for help on using tickets.