I have a custom module which defines a new widget. As far as I can tell, I cannot make FCS aware of my widget and the states it supports (at least not without hacking the module).

Can we have a way to alter the array coming out of _field_conditional_state_get_field_states()? A simple drupal_alter should suffice I guess.

Maybe I'm over-simplifying things, but I think exposing an alter hook would make it much easier for other contrib projects to support FCS. Maintainers wouldn't need to ask to be supported in this issue queue, they could simply implement it themselves.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Tobias Xy’s picture

Yes, I will add a hook for that. :-)

Tobias Xy’s picture

Version: 7.x-2.0 » 7.x-2.x-dev
Status: Active » Needs review
FileSize
19.54 KB
2.07 KB

I cleaned up the code a bit to be able to implement the hook hook_field_conditional_states_settings_alter.
See the field_conditional_states.api.php to learn how to use it.

I had to remove the tests, because they will be always successful now -> senseless.

Tobias Xy’s picture

FileSize
19.54 KB

New patch. Use hook_field_conditional_state_settings_alter instead of hook_field_conditional_states_settings_alter.

marcvangend’s picture

I haven't tried it yet, but just seeing the code this looks very promising.

Just a thought: maybe it's a good idea to add static caching (or even database caching) in _field_conditional_state_get_element_settings(), so you don't have build the settings array and do a drupal_alter on it every time the function is called.

Tobias Xy’s picture

FileSize
20.1 KB

True, that would be a good thing. New patch including static caching.

The last submitted patch, 2: add_hook-2270781-2.patch, failed testing.

The last submitted patch, 3: add_hook-2270781-3.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: add_hook-2270781-5.patch, failed testing.

Tobias Xy’s picture

FileSize
20.23 KB
Tobias Xy’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: add_hook-2270781-9.patch, failed testing.

Tobias Xy’s picture

Status: Needs work » Needs review
FileSize
20.61 KB

Yes, we don't have any tests currently... I know.

Status: Needs review » Needs work

The last submitted patch, 12: add_hook-2270781-12.patch, failed testing.

Tobias Xy’s picture

Status: Needs work » Needs review
FileSize
20.32 KB

I'm starting to hate the testbot...

hass’s picture

+++ b/field_conditional_state.module
@@ -282,22 +365,16 @@ function _field_conditional_state_get_field_states($type) {
+    case 'taxonomy_shs':

What does shs mean? Please add some inline documentation and never abbreviate... it hurts back somedays :-)

Testbot is the last line of defense against QA issues. :-)

Tobias Xy’s picture

Thats just the name of the widget as it is defined by Simple hierarchical select.
They abbreviated it, so I have to use it that way ;-)

The patches 9 and 12 should work exactly as 14 does. The testbot only didn't like it, that I removed the tests, because they are not longer needed with this patch.

Tobias Xy’s picture

Title: Allow modules to alter _field_conditional_state_get_field_states() » Add hook to allow other modules to add support by themselves

Changed title, still needs review.

  • Tobias Xy committed 5cacb9c on 7.x-2.x
    Issue #2270781: Add hook to allow other modules to add support by...
Tobias Xy’s picture

Status: Needs review » Fixed
FileSize
20.24 KB

Committed (a slighlty different patch to respect the recent changes).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.