billythekid / conditional-fields

Craft plugin to only use fields if certain conditions (other field values) match
MIT License
12 stars 1 forks source link

Does not appear to be working for Dropdown fields #12

Closed stevecomrie closed 4 years ago

stevecomrie commented 4 years ago

Has this been tested and proven working to conditionally evaluate the selected option of a dropdown/select field?

In my experience, the matching appears to always evaluate to "contains match" for dropdown fields since valueWeHave always contains the value of the first dropdown option (no matter which one is actually selected) and valuesWeHave is an array of all the dropdown option values (again, regardless of which option is actually selected).

I added a couple of lines to get it working for dropdowns, but I just wanted to check and see if I was doing something wrong on my end before I submitted a PR.

Basically it's just adding something like this to the showOrHideTheField function in Conditional.js

    if( _this.fieldToWatch[0].dataset.type.match(/dropdown/gi) ) {
      valueWeHave  = _this.fieldToWatch.find('select').val();
      valuesWeHave = _this.fieldToWatch.find('select').val();
    }
billythekid commented 4 years ago

Heya @stevecomrie

You're not doing anything wrong no! I actually came across this same issue myself yesterday and popped on here to raise my own issue about this to look at when I have a minute.

Please feel free to raise the PR if you already have it fixed and I'll merge it in.

stevecomrie commented 4 years ago

The above code solves the issue for dropdown fields, but I haven't tested it against all possible conditions (i.e. contains / does not contain / equals exactly / etc.)

I also haven't tested it to make sure it doesn't introduce bugs when using any of the other field types, however, the if statement to make sure the field is a "dropdown" before re-setting the valueWeHave and valuesWeHave variables should account for that .. just I haven't tested to confirm it.

billythekid commented 4 years ago

Cheers @stevecomrie I'm happy to look at it further.

As you've seen the JS behind it is very simple so it's likely that I'd use the same code. It's going together in a very bit-by-bit way (hopefully not haphazard) and this code fits that same process, lol.

I can't think of any bugs it would introduce with it matching dropdowns specifically but I was trying to not have it limited to any particular field type. I guess that may be a limitation of the plugin though that I need to add support for each possible type as there's no generic interface for all possible fields, particularly plugin-defined fields.

Yeah I think that I'm happy with this going forward. If plugin authors want fields supported that don't work they'll need to raise a PR / issue for them. I can work through the core craft fields at least.

I'll bang your code in and test it works for dropdowns (which I'm sure it will, of course) and release it when I can.

I have a feeling that radio buttons will also have this bug come to think of it, will need to test but I expect very similar code for that case too!

Cheers

billythekid commented 4 years ago

Thanks for the report @stevecomrie,

Fixed in 0.0.6