- Issue created by @jessebaker
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Thanks, @jessebaker!
This will build on top of
\Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated
, which was added in https://git.drupalcode.org/project/experience_builder/-/merge_requests/15, and gained end-to-end test coverage in โจ [MR Only] Edit any component prop, powered by a new FieldForComponentSuggester service, which will power the JS UI Fixed late last week. - Assigned to jessebaker
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Actually, first back to Jesse for
This makes no affordance for wrapping individual editable pieces or attributes within a component but I donโt think it does anything that would prevent us adding that later. I'm going to continue exploring how we might annotate that.
Having that exploration done by @jessebaker would allow me to more holistically tackle this MR ๐
- ๐ฌ๐งUnited Kingdom jessebaker
In order to enable inline editing which I think is the main purpose of the additional annotations we need to
1. Identify the element whose content is editable
2. Identify the field that stores the content
3. Identify the type (plain text vs rich content)The following example shows how using data attributes we might go about that. The presence of a
data-xb-editable
attribute identifies the element. The value of that attribute (combined with the wrapping comment to identify the component) tells us the field and thedata-xb-text-content
would tell us if, when editing we should show CKEditor or just allow text to typed in.<!-- xb-start-fcd2490d-1124-4146-82b6-b1e049ed8026 --> <div class="wrapper"> <div class="content" data-xb-editable="properties.content" data-xb-text-content="rich"> <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Libero vel ullam repellendus eos aliquam tempore nesciunt. Eveniet quidem veniam debitis quas quasi neque? Beatae suscipit qui, doloribus aliquid debitis illum!</p> <p>Natus minus perferendis explicabo provident, in qui sed sapiente, nemo nulla veritatis quae voluptatem numquam eveniet eius est corporis laborum ipsum minima.</p> </div> </div> <!-- xb-end-fcd2490d-1124-4146-82b6-b1e049ed8026 -->
This next example shows how we might annotate a component so that the UI would be able to highlight a specific element in the preview when hovering over the corresponding field in the contextual panel.
<!-- xb-start-fcd2490d-1124-4146-82b6-b1e049ed8026 --> <div class="wrapper"> <img data-xb-editable-attr="[{attr: 'src', field: 'properties.img_src'}, {attr: 'alt', field: 'properties.img_alt'}]" src="https://placehold.co/200x30" alt="alt text for image" height="800" width="600" /> </div> <!-- xb-end-fcd2490d-1124-4146-82b6-b1e049ed8026 -->
We can parse the value of
data-xb-editable-attr
to a JS object and using that mapping, when a user hovers the Image Source or Alternate Text fields in the form, we can draw a border around the<img>
in the preview.Both examples use data-attributes and I understand that there is going to be a challenge in implementing them and that it might not be technically possible without the developer who makes the SDC adding in certain markup/affordances to the template of their SDC.
With that said, if that can be solved, then actually replacing the HTML comment annotations (as outlined in the initial issue description) with
data-
attributes would simplify and optimise things on the front end.I'm not sure there is a scenario where it's not possible to add data attributes but it is somehow possible to add HTML comments. But, if that is the case that I suggest (and I've not put AS much thought into this) something like the following example:
<!-- xb-start-fcd2490d-1124-4146-82b6-b1e049ed8026 --> <div class="wrapper"> <!-- xb-editable-attr-fcd2490d-1124-4146-82b6-b1e049ed8026 src properties.image_src --> <!-- xb-editable-attr-fcd2490d-1124-4146-82b6-b1e049ed8026 alt properties.image_alt --> <img src="https://placehold.co/200x30" alt="alt text for image" height="800" width="600" /> </div> <!-- xb-end-fcd2490d-1124-4146-82b6-b1e049ed8026 -->
In this example the HTML comment immediately preceding an HTML element is what contains the mapping between the editable attribute and the field that has the value.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
The following example shows how using data attributes we might go about that. The presence of a
data-xb-editable
attribute identifies the element.How would that
data-xb-editable
attribute get added?Because product requirement
3. Real-time page preview
states:As a creator, when I'm editing content, I want to see a real time preview of what I'm editing. Custom component code should not have to be aware that it may be rendered in the page builder.
You did anticipate this question (the paragraphs below the last
<hr>
).Would it be possible to achieve at least the first example using again only HTML comments? An SDC prop that has a destination an HTML attribute cannot accept HTML comments, but the first example is actually for a DOM text node, so a HTML comment could work? Is that what the last example is referring to?
But I think that the implication of everything you wrote is that in order to support everything we need for
3. Real-time page preview
is the ability to get an AST for any component, because that's the only way to meet the product requirements? - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
By now, @jessebaker has effectively spelled out what his desired end state is for the markup received by the client side (UI) to achieve the required UX.
I'd still like @jessebaker to respond to #5 (leaving assigned to him for that). After that, I'll take on breaking this up in individually sequenced issues.
- ๐ฌ๐งUnited Kingdom jessebaker
the implication of everything you wrote is that in order to support everything we need for 3. Real-time page preview is the ability to get an AST for any component
During the initial exploration of this a couple of months back, I believe that was the conclusion we came to. Either AST of the component so we can inject things as needed OR implement an API for SDC developers to "opt-in" to having an improved UX by adding some kind of markup or comments to the SDC's that they make. These could be automatically added to SDC's created with the no-code component editor that we plan on building later.
Would it be possible to achieve at least the first example using again only HTML comments?
I'm not actually sure. If the SDC component is just one HTML element and that HTML element's content is the editable piece then yes. However consider the following example where the structure of the SDC is slightly more complex.
<!-- xb-start-fcd2490d-1124-4146-82b6-b1e049ed8026 --> <div class="left"> <div class="content"> <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Libero vel ullam repellendus eos aliquam tempore nesciunt. Eveniet quidem veniam debitis quas quasi neque? Beatae suscipit qui, doloribus aliquid debitis illum!</p> <p>Natus minus perferendis explicabo provident, in qui sed sapiente, nemo nulla veritatis quae voluptatem numquam eveniet eius est corporis laborum ipsum minima.</p> </div> <div class="right"> <img src="..." /> </div> </div> <!-- xb-end-fcd2490d-1124-4146-82b6-b1e049ed8026 -->
Assuming the
div.content
is the editable part we are interested in, is it actually any more feasible to programmatically insert an HTML comment directly above the tag than it is to insert adata-
attribute into the tag? If the lift to achieve either option is similar then thedata-
attribute route has my vote because it's significantly easier to work with in JS.On a different note:
As a creator, when I'm editing content, I want to see a real time preview of what I'm editing. Custom component code should not have to be aware that it may be rendered in the page builder.
(emphasis mine)
Perhaps we could interpret this requirement as "Custom component code must work without being aware it may be rendered in the preview but could be enhanced to work better". E.g. it must render, but a developer can add certain extra markup to enhance the user experience. But I'm not sure we should be trying to game the wording!
TLDR: I also think we need the ability to get an AST for any component to honestly hit that requirement.
- Assigned to wim leers
- Status changed to Postponed
7 months ago 12:35pm 11 June 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ Thanks for confirming!
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Reading this am I right in thinking the comments approach is preferred over the data-attributes?
Is there a plan for how we would implement this?
The data-attributes feels more robust but also has the same brittleness as other places we've used that in core (layout builder blocks, layout builder sections, layout builder regions, contextual links, RDF).
The AST is that the work I have been doing in my spare-time on ๐ Experiment: Render Twig as React Active that I demoed in ๐ฑ Meeting notes: May 20th 2024 - Isomorphic rendering Fixed ?
I had paused it because it felt like it wasn't needed - but are we now saying it will be? If so I could pick it up again.
One other comment on the data-xb-text-content attribute - this sounds a lot like widgets - which is what was implemented in decoupled layout builder. This would fit well with the Component config entity because we could say 'use this widget' for this prop. We could also make the React component for each widgets swappable too (per this question from nod_)
- Issue was unassigned.
- Status changed to Needs review
7 months ago 8:01am 12 June 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Is there a plan for how we would implement this?
HTML comments around components should be easy to achieve. But as described in #4, that won't work for component props (or at least not all possible component props).
I was thinking of splitting "real-time preview" up in 3 parts:
- Real-time preview of rendered components WITHOUT in-place editing (i.e. sidebar prop editing only): ONLY able to click a component and have a form for it appear in the right sidebar, NO ability to in-place edit e.g. the title of a hero component
- Real-time preview of rendered components WITH AFFORDANCES for in-place in-place editing: ONLY able to click a component and have a form for it appear in the right sidebar, with PARTIAL ability to in-place edit: upon hovering component markup marked up for in-place editing we'd be able to highlight in the right sidebar which part of the form corresponds to it
- Real-time preview of rendered components WITH ACTUAL in-place in-place editing: on top of the above, we'd need to use some input widgets, presumably only text initially (think
contenteditable
but with restrictions).
- The first could be a fairly fast iteration, AFAICT it's technically trivial to achieve.
- The second requires an AST and I have no technical details for that in my head at all yet (other than me having grepped
vendor/twig/twig
for "ast" and having found a number of matches, making me hopeful we can achieve this). This require 1โ3 orders of magnitude more work. Once this is done, we may want/need to throw away some work from the first part.
If you could push forward the AST work that'd mean we might be able to rely on that from day one, which might make things simpler ๐ค๐ค That being said: I don't understand yet why Twig-to-React is appropriate โ everything in this issue so far only presumes annotated HTML (components rendered but with additional metadata markup, either HTML comments or attributes), not React. ๐ค
The summary of ๐ Experiment: Render Twig as React Active is empty and its MR has a single commit with no docs โ perhaps explaining the rationale of where that is going might help make me and others grok the intent? ๐ Or perhaps your point was simply that you've already used the Twig AST, and making it generate just the same markup but with additional attributes would be trivial?
(The "render as React" and "JSX" pieces in #3448927 that "we" as in @bnjmnm worked on for a PoC were not intended for rendering the components on the XB canvas, but for the Drupal Field Widgets in the sidebar for editing props *for* the components. See https://git.drupalcode.org/project/experience_builder/-/commit/fa2ece58b...)
- ๐ฌ๐งUnited Kingdom jessebaker
I think @Wim Leers has done a great job of summarising things. At risk of adding too much redundant noise I'll keep this brief.
My suggestion to use wrapping HTML comments and not data attributes was based purely on them being much simpler to implement on the Twig side because of the need to move fast on this. Data attributes would be a preferred/superior solution.
It wouldn't be a huge amount of work to migrate from HTML comments to data attributes should we take a phased approach. We can definitely start with comments and then refactor to attributes later.
I agree with @Wim Leers that in the context of the preview there is no requirement/desire to start rendering anything with React. Any work on producing an AST in this task would purely be to enable the addition of annotative markup. The preview will be rendered inside an iFrame and I have no intention of trying to get React running inside that iFrame. The React app for the XB UI will exist entirely outside of the iFrame and the data attributes will allow us to interrogate/interact with the preview DOM from outside.
- Assigned to larowlan
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
It might be worth setting up a meeting to demo my work - both of you weren't in the isomorphic rendering call. It's all in my private github at the moment because I think it is generic and can be a composer package rather than part of this module.
tl;dr given a twig component, you get Javascript (not JSX) that has been transpiled to what you get from babel's react preset - so basically a function that takes props as args and uses the jsx runtime to render. It needs a lot more work but I have some tricky test cases working.
And yes, it does use the Twig token stream - but there's something important to note about Twig - it does not parse HTML - my work combines it with a mastermind/html5 and a custom event subscriber to build an AST of dom components and Twig logic. The twig token stream treats markup as plain text so there is no concept of component hierarchy. i.e it does not see a 'li' as a child of a 'ul'.
In terms of what's required here - it feels like we want comments before/after a component, comments before/after a prop and components before/after a slot.
I assume that you're going to rely on a round trip back to Drupal to update previews. My Twig to Javascript w/ jsx runtime experiment was to avoid that.
Twig doesn't really see the distinction between a prop and a slot, it just sees them as a print statement - _but_ the issue we get into here is things like logic - e.g. sometimes the twig template could be modifying a prop or passing it through a filter.
So for the comments before/after the component - we can easily do that without much work - we already do that with twig debug (and SDC debug - the emoji thing). The before/after prop/slot thing we will need to drop down to the token stream layer and try to hook into variable nodes.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
comments before/after a prop
"Yes" to the other statements in that paragraph, but "no" to this one. This is the part that is impossible, because any SDC prop targeting a HTML attribute won't be able to use HTML comments: HTML comments aren't allowed inside an attribute!
That's why the conversation above led to the conclusion we eventually want
data-
attributes for everything.I assume that you're going to rely on a round trip back to Drupal to update previews. My Twig to Javascript w/ jsx runtime experiment was to avoid that.
I definitely wanted to avoid that.
_but_ the issue we get into here is things like logic - e.g. sometimes the twig template could be modifying a prop or passing it through a filter.
That is indeed the issue ๐ฌ SDCs should ideally be written without relying on Twig functions that rely on Drupal data. For example:
<div>path (as route) not absolute: {{ path('user.register') }}</div> <div>url (as route) absolute despite option: {{ url('user.register', {}, {'absolute': false }) }}</div>
Any SDC that has a Twig template like that will unavoidably require a round trip (more so still if the parameter to either of those functions is an SDC prop).
So for the comments before/after the component - we can easily do that without much work - we already do that with twig debug (and SDC debug - the emoji thing).
Yep, hence the first part in #10: I could do that without any of the advanced things you're talking about. But we will eventually need all 3 parts, so if you could push that forward, then that'd be fantastic! ๐คฉ
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
I've implemented a precursor to #10.1 (Real-time preview of rendered components WITHOUT in-place editing (i.e. sidebar prop editing only)) using the exact same mechanism the UI is currently using ๐
See it in action at #3455898-3: Connect client & server, with zero changes to client (UI): rough working endpoints that mimic the UI's mocks โ .
I propose to wait for #10.2 + #10.3 until the AST work by @larowlan is ready โ I think there's plenty of other things to do in the short term.
(@jessebaker, @lauriii and I had a call 1.5 hour ago with @larowlan but he couldn't make it unfortunately.)
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ to indicate which steps we're waiting to start on until we have more clarity on @larowlan's AST work.
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Sorry for missing the meeting, tech and brain failure
here's a recording of a demo of what I wanted to show https://drive.google.com/file/d/1XFxiMOW2hDrIqXlzdk5K_8bL3RdpO2X6/view
- Assigned to jessebaker
- Status changed to Needs work
6 months ago 3:39pm 27 June 2024 - ๐ซ๐ฎFinland lauriii Finland
Thank you @larowlan! Pinged @jessebaker to watch the video.
Moving to needs work since the in-place editing needs work still.
- Status changed to Needs review
6 months ago 3:42pm 11 July 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Actually, , because it needs input from @jessebaker, and this is a meta issue, so no actual implementation work happens here.
- Assigned to larowlan
- ๐ฌ๐งUnited Kingdom jessebaker
Just a note to say that I have watched the video and I am in favour of @larowlan's approach. I don't see another alternative to get the level of detail we need in the annotations so unless there are any specific questions about the implementation, I think this is ready to be worked on.
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
to clarify are we talking about the full 'here's some twig give me some JS' or 'here's some twig inject some attributes'
- ๐ฌ๐งUnited Kingdom jessebaker
To start with, injecting the comment annotations suggested in point 2 (Real-time preview of rendered components WITHOUT in-place editing (i.e. sidebar prop editing only)) of the issue description. So "here's some twig inject some attributes".
There is no way we are going to be able to achieve "Twig to Javascript w/ jsx runtime" in time for the Barcelona demo but pushing forward the annotations and AST work will have us moving in the right direction.
- Status changed to Needs work
5 months ago 12:33pm 17 July 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
@larowlan WDYT about limiting the scope of this initially as proposed by @jessebaker?
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
โจ Contextual form values need to be integrated with Redux Active implemented step 2.
Next up is step 3, which is
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Added child ๐ Add HTML comments to twig output to attempt to allow in-place editing Active for the easy-ish bits