- Issue created by @larowlan
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
I cross-posted with you while you created this. I just wrote:
I want this to be consistent with
ComponentSourceInterface::getExplicitInput()
et cetera.That method and all others on the source is specifying the singular โinputโ.
But youโre right that it could be argued that the plural is better.
So: would you prefer keeping this as-is and doing a follow-up to update the source plugins to match? ๐ค
Curious what you think ๐ค Getting mental models and naming right is hard!
โฆ because I agree with your statement:
there can still be multiple 'inputs' because of multiple props - I think the naming is appropriate - if we need to change it to 'input' could we do that in a follow up?
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
I am leaning towards . Because @larowlan already articulated on the discussion he linked why he feels it should be plural.
Why is it singular now?
The reason they are currently singular:
* @param array $inputs * Component inputs โ both implicit and explicit. โฆ */ public function renderComponent(array $inputs, string $componentUuid, bool $isPreview): array;
which then results in this in the
block
source plugin:public function renderComponent(array $inputs, string $componentUuid, bool $isPreview = FALSE): array { $block = $this->getBlockPlugin(); foreach ($inputs[self::EXPLICIT_INPUT_NAME] ?? [] as $key => $value) { โฆ
and this in the
sdc
source plugin:public function renderComponent(array $inputs, string $componentUuid, bool $isPreview = FALSE): array { return [ '#type' => 'component', '#component' => $this->configuration['local_source_id'], '#props' => ($inputs[self::EXPLICIT_INPUT_NAME] ?? []) + [ โฆ
Proposal
It'd be much clearer to have multiple parameters instead ( ๐ Handle block contexts Active could add another parameter to
::renderComponent()
if needed).So I propose:
- every single method and parameter on
ComponentSourceInterface
should NOT say "input" or "inputs", but "explicit_inputs" - this means
GeneratedFieldExplicitInputUxComponentSourceBase::EXPLICIT_INPUT_NAME
and\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent::EXPLICIT_INPUT_NAME
become obsolete
See outline in the attached patch.
- every single method and parameter on
- ๐ฎ๐ณIndia libbna New Delhi, India
Hi @wim leers,
I was considering working on this issue and, before proceeding, I wanted to confirm my understanding based on your comments. From what I gather, the goal is to rename "explicit_input" to "explicit_inputs" specifically in the ComponentSourceInterface.Could you please confirm if this change should be limited to the interface only, or should I also update the related implementations accordingly?
Additionally, should I go ahead and create a merge request for the patch, or would you suggest any prerequisites before that?
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
or should I also update the related implementations accordingly?
Yes, please! ๐
should I go ahead and create a merge request for the patch
Yes! ๐
Thank you!
- ๐ฎ๐ณIndia libbna New Delhi, India
Thank you for confirming. Assigning the issue to myself.
- ๐ฎ๐ณIndia libbna New Delhi, India
There is one more file,
GeneratedFieldExplicitInputUxComponentSourceBase.php
, under the same interface in which $inputs is used. Should I update the code there as well?
public function getDefaultExplicitInput(): array { $inputs = []; foreach (array_keys($this->configuration['prop_field_definitions']) as $prop_name) { assert(is_string($prop_name)); $inputs[$prop_name] = $this->getDefaultStaticPropSource($prop_name)->toArray(); } return $inputs; }
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Thanks so much for getting this going! ๐๐
Unfortunately, the
phpcs
andphpstan
CI jobs are failing, as are the tests. That means this is not quite ready for review yet.Any chance you could look into fixing those failures?๐ค
- ๐ฎ๐ณIndia libbna New Delhi, India
Sure, I will do that. Also, I'm not able to run drush cr because I'm getting the following error:
'Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent' does not implement the methods 'requiresExplicitInputs', 'getDefaultExplicitInputs', 'getExplicitInputs', 'explicitInputsToClientModel'.
I believe I need to define all the required methods in every file where the interface is implemented.
- ๐ฎ๐ณIndia libbna New Delhi, India
I have resolved the PHPCS and PHPStan issues and unassigned myself so that others can pick it up and work on the test issues.
- First commit to issue fork.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Nice progress here! ๐คฉ
I think
Exception: Warning: Undefined array key "resolved" Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase->hydrateComponent()() (Line: 200)
explains the majority of the test failures, so I recommend looking at that first :)
- First commit to issue fork.