themeum / kirki

Extending the customizer
https://kirki.org
MIT License
1.26k stars 328 forks source link

Clearing a by default empty color field doesn't reset, color remains #1720

Closed AlxMedia closed 6 years ago

AlxMedia commented 6 years ago

Issue description:

  1. I have a 'color' option with no default value set.
  2. I set a color in this option in the customizer, it changes color and works
  3. I want to go back to transparent/clear/no option, but when I clear the field so that it is empty, it doesn't reload.
  4. Even when changing other color fields to get it to save, the color remains in the field that is supposed to be empty/clear.

Setting the 'color' option to empty should save it as empty, but it doesn't do that - or am I missing something? I can't get back to the cleared option no matter how I try.

https://screenshots.firefoxusercontent.com/images/45eecc91-32c6-4145-96e5-39b71d836d27.png The field above still loads a red color that I had before emptying the field.

Code example:

Kirki::add_field( 'alx_writeup', array(
    'type'          => 'color',
    'settings'      => 'color-topbar-menu',
    'label'         => esc_attr__( 'Topbar Menu Color', 'writeup' ),
    'description'   => esc_attr__( '', 'writeup' ),
    'section'       => 'styling',
    'default'       => '',
) );

And this:

if ( get_theme_mod('color-topbar-menu','') != '' ) {
    $styles .= '#nav-topbar.nav-container { background: '.get_theme_mod('color-topbar-menu').'; }'."\n";
}
AlxMedia commented 6 years ago

I can send you a theme with the issue if you want. This is the only issue holding me back updating all themes I have to Kirki.

aristath commented 6 years ago

You could do this:

Kirki::add_field( 'alx_writeup', array(
    'type'        => 'color',
    'settings'    => 'color-topbar-menu',
    'label'       => esc_attr__( 'Topbar Menu Color', 'writeup' ),
    'description' => esc_attr__( '', 'writeup' ),
    'section'     => 'styling',
    'default'     => 'rgba(0,0,0,0)',
    'transport'   => 'auto',
    'output'      => array(
        array(
            'element'  => '#nav-topbar.nav-container',
            'property' => 'background-color',
        ),
    )
) );

Note the output argument there... makes the manual css output unnecessary (read more on https://aristath.github.io/kirki/docs/modules/css.html about it) Also the use of 'transport' => 'auto', in combination with the output argument will live-update without a refresh. If you use output & transport in most of your fields it will be a lot smoother and faster for your users 😉 If you want it transparent, then using rgba(0,0,0,0) (or any other rgba color with the alpha channel set to 0) will do what you need.

I can send you a theme with the issue if you want.

That would make things easier 👍

AlxMedia commented 6 years ago

Sent you a mail with one of the themes with the issue. Hopefully it makes it more clear!

Thanks for the explanation here, I'll need to learn and see what I can do. Feel free to quality check the theme and see where I do things wrong.

AlxMedia commented 6 years ago

Were you able to reproduce the issue with the theme? I guess using rgba(0,0,0,0) could work as a workaround, but it would be much nicer to be able to just use an empty field that resets to that when clearing the option.

Also an additional note, I only pointed out that the topbar option didn't work but forgot to say that the same applies to the other 2 fields that are empty, (https://i.imgur.com/Q9pxbVB.png) but I guess you figured that out already :-)

aristath commented 6 years ago

I didn't have time to check this out yet, I'll have some free time to check the theme and find a solution to this tomorrow. I agree, using rgba is a workaround, not a solution. And I hate workarounds. I'll get this fixed 👍

AlxMedia commented 6 years ago

Great, thank you. 👍

aristath commented 6 years ago

Turns out this was a simple typo. Fixed in the develop branch, will be included in the next release 👍

AlxMedia commented 6 years ago

Awesome! 👍 Merry Christmas!