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.

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

Status: Active » Needs review
StatusFileSize
new8.53 KB

Patch attached.

llamech’s picture

StatusFileSize
new17.41 KB

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

Status: Needs work » Needs review
StatusFileSize
new17.49 KB
new1.69 KB

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.