- Issue created by @wim leers
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
This should be feasible to do at any time, actually; it's not urgent because AFAICT it's low-risk.
- Assigned to roshni upadhyay
- 🇮🇳India roshni upadhyay
Hi @wim leers, The wrapper comments in the rendered output are being added by both the Component and ComponentTreeHydrated classes. I wanted to check if the XbTwigExtension is actively being used, or if we can directly handle the wrapper comments logic within the Component and ComponentTreeHydrated classes instead.
Could you please guide me on whether we should continue using XbTwigExtension or refactor the logic to manage these comments directly within the respective classes?
- Merge request !812Issue:3499352 displayed comments on xb preview mode → (Open) created by Unnamed author
- First commit to issue fork.
- 🇳🇿New Zealand danielveza Brisbane, AU
Just to keep this one moving along, I've started to update some of the tests, had a lot of failures before, lets see how this run goes
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Well on its way, the test coverage you added is excellent, @danielveza — thanks! 😄
- 🇳🇿New Zealand danielveza Brisbane, AU
Made some progress on this today, assigning to me to wrap up
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Actually, this only affects
\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent
, so it belongs under 🌱 [META] Production-ready ComponentSource plugins Active , not 🌱 [META] Production-ready client-side data model + internal HTTP API Active . - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
This will need test coverage for
\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent::renderComponent()
withisPreview: true|false
, which we de facto have now:\Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponentTest::testRenderComponentLive()
tests withisPreview: TRUE
\Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponentTest::testGetClientSideInfo()
tests withisPreview: FALSE
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Per #3473761-15: [PP-1] Render a placeholder DIV inside empty Regions/Slots in the preview → , this blocks that.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
This MR is not yet updating
\Drupal\experience_builder\Extension\XbWrapperNode
and friends, but I'd be happy to land the current MR first — it just needs the test expectations to be updated now 🤓 - 🇬🇧United Kingdom thoward216
@wimleers - I have pushed changes to the tests, and removed the test that was added "simplest component tree with nesting (Non preview)" as I believe it is now a duplicate of "simplest component tree with nesting" which tests "non preview" and "simplest component tree with nesting in preview" which tests in the preview. Everything looks to now be passing.
As mentioned I'll next need to look at
\Drupal\experience_builder\Extension\XbWrapperNode
and update to only show the HTML comments when previewing. - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Looks like this is on the right track, thanks! 👍
Your test expectation changes revealed a bug in
\Drupal\experience_builder\Entity\Pattern::normalizeForClientSide()
though, which I posted a one-liner fix for on the MR 😄