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

Kirki_Helper::compare_values vs. field-dependencies.js boolean issue [fix] #1825

Closed
CaptJiggly opened this issue Mar 7, 2018 · 1 comment
Milestone

Comments

@CaptJiggly
Copy link

CaptJiggly commented Mar 7, 2018

Issue description:

In the required combo below, if one of those values was false (Mainly the != for example), should of caused the CSS to not output for this field. Alas, the CSS was still being spat out even if setting2 was not val4. (I replaced the names/values with random things because the project is private).

I discovered for example inside of the KirkiHelpers::compare_values, the comparisons were done such as:

elseif ( ( '!=' === $operator || 'not equal' === $operator ) && $value1 != $value2 ) { $return = true; }

BUT, inside of field-dependencies.js , they were done as:

else if ( '!==' === operator ) { result = value1 !== value2; }

Which is why the hiding of elements inside of customizer worked properly, but on the PHP side, it was screwing up.

Small oversight it seems, so I changed the PHP compare_values to mimic the JS side and everything seems to be working properly.

Thought I'd send the fix over to ya for review and possibly addition into the next release. Takes a village, right? Let me know if I had an oversight on something you were trying to pull off by validating differently in PHP, but this fixed our issues.

Version used:

develop branch

Using theme_mods or options?

theme_mods, but not applicable to the current issue.

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

Kirki::add_field($opt_name, array(
'section' => 'section1',
'settings' => 'setting_test',
'type' => 'background',
'required' => array(
array(
'setting' => 'setting3',
'operator' => 'contains',
'value' => array('val1', 'val2', 'val3') //I made mods to the code to allow this to work,
),
array(
'setting' => 'setting2',
'operator' => '!=',
'value' => 'val4'
),
array(
'setting' => 'setting3',
'operator' => '==',
'value' => 'val5'
)
),
'default' => array(
'background-color' => '#fff'
),
'output' => array(
array(
'element' => '#element',
'property' => 'background'
)
)
));

Here are the mods I've made to field-dependencies.js and class-kirki-helper.php

class-kirki-helpers.php https://pastebin.com/1Y8QK4aq
field-dependencies.js https://pastebin.com/14qsVBGS

aristath added a commit that referenced this issue Mar 11, 2018
@aristath
Copy link
Contributor

@CaptJiggly thank you for the fixes, I really appreciate it!
Fixed in #1826
I took the opportunity to simplify some things in there too... All those elseif that we previously had are not really needed, the function can simply return.

It should be ok now, I just merged the branch with the fixand also added some phpunit tests to confirm things work the way they should.

Please also test on your end if you can so that we're 100% certain there are no more issues with this 👍

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

No branches or pull requests

2 participants