Overview

See #3453690-4: [META] Real-time preview: supporting back-end infrastructure
The final comment

Proposed resolution

Do the above, then get rid of wrapping markup in #3491021: Leverage HTML comment annotations, remove wrapping div elements .

User interface changes

None.

Issue fork experience_builder-3486236

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:

About issue forks

Comments

larowlan created an issue. See original summary.

larowlan opened merge request !399

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned
Status: Active » Needs review

I was able to do the easy bits here - prop wrappers for when we're not inside an element or attribute, and wrappers for the component.

For the props-no-slots component in the xb_test_sdc module we get this

<!-- xb-start-9fff3623-d783-44c7-bb94-896d322f0d66 -->
  <div  data-component-id="xb_test_sdc:props-no-slots" style="font-family: Helvetica, Arial, sans-serif; width: 100%; height: 100vh; background-color: #f5f5f5; display: flex; justify-content: center; align-items: center; flex-direction: column; text-align: center; padding: 20px; box-sizing: border-box;">
    <h1 style="font-size: 3em; margin: 0.5em 0; color: #333;">
      <!-- xb-prop-start-heading -->
        se3hqtu5
      <!-- xb-prop-end-heading -->
    </h1>
  </div>
<!-- xb-end-9fff3623-d783-44c7-bb94-896d322f0d66 -->
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Getting HTML attribute support will require a full syntax tree that mixes twig nodes with HTML - which is what I'm working on in a separate PHP library. But I think this is a good step forward.

jessebaker’s picture

jessebaker commented
Assigned: Unassigned » larowlan
Status: Needs review » Needs work

Just adding a comment for extra visibility in case my comment on the MR was missed.

Would you be able to change the HTML comments so that there is a difference between a comment wrapping a slot and a comment wrapping a prop?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review

.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

TIL this was happening!

VERY cool! 🤩

A bunch of question/requests for clarification, but I think this is close to ready already 😄

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Once the bits I do not fully grok yet are clarified by @larowlan, I will approve + merge because the #1 requirement is met:

but the output is what I need and the tests look good!

— @jessebaker stating that this unblocks him on next steps!

The #2 requirement is that I and others understand this sufficiently to maintain/improve this in case @larowlan is unavailable — and that's essentially what my review focuses on 😊

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Issue tags: +blocker

This will enable #3491021: Leverage HTML comment annotations, remove wrapping div elements ! 🚀

larowlan credited longwave.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review

Addressed feedback

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: larowlan » wim leers
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community

While I definitely don't grok every detail of XbTwigExtension, the docs @larowlan added make it 100x more clear what it does. I'm confident now that somebody can step through it with a debugger and combined with @larowlan's comments understand what is going on, and iterate on his work 👍

The things least clear to me at this point:

  • Reliance on $node->hasAttribute('data') and its meaning?!
  • Reliance on $node->hasAttribute('name') and its meaning?!

… but turns out that this is just (AFAICT undocumented, and hard to search for) Twig's architecture, because \Twig\Node\TextNode::compile() uses it too! 🤷

  • wim leers committed 0d02dab4 on 0.x authored by larowlan 
    Issue #3486236 by larowlan, wim leers, jessebaker, longwave: Add HTML...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

This unblocked #3491021: Leverage HTML comment annotations, remove wrapping div elements ! 🚀

Status: Fixed » Closed (fixed)

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