Problem/Motivation
As proposed in the parent issue of this issue, all functions in conditional_field.module (except hooks) are candidates to be moved to a service.
Proposed resolution
Create a service to cover the conditional_fields_element_after_build logic.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 3217413-10.patch | 20.58 KB | paulocs |
| #2 | 3217413-2.patch | 20.72 KB | paulocs |
Issue fork conditional_fields-3217413
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
paulocsNotice that I didn't fix any code standard because it is out of the scope of this issue.
Comment #3
colan👍
Comment #4
marcusvsouza commentedThe patch applies without problems and work as expected, does not affect or change module functioning.
Comment #5
colanCould someone do a code review? I just thumbs upped the idea, and don't have time right now.
Comment #6
marcusvsouza commentedI will do the code review!
Comment #7
marcusvsouza commentedThe function logic of “function conditional_fields_element_after_build” was transferred to a service and having no change in its functionality, no code pattern work was done because it escapes the scope of the issue.
Comment #8
colanI was hoping to commit this, but it no longer applies.
Can we get it ready to go again before committing anything else? It's a big and important change. Thanks!
Comment #9
paulocsI'll provide a re-roll.
Comment #10
paulocsComment #12
colanThanks, committed! I re-queued all of the other 4.x RTBCs for testing to ensure they still work.
Comment #13
colanActually, there are method names like
conditionalFieldsIsPriorityField(), which should be renamed to drop the CF prefixes. For example, this one should beisPriorityField(). Let's keep this open until we can update these?Comment #14
paulocsWorking on #13. I'll open a MR.
Comment #16
paulocsComment #17
andregp commented@paulocs' MR correctly updates all methods from ConditionalFieldsElementAlterHelper.php that starts with "conditionalFields...". Thanks!
I have a question though. There are some methods on Conditions.php that also have the CF prefixes (conditionalFieldsDependencyDefaultSettings, conditionalFieldsStates, conditionalFieldsEffects, conditionalFieldsConditions).
@colan, should they be updated too (on a separated issue)?
Comment #19
colanThanks all!
#17: Sure, that would be a good idea.