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

Typography control does not apply changes after save [$100] #717

Closed
aristath opened this issue Mar 3, 2016 · 11 comments
Closed

Typography control does not apply changes after save [$100] #717

aristath opened this issue Mar 3, 2016 · 11 comments
Assignees

Comments

@aristath
Copy link
Contributor

aristath commented Mar 3, 2016

Issue: Typography control stops responding after its changes have been saved

How to replicate:

Install the develop branch of the plugin in a WordPress installation & activate the plugin.
Go to your theme's functions.php file and add the following:

Kirki::add_section( 'kirki_typography_test', array(
    'title'      => esc_attr__( 'Typography Test', 'test' ),
    'priority'   => 1,
    'capability' => 'edit_theme_options',
) );

Kirki::add_field( 'global', array(
    'type'        => 'typography',
    'settings'    => 'typography_test',
    'label'       => esc_attr__( 'Typography Control', 'test' ),
    'description' => esc_attr__( 'This is a composite typography control. It saves the data as an array, and you can define which of the controls you want shown.', 'test' ),
    'tooltip'     => esc_attr__( 'This is a tooltip', 'test' ),
    'section'     => 'kirki_typography_test',
    'priority'    => 10,
    'output'      => array(
        array( 'element' => 'body, body p' ),
    ),
    'default'     => array(
        'font-family'    => 'Roboto',
        'variant'        => 'regular',
        'font-size'      => '1.1em',
        'line-height'    => '1.5',
        'letter-spacing' => '0',
        'color'          => '#333333',
    ),
) );

Then go to the customizer, make a change to the font-family for example, and you should see the font-family of the body change to what you have defined.
Next, save your changes.
Once you've saved your options, try changinf the font-family again.
You'll see that the preview refreshes but the changes made to the control are not getting applied.
the value is properly detected via the control's JS (using control.setting.set( value );, if you do a console.log( value ); you'll see it's fine there), but for some reason it's just not getting detected by the customizer.
Please note that other controls work fine, this appears to be a glitch specific to this control.

There is a $100 open bounty on this issue. Add to the bounty at Bountysource.

@aristath aristath changed the title Typography control does not apply changes after save Typography control does not apply changes after save [$50] Mar 4, 2016
@aristath aristath added the bounty label Mar 4, 2016
@aristath aristath changed the title Typography control does not apply changes after save [$50] Typography control does not apply changes after save [$100] Mar 24, 2016
@michellomp
Copy link

'Please note that other controls work fine, this appears to be a glitch specific to this control.'
When i change the font-family and save all the control are not getting detected not just the font-family
if i change font-size or line height there aren't getting detected

@carasmo
Copy link

carasmo commented Apr 10, 2016

Yes, after save letter spacing also doesn't change.

@aristath
Copy link
Contributor Author

When i change the font-family and save all the control are not getting detected not just the font-family
if i change font-size or line height there aren't getting detected

Yes, everything in the typography control stops working after save.
Other controls though (for example a color control separate from the typography controls, or a repeater) all work fine.
The issue is limited to typography controls.

Yes, after save letter spacing also doesn't change.

Yup, that's because letter-spacing is part of the typography control.

@aristath
Copy link
Contributor Author

@carasmo thank you for contributing to the bounty... Hopefully someone will be able to help us resolve this one!

@carasmo
Copy link

carasmo commented Apr 10, 2016

This is significantly beyond my chops, but have you looked in the Easy Google Fonts customize-preview.js file or any of their stuff? The plugin is really smooth. The head comments read:

 * This file contains all custom jQuery plugins and 
 * code used on the WordPress Customizer screen. It 
 * contains all of the js code necessary to enable 
 * the live previewer. Big performance enhancement 
 * in this version, this file has been completely 
 * rewritten from the ground up to leverage the new 
 * js customizer api.

@manuelmoreale
Copy link
Contributor

Ok after having spent a bit of time on this one I found a workaround for the problem even though I'm not 100% why using this method works while the old approach doesn't.

The workaround is the following:
Instead of passing the value object to the control.setting.set function I create a new temporary object inside the on event. I then copy the content of the value object inside the temporary object and use that as an argument for the set function.
Something like this

this.container.on( 'change', '.text-transform select', function() {

    // Add the value to the array and set the setting's value.
    value['text-transform'] = jQuery( this ).val();
    var tmp = {};

    jQuery.each(value , function(index, el) {
        tmp[index] = el;
    });

    control.setting.set(tmp);

    // Refresh the preview
    wp.customize.previewer.refresh();

});

Now, this is just a workaround and I'm not even 100% sure why is working but maybe it will help @aristath to find the real problem

@carasmo
Copy link

carasmo commented Apr 17, 2016

@manuelmoreale Thanks! I hope this gets sorted soon.

@aristath
Copy link
Contributor Author

Finally fixed!!!
@manuelmoreale thank you for pointing me to the right direction!

The issue was that the object needed to be rebuilt when there was a change, just a JS quirk!

@carasmo
Copy link

carasmo commented Apr 17, 2016

Super excited! Thanks @aristath and @manuelmoreale!

@manuelmoreale
Copy link
Contributor

@aristath nice work.

@michellomp
Copy link

Thanks to both of you

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