Overview

ClientsideConversionTrait has SDC specific logic that should be moved into the component source plugin

Proposed resolution

User interface changes

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

larowlan created an issue. See original summary.

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

longwave’s picture

Status: Active » Needs review

First pass at doing the simplest thing possible here, also opened #3497203: Throw exceptions instead of returning tuples for validation errors as passing the violations around is kinda messy when we could just throw exceptions.

I also feel like the source plugins are now doing way too much, SingleDirectoryComponent has 11 dependencies and 41 use statements and we're not really close to finishing it.

tedbow’s picture

Assigned: Unassigned » tedbow

reviewing

tedbow’s picture

Assigned: tedbow » Unassigned
Status: Needs review » Reviewed & tested by the community

I also feel like the source plugins are now doing way too much, SingleDirectoryComponent has 11 dependencies and 41 use statements and we're not really close to finishing it.

I agree that is a lot. But also seems like this issue doesn't make it much works. Adds 1 dependency, entity type manager, but maybe the would removed in #3473336: The UI only handles resolved props values, hence complex widgets do not show the current value in props form (e.g. image with Media Library Widget) which is linked from \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent::findTargetForProps

also seems like findTargetForProps could be replace by generic getTargetForSrc(string $src, string $type) somewhere but maybe it will go away completely.

I had 1 question on the MR but think it is find the way it is

  • tedbow committed 18d36048 on 0.x authored by longwave
    Issue #3495126: Move SDC specific logic out of ClientsideConversionTrait...
tedbow’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @longwave!

wim leers’s picture

Component: Page builder » Data model
Related issues: +#3497203: Throw exceptions instead of returning tuples for validation errors

Nice incremental improvement 👍

@larowlan's post-merge MR comment will be addressed in #3497203: Throw exceptions instead of returning tuples for validation errors 👍

Status: Fixed » Closed (fixed)

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