Overview
\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::getClientSideInfo() has grown rather unwieldy recently, between:
- #3493941: Maintain a per-component set of prop expressions/sources
- #3501902: Adding the Image component results in a state considered invalid
having both recently landed.
I worry that it's become impossible to understand. Plus, there's a few leftover @todos that AFAICT are obsolete. For sure this one:
// @todo Add support for default images in SDCs: /components/image/image.component.yml. (And entity references in general.)
(That's literally what #3501902 fixed.)
Proposed resolution
Clarify it based on recent changes. This should also help pave the path for #3493943: SDC+JS Component Sources: split default values into `resolved` and `source`.
User interface changes
None.
Issue fork experience_builder-3507749
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 #3
wim leersComment #5
longwaveI've gone a bit further with the refactoring of
::getClientSideInfo(), several times we initialize one-time variables far away from where we use them so I moved them all closer to their usages, and also reordered some other things that make more sense to me. It's probably best to review individual commits and then the final result as a whole for it to make the most sense.Comment #6
wim leers@longwave took what I did and performed a master class in how to refactor further, step-by-step. The end result is much more readable than what I proposed.
Wonderful, thanks! 🤩👏
Landing this because it's being built on by #3507641: Allow components containing non-required images and no examples to be placed, which is in the sprint. At this point, @longwave and I made equal amounts of changes, and clearly this refactoring is helpful in addressing actual bugs, otherwise he wouldn't have started fixing #3507641 on top of this!
Comment #7
wim leers