Overview

\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::getClientSideInfo() has grown rather unwieldy recently, between:

  1. #3493941: Maintain a per-component set of prop expressions/sources
  2. #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.

Command icon 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

wim leers created an issue. See original summary.

wim leers’s picture

Assigned: Unassigned » larowlan
Status: Active » Needs review

longwave made their first commit to this issue’s fork.

longwave’s picture

I'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.

wim leers’s picture

Assigned: larowlan » Unassigned
Status: Needs review » Reviewed & tested by the community
Issue tags: +blocker

@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!

wim leers’s picture

Status: Reviewed & tested by the community » Fixed

  • wim leers committed 4f793654 on 0.x
    Issue #3507749 by longwave, wim leers: Clarify default 'resolved' vs '...

Status: Fixed » Closed (fixed)

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