Fails to render `@inner_content` of layout

[leandrocp]

When a page in rendered with this layout https://github.com/BeaconCMS/beacon/blob/main/guides/recipes/create-a-blog.md#create-the-layout it fails to render the slot and instead render plain text:

[leandrocp]

@cibernox and @cbroeren this one was recently reported and I believe it fails because the <%= @inner_content %> expression is inside a node that also contains an expression and is nested. If you move the <%= @inner_content %> into the root (first level) it works.

[cbroeren]

I dived a bit more into this, and noticed that it's failing to render whenever a conditional slot is expected to be rendered, cq :if={true} vs :if={false}.

(Example layout template)

<div>
  <h1 class="font-heading text-xl font-bold rounded">
     Issue 274 - Layout with conditional inner content
  </h1>

  <h3 class="font-heading text-lg">true</h3>
  <div :if={true} class="border-2 border-slate-200">
    <%= @inner_content %>
  </div>
  
  <h3 class="font-heading text-lg">false</h3>
  <div :if={false} class="border-2 border-slate-200">
    <%= @inner_content %>
  </div>
</div>

(Visual Editor Result)

In our Visual Editor we check whether the ast has a rendered_html value, and if so, use that. If it doesn't have it, we render the inner content as html. https://github.com/BeaconCMS/beacon_live_admin/blob/e4f18f83ae1c1c0c60e4cfdf2b6ae5c114f0fe91/assets/svelte/components/LayoutAstNode.svelte#L14-26

The ast for the sections that conditionally should be rendered, do have the rendered_html, the other ones don't.

(Different ast values for conditional blocks)

From the visual editor perspective, we can only work with the ast we have, and I don't think there's an easy way to cover for this difference from inside the editor itself, because the existence of rendered_html is based on different conditions. Beacon is adding this value based on whether the element has eex attrs, and then sets the rendered_html when it should be rendered. https://github.com/BeaconCMS/beacon/blob/6db870adff76db2bc45d742ec07cc08f134fe3fb/lib/beacon/template/heex/json_encoder.ex#L257-L262 https://github.com/BeaconCMS/beacon/blob/3cd6d605ef456516563f5b1416eb1aea9ea05a48/lib/beacon/template/heex.ex#L63

@leandrocp any ideas on where you think we should make a change or add a check on the eex conditional rendering or the rendered_html being set?

[leandrocp]

Essentially it only supports @inner_content in the root level: {:else if node.tag === "eex" && node.content[0] === "@inner_content"} as proven in this test https://github.com/BeaconCMS/beacon/blob/6f0e4ffdb9a9e03ccc6b6fce424e828075a6e1af/test/beacon/template/heex/json_encoder_test.exs#L464-L488

But it falls into {:else if node.rendered_html} if the node is nested as children of a parent node that contains eex expresions, as per your example:

  test "layout inner_content inside eex" do
    layout_template = ~S|
    <header>my_header</header>
    <div :if={true}>
      <span>Today</span>
      <%= @inner_content %>
    </div>
    |

    page_template = ~S|
    <div>page</div>
    |

    assert_output(
      layout_template,
      [
        %{
          "tag" => "header",
          "attrs" => %{},
          "content" => ["my_header"]
        },
        %{
          "tag" => "div",
          "attrs" => %{":if" => "{true}"},
          "content" => [
            %{"tag" => "span", "attrs" => %{}, "content" => ["Today"]},
            %{
              "tag" => "eex",
              "attrs" => %{},
              "content" => ["@inner_content"],
              "metadata" => %{"opt" => ~c"="},
              "rendered_html" => "\n    &lt;div&gt;page&lt;/div&gt;\n    "
            }
          ],
          "rendered_html" => "<div>\n  <span>Today</span>\n  \n    &lt;div&gt;page&lt;/div&gt;\n    \n</div>"
        }
      ],
      %{inner_content: page_template}
    )
  end

Since the parent div has a rendered_html field, it will ignore the children and just render it. That's tricky to solve, @cibernox mentioned "flag it somehow and the rendered_html have some marker of where to render the nested content" when we first discussed it, something like:

    "metadata": {
       slot_position: 94 // The slot is in the 94th position of the string
     },

Instead of string position, we could maybe mark the index position in the AST:

%{
  "tag" => "div",
  "slots" => [1] # second position in `content` list
  # ...
}

That's important because we can't lose what comes before and after the slot, ie: it should still render the span element so users can edit it, and any other elements that comes after.

Do you think that's something you work with in the visual editor?

[cbroeren]

Essentially it only supports @inner_content in the root level: {:else if node.tag === "eex" && node.content[0] === "@inner_content"} ... But it falls into {:else if node.rendered_html} if the node is nested as children of a parent node that contains eex expressions

That’s not just supported in the root level (from the visual editor side). If it’s nested it will also work because it reaches the same logic for children. It’s just only rendering the @inner_content when it’s the only content in the ast that’s marked as eex (theoretically the first, but I think it should check for only).

Since the parent div has a rendered_html field, it will ignore the children and just render it. That’s tricky to solve,

Correct. But I try to understand the tricky part if that’s because of your next comment:

That’s important because we can’t lose what comes before and after the slot, ie: it should still render the span element so users can edit it, and any other elements that comes after.

In the visual editor, if the ast doesn’t have the rendered_html, it will render it’s content correctly using the list of content items (like the span from your example). Then it will act the same as when the condition is :if={false} or for any other non-conditional parent.

Note: This also affects / applies to conditional elements for pages, similar to the layout. In that case, it will currently also just use the rendered_html and not render each content item separately, so also not being able to select them separately in the editor (the visual part, we still show it in the list of Content in the sidepanel though).

---

This is all considered just from this conditional use case. If it's just about this :if attribute, I would say there might not be a reason for the ast to have or not have the rendered_html. But I know we need to add that there's more logic involved in when and why that's being set (whether it has any other eex attributes for example). So there might be other use cases where the inner content (or any page content) will not be rendered correctly or as separate (selectable/editable) elements.

So the real question is maybe: What eex attributes could/should affect this rendered_html being set? here (maybe_add_rendered_html) and here (has_eex_in_attrs?) And/or when should we have and use the rendered_html as precedence over the content list? Extracting the content from the html string from inside the visual editor sounds really tricky and not favorable.