Methods in the new ConditionalFieldsFormHelper class are generally static. Let's stop doing that so that we can instantiate objects of that class.

This is a follow-up issue to #2926132: OOify contents of conditional_fields.api.inc and then delete it. Please see that issue for background information.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

colan created an issue. See original summary.

haskweb’s picture

Assigned: Unassigned » haskweb
Status: Active » Needs work
colan’s picture

Status: Needs work » Active

#2: Thanks for offering to work on this!

I don't see any patch here so this should still be Active. Fixing status.

ergonlogic’s picture

@haskweb Have you made any progress on this?

To make quicker progress on #1161314: Add basic Field Group support for ANDing conditions, it'd be handy to be able to refactor ConditionalFieldsFormHelper::afterBuild(). But without instance vars, this'd be nearly pointless.

colan’s picture

Assigned: haskweb » Unassigned

Anyone if free to take this over as there's been no update in a while.

ergonlogic’s picture

We've got this working, though a number of the methods remains static at the moment. We'll continue re-factoring ConditionalFieldsFormHelper in #1161314: Add basic Field Group support for ANDing conditions.

Patch forthcoming.

llamech’s picture

Patch attached.

llamech’s picture

Here's an updated patch with further cleanup.

colan’s picture

Status: Needs review » Needs work
+++ b/src/ConditionalFieldsFormHelper.php
@@ -21,139 +29,143 @@ class ConditionalFieldsFormHelper {
+      $dependees = $dependent_info['dependees'];
+      if (empty($dependees)) {
         continue;
       }

The first line here is producing this on node edit pages with conditional fields (probably where there are no dependees):

Notice: Undefined index: dependees in Drupal\conditional_fields\ConditionalFieldsFormHelper->processDependentFields() (line 57 of modules/contrib/conditional_fields/src/ConditionalFieldsFormHelper.php).

+++ b/src/ConditionalFieldsFormHelper.php
@@ -21,139 +29,143 @@ class ConditionalFieldsFormHelper {
+    $this->processDependentFields();
+    $this->addJavascriptEffects();
+    $this->addValidationCallback();
+
+    return $this->form;

I personally like method chaining, but I'm happy either way so feel free to skip it. So if all of these methods return $this, we can do:

return $this->processDependentFields()->addJavascriptEffects()->addValidationCallback()->form;

Overall, seems to work well in my tests except for the undefined-index error above.

ergonlogic’s picture

I had committed a fix for the "undefined index" warning over in #1161314: Add basic Field Group support for ANDing conditions. I've cherry-picked it here.

I also added the fluent style, as suggested.

ergonlogic’s picture

  • ergonlogic committed 37cd600 on issue-3077803
    Issue #3077803 by ergonlogic, llamech: Make ConditionalFieldsFormHelper...
colan’s picture

Status: Needs review » Reviewed & tested by the community

Much better.

  • 2951e51 committed on 8.x-1.x
    Issue #3077803: Merge branch 'issue-3077803' into 8.x-1.x
    
  • ergonlogic committed 37cd600 on 8.x-1.x
    Issue #3077803 by ergonlogic, llamech: Make ConditionalFieldsFormHelper...
colan’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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