#3486236: Add HTML comments to twig output to attempt to allow in-place editing added HTML comments to Twig output, to enable certain XB capabilities without interfering with CSS.
#3491021: Leverage HTML comment annotations, remove wrapping div elements expands on this.
But they result in output like
<!-- xb-start-acf26597-72a4-4833-9cef-97250646576f -->
<div id="block-acf26597-72a4-4833-9cef-97250646576f" class="site-branding block block-system block-system-branding-block">
<div class="site-branding__inner">
<div class="site-branding__text">
<div class="site-branding__name">
<a href="/" title="Home" rel="home">XB</a>
</div>
</div>
</div>
</div>
<!-- xb-end-acf26597-72a4-4833-9cef-97250646576f -->
Even for the anonymous user!
+ slot-conveying comments:
<!-- xb-slot-start-uuid-level-2/the_body -->
…
+ prop-conveying comments:
<h1 style="font-size: 3em; margin: 0.5em 0; color: #333;"><!-- xb-prop-start-uuid-level-3/heading -->Hello, from slot level 3!<!-- xb-prop-end-uuid-level-3/heading --></h1>
(Those last 2 are powered by \Drupal\experience_builder\Extension\XbPropVisitor.)
Make XbTwigExtension only take effect in the context of rendering an XB preview, i.e. when executed by \Drupal\experience_builder\Render\MainContent\XBPreviewRenderer
Show commandsStart 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:
changes
Comments
Comment #2
wim leers…
Comment #3
wim leersThis should be feasible to do at any time, actually; it's not urgent because AFAICT it's low-risk.
Comment #4
roshni upadhyay commentedComment #5
wim leersThanks, @roshni upadhyay!
Comment #6
roshni upadhyay commentedHi @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?
Comment #8
lauriiiComment #9
wim leersComment #11
danielvezaJust 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
Comment #12
wim leersWell on its way, the test coverage you added is excellent, @danielveza — thanks! 😄
Comment #13
danielvezaMade some progress on this today, assigning to me to wrap up
Comment #14
wim leersClarify issue title. Thanks, @danielveza!
Comment #15
wim leersActually, this only affects
\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent, so it belongs under #3520484: [META] Production-ready ComponentSource plugins, not #3521002: [META] Maintainable client-side data model + internal HTTP API.Comment #16
wim leersThis 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: FALSEComment #17
wim leersPer #3473761-15: Render a placeholder DIV inside empty Regions/Slots in the preview, this blocks that.
Comment #18
wim leersComment #19
thoward216 commentedComment #20
wim leersThis MR is not yet updating
\Drupal\experience_builder\Extension\XbWrapperNodeand friends, but I'd be happy to land the current MR first — it just needs the test expectations to be updated now 🤓Comment #21
thoward216 commented@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\XbWrapperNodeand update to only show the HTML comments when previewing.Comment #22
wim leersLooks 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 😄Comment #23
wim leersComment #24
thoward216 commentedHave pushed an update to remove the other HTML comments, unsure if this is the right track or not. Does look like a number of tests are failing though.
Comment #25
thoward216 commentedComment #26
penyaskitoReviewed and LGTM. Assigning to Wim for sign-off.
Comment #27
penyaskitoX-posted 😇
Comment #28
wim leersTruly great work here, @thoward216! Especially retaining test simplicity and hence DX, and your much simpler approach than what I proposed in the issue summary 👏
Also very nice to see @isholgueras' work at #3518838: ComponentSource robustness: add `ComponentSourceTestBase::testSettings()` paying off here :)
Comment #29
wim leersApproved the MR, with 3 bits of missing test coverage that are trivial to add:
As soon as those 3 are addressed, feel free to merge yourself 😄
Comment #30
thoward216 commentedComment #31
penyaskitoComment #34
larowlanCommitted to 0.x thanks!
Comment #35
wim leers