Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkbox and Toggle don't respect "value_pattern" #1467

Closed
bawright opened this issue Jul 16, 2017 · 14 comments
Closed

Checkbox and Toggle don't respect "value_pattern" #1467

bawright opened this issue Jul 16, 2017 · 14 comments

Comments

@bawright
Copy link

bawright commented Jul 16, 2017

Issue description:

Checkboxes and Toggles do not respect value_pattern and exclude.

Version used:

Tried both, the latest dev and the stable on WP (3.0.9). It works on 3.0.8 as I just tested it.

Using theme_mods or options?

theme_mods

Code to reproduce the issue (config + field(s))

P3_Kirki::add_field( 'p3_config', array(
    'type'        => 'checkbox',
    'settings'    => 'demo_checkbox',
    'label'       => __( 'Intro Paragraph', 'p3' ),
    'section'     => 'layout_post',
    'default'     => '1',
    'priority'    => 10,
    'output'      => array(
        array(
            'element'       => '.single-post .entry-content > p:first-of-type',
            'property'      => 'font-size',
            'value_pattern' => '125%',
            'exclude'       => array( false ),
        ),
        array(
            'element'       => '.single-post .entry-content > p:first-of-type',
            'property'      => 'line-height',
            'value_pattern' => '1.6',
            'exclude'       => array( false ),
        ),
    ),
) );
@ryanlabelle
Copy link

ryanlabelle commented Jul 21, 2017

I'm finding similar results for Switch.

Issue description:

Switches do not respect value_pattern and exclude.

Version used:

3.0.9

// Bundled Font : Ludicrous
Embark_Kirki::add_field( 'embark_theme', array(
    'type'        => 'switch',
    'settings'    => 'headers_typography_ludicrous',
    'label'       => esc_html__( 'Bundled Headings Font', 'embark' ),
    'description' => esc_attr__( 'Enable the bundled "Ludicrous" font for your main headings.', 'embark' ),
    'section'     => 'typography',
    'default'     => 'on',
    'priority'    => 10,
    'choices'     => array(
        'on'  => esc_attr__( 'Enable', 'embark' ),
        'off' => esc_attr__( 'Disable', 'embark' ),
    ),
    'output' => array(
        array(
            'element' => array( 'h1:not(.product_title)', '.th-header-wrap h2'),
            'property' => 'font-family',
            'value_pattern' => 'ludicrous',
            'suffix' => '!important',
            'exclude' => array('0'),
        ),
        array(
            'element' => array( 'h1:not(.product_title)', '.th-header-wrap h2'),
            'property' => 'line-height',
            'value_pattern' => '110%',
            'exclude' => array('0'),
        ),
        array(
            'element' => array( 'h1:not(.product_title)', '.th-header-wrap h2'),
            'property' => 'text-transform',
            'value_pattern' => 'uppercase',
            'exclude' => array('0'),
        ),
        array(
            'element' => array( 'h1:not(.product_title)', '.th-header-wrap h2'),
            'property' => 'letter-spacing',
            'value_pattern' => '0.05em',
            'exclude' => array('0'),
        ),
        array(
            'element' => array( 'h1:not(.product_title)'),
            'property' => 'font-size',
            'value_pattern' => '76px',
            'suffix' => '!important',
            'exclude' => array('0'),
        ),
        array(
            'element' => array('.th-header-wrap h2'),
            'property' => 'font-size',
            'value_pattern' => '42px',
            'suffix' => '!important',
            'exclude' => array('0'),
        ),
        array(
            'element' => array('.single-post h1'),
            'property' => 'font-size',
            'value_pattern' => '56px',
            'suffix' => '!important',
            'exclude' => array('0'),
        ),
    ),

) );

@aristath
Copy link
Contributor

aristath commented Jul 21, 2017

@bawright fixed
@ryanlabelle checkboxes, toggles and switches are pretty much the same thing, they are all just checkboxes. The only thing that changes is their CSS to change the way they are visually represented.
In your code you should replace 'exclude' => array('0'), with 'exclude' => array( false ), because checkboxes save boolean values.
Other than that it now works fine, it was just a faulty check. 👍

Fix will be included in 3.0.10, until then you can use the development branch.

@ryanlabelle
Copy link

@aristath thank you!!

@kodeoagency
Copy link

kodeoagency commented Sep 18, 2017

Hi Aristeides,
in my case the issue still exists. I'm using the current development branch.
I was wondering if my code is correct:

 [
    'label' => 'Uppercase',
    'type' => 'checkbox',
    'default' => false,
    'output' => [
         ['element' => 'p',
           'property' => 'text-transform',
            'value_pattern' => 'uppercase',
            'exclude' => [false]
         ]
    ]
]

@aristath
Copy link
Contributor

That's weird... can you please try with this?
Don't forget to go in the customizer and switch the value on/off to test this.

[
	'label'     => 'Uppercase',
	'type'      => 'checkbox',
	'default'   => false,
	'transport' => 'auto',
	'output'    => [
		[
			'element'       => 'p',
			'property'      => 'text-transform',
			'value_pattern' => 'uppercase',
			'exclude'       => [ false, 0, '0' ]
		]
	]
]

@kodeoagency
Copy link

Unfortunately, that doesn't make any difference.
In my case, the output style is still wrong.
If the checkbox is checked = p {text-transform:true}
If not checked = p {text-transform:false}

@aristath
Copy link
Contributor

@kodeoagency that's weird... I just tested your code and it seems to be working fine.
Are you sure you're testing with the develop branch and not the version from wordpress.org?
It was a bug in v3.0.9 that has been since been fixed (or at least it's supposed to have been fixed and it certainly appears that way in my tests)

@aristath aristath reopened this Sep 20, 2017
@kodeoagency
Copy link

Hi Aristeides.
We don't use the plugin. Instead we implemented Kirki in our framework folder.
Currently we are using 3.0.10-beta.1.
I will send you a download link by email, so that you check our theme and see the problem on your screen.

@kodeoagency
Copy link

Hi Aristeides,
here is an IMPORTANT notice for you.
We figured out that the issue I described only appears in 'postMessage' refreshing.

@aristath
Copy link
Contributor

@kodeoagency I'm still unable to replicate this one.
Would it be possible to post a link to your theme so I can download it and tell me which control exactly to check?

@kodeoagency
Copy link

Hi Aristeides.
We extracted the Kirki function from our theme to twentyseventeen to demonstrate the issue.
Please refer to the configuration in the file kirki-config-static.php
Download the theme here: https://www.dropbox.com/s/4wy33tlx07oktav/twentyseventeen.zip?dl=0

@kodeoagency
Copy link

BTW: You can see the same issue with the multicolor control output, using choices for different CSS attributes

@aristath
Copy link
Contributor

aristath commented Sep 30, 2017

@kodeoagency The problem is that you are using output, js_vars, and set transport to postMessage.
The js_vars you're using don't contain all the arguments of output arguments, and since you set transport to postMessage we don't auto-calculate anything and assume that the provided js_vars are correct.

The solution is simple: DELETE your js_vars and use 'transport' => 'auto'.
When you do that the js_vars will be auto-calculated from your output argument and everything will work as intended.

@kodeoagency
Copy link

Thank you very much! You saved my day!

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

No branches or pull requests

4 participants