This is a follow-up to my misguided <a class="issue-link js-issue-link" data-error-tex

<a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/us

Also, these test examples are valuable: <a href="https://github.com/shannonmoeller/han

`blank.hbs` should be easier to extend,about cloudfour/drizzle

Comments (31)

gerardo-rodriguez commented on August 10, 2024 1

Sharing some findings...

If I modify in drizzle-builder line 28 of render/pages.js from this:

{{#content 'body'}}${page.contents}{{/content}}

to this:

{{#content 'body' mode='append'}}${page.contents}{{/content}}

you can successfully extend and use append or prepend with the body block and the appropriate mode value giving the expected result in @tylersticka's original attempt.

I feel uncomfortable making that the fix because it seems to not matter whether I add mode='append' or mode='prepend' to the render/pages.js file. I do not understand why this fixes it at the moment.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

@tylersticka @erikjung I'm assuming this could be done before with Fabricator?

I'm testing this right now and have this as my setup:

`blank.hbs`

<!DOCTYPE html>
<html class="{{htmlclass}}">
  <head>
    {{#block "head"}}
       {{!-- (same as before) --}}
    {{/block}}
  </head>
  <body class="{{bodyclass}}">
    {{#block "firstbody"}}

    {{/block}}
    {{#block "foot"}}

    {{/block}}
  </body>
</html>

`default.hbs`

{{#extend "blank" htmlclass="drizzle-Layout" bodyclass="drizzle-Layout-body"}}
  {{#content "head" mode="append"}}
    <link rel="stylesheet" href="{{baseurl}}/assets/drizzle/styles/drizzle.css">
    <link rel="stylesheet" href="{{baseurl}}/assets/toolkit/styles/toolkit.css">
  {{/content}}
  {{#content "firstbody"}}
      {{> drizzle.nav}}
      <div class="drizzle-Layout-main drizzle-u-cf">
        <div class="drizzle-u-pad drizzle-u-rhythm">
          {{#block "secondbody"}}

          {{/block}}
        </div>
      </div>
      <script src="{{baseurl}}/assets/drizzle/scripts/drizzle.js"></script>
      <script src="{{baseurl}}/assets/toolkit/scripts/toolkit.js"></script>
  {{/content}}
{{/extend}}

`collection.hbs`

{{#extend "default"}}
  {{#content "secondbody"}}
    <h1>{{name}}</h1>
    {{#each patterns}}
      {{> drizzle.item}}
    {{/each}}
  {{/content}}
{{/extend}}

With this setup, the secondbody content in collection.hbs is not showing up at all.

from drizzle.

erikjung commented on August 10, 2024

@mrgerardorodriguez

I'm assuming this could be done before with Fabricator?

No, we weren't using the layout helpers for page-level templates with that setup (there was no extending of layouts).

Here is an example of the nesting situation working as expected. Not sure why we're running into issues with our stuff.

https://tonicdev.com/erikjung/5719c9333749a112004f164b

I recommend that we rework things to not require any nested blocks.

from drizzle.

tylersticka commented on August 10, 2024

FWIW, if we need to make the layout extensions more shallow and rely more on partials instead, I'm open to that. I just didn't expect to hit a wall with layout extensions the first time I made one!

from drizzle.

tylersticka commented on August 10, 2024

After chatting with @mrgerardorodriguez and getting closer to a PR for the thing I'm working on, I'm actually leaning toward a simple partial being a better way of doing this FWIW. 😆

This issue might still be an issue. Just wanted to mention that in case it takes the pressure off.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

@tylersticka I was trying to replicate your attempt outside of our environment. Does this match what you were expecting?

https://tonicdev.com/gerardorodriguez/nested-handlebars-block

from drizzle.

tylersticka commented on August 10, 2024

@mrgerardorodriguez It appears to, yes! 😮

from drizzle.

gerardo-rodriguez commented on August 10, 2024

Hmm...interesting. It's almost as if there is something special within our setup and the "body" block within blank.hbs.

from drizzle.

erikjung commented on August 10, 2024

@tylersticka @mrgerardorodriguez

I think the flaw may here:

  {{#content "firstbody"}}
      {{> drizzle.nav}}
      <div class="drizzle-Layout-main drizzle-u-cf">
        <div class="drizzle-u-pad drizzle-u-rhythm">
          {{#block "secondbody"}}

Defining the block within the content. We do this, yet I don't see any examples of this in the helper docs: https://github.com/shannonmoeller/handlebars-layouts#helpers

from drizzle.

erikjung commented on August 10, 2024

Also, these test examples are valuable: https://github.com/shannonmoeller/handlebars-layouts/tree/master/test

from drizzle.

tylersticka commented on August 10, 2024

Defining the block within the content. We do this, yet I don't see any examples of this in the helper docs: https://github.com/shannonmoeller/handlebars-layouts#helpers

I do see it referenced in this issue, though via helpers other than {{#content}}: shannonmoeller/handlebars-layouts#6

from drizzle.

tylersticka commented on August 10, 2024

And the deep-extend test example (see @erikjung's link above) seems to be doing all kinds of crazy stuff across multiple extension depths.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

@erikjung @tylersticka Doesn't this example in tonicdev prove, though, that what Tyler wanted to do is possible?

from drizzle.

gerardo-rodriguez commented on August 10, 2024

BTW, looking back at my first example, we should disregard that. It is inaccurate and not what @tylersticka was trying to do.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

And the deep-extend test example (see @erikjung's link above) seems to be doing all kinds of crazy stuff across multiple extension depths.

Yes, indeed.

from drizzle.

tylersticka commented on August 10, 2024

Doesn't this prove, though, that what Tyler wanted to do is possible?

Yes, what you shared in that comment matches what I wanted to do. And it appears to be possible.

At this point, I've already accomplished the thing I initially wanted to use layouts for, so I'm not sure how to prioritize. But it certainly seems as if we've lost some of the magic superpowers of handlebars-layouts along the line in this specific case.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

Removing myself from assignment for now since I'm not actively looking to solve this.

from drizzle.

erikjung commented on August 10, 2024

Changes applied by this PR should also be incorporated here: cloudfour/cloudfour.com-patterns#155

from drizzle.

megnotarte commented on August 10, 2024

Small - Med

from drizzle.

gerardo-rodriguez commented on August 10, 2024

Keep in mind, the issue we are running into only applies to any blocks that are named body. For example:

`blank.hbs`

<!DOCTYPE html>
<html class="{{htmlclass}}">
  <head>
    {{#block "head"}}
       ...
    {{/block}}
  </head>
  {{#block "wrapper"}}
    <body>
      {{#block "body"}}

      {{/block}}
      {{#block "bodybody"}}

      {{/block}}
    </body>
  {{/block}}
</html>

`another-layout.hbs`

{{#extend "blank"}}
  {{#content "body" mode="append"}}
    <p>Currently, this doesn't work.</p>
  {{/content}}

  {{#content "bodybody" mode="append"}}
    <p>This will work just fine, as expected.</p>
  {{/content}}
{{/extend}}

I wanted to point this out to to clear up an earlier assumption we had where we believed the issue was because the body block was nested within the wrapper block.

This example proves our initial hypothesis was incorrect.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

What I've gathered

After doing some more digging, testing as well as chatting with @erikjung, I now have a much better grasp of what is going on.

To help explain, let's use these:

`templates/blank.hbs`

<!DOCTYPE html>
<html class="{{htmlclass}}">
  <head>
    {{#block "head"}}
      ...
    {{/block}}
  </head>
  {{#block "wrapper"}}
    <body>
      {{#block "body"}}

      {{/block}}
    </body>
  {{/block}}
</html>

`templates/my-custom-layout.hbs`

{{#extend "blank"}}
  {{#content "body" mode="append"}}
    <p>We expect this content to be appended.</p>
  {{/content}}
{{/extend}}

`pages/test-page.hbs`

---
title: Test Page
layout: my-custom-layout
---
<h1>{{title}}</h1>

<p>This is a test page.</p>

The way things are setup right now, the <p>We expect this content to be appended.</p> will never show up because it will be replaced by the content in the pages/test-page.hbs file. This is happening because when drizzle-builder wraps the pages/test-page.hbs content inside of an {{#extend "my-custom-layout"}} it doesn't set a mode value on the {{#content 'body'}} block, therefore, uses the default handlebars-layouts value of replace. (See drizzle-builder line ~28)

Understanding that, I wonder, what do we want to happen? There is nothing wrong with how it is setup as long as we understand and expect it to work the way it does. Are we okay with this? If so, then there is no changes to be made to drizzle-builder.

The current solution

There is a way to do what was originally attempted by @tylersticka. To get what we intended to do (not replace <p>We expect this content to be appended.</p>), we need to do this:

`pages/test-page.hbs`

---
title: Test Page
layout: my-custom-layout
---

{{#extend "my-custom-layout"}}
  {{#content "body" mode="append"}}
    <h1>{{title}}</h1>

    <p>This is a test page.</p>
  {{/content}}
{{/extend}}

This works as was originally intended. What are your thoughts @tylersticka @erikjung?

from drizzle.

tylersticka commented on August 10, 2024

So to summarize the core issue, if you use {{#content mode="append"}}, followed by {{#content mode="replace"}} (which is the default), it will replace everything (default, appended, prepended, etc.). I guess that makes sense.

In that case, I kinda think one of my original suggestions might still make sense:

<!DOCTYPE html>
<html class="{{htmlclass}}">
  <head>
    {{#block "head"}}
      {{!-- (this part stays the same) --}}
    {{/block}}
  </head>
  <body class="{{bodyclass}}">
    {{#block "body"}}

    {{/block}}
    {{#block "foot"}}

    {{/block}}
  </body>
</html>

The main reason I think this might be a good idea is that I think it will be really common to want to shove content below the body elements. This is where most <script> elements will have to go, for example.

I think your suggestion is a good one if you want to do anything fancier than that. But I think it would be nice to have a block ready for the most common use-case.

from drizzle.

erikjung commented on August 10, 2024

There is nothing wrong with how it is setup as long as we understand and expect it to work the way it does. Are we okay with this? If so, then there is no changes to be made to drizzle-builder.

@mrgerardorodriguez To clarify...

What is the fix for the original issue? cloudfour/drizzle-builder#81

Specifically, what would need to change in the page itself (not in any template) in order for @tylersticka's hypothetical "Hello world" example to work?

from drizzle.

gerardo-rodriguez commented on August 10, 2024

What is the fix for the original issue? cloudfour/drizzle-builder#81

@erikjung: Regarding the "Hello world" example in cloudfour/drizzle-builder#81, if we assume:

Given a hypothetical test.hbs layout that extends blank.hbs:
{{#extend "blank"}}
  {{#content "body" mode="append"}}
    world
  {{/content}}
{{/extend}}

You would update the page from this:

---
title: Test
layout: test
---

Hello

To this:

---
title: Test
layout: test
---

{{#extend "test"}}
  {{#content "body" mode="prepend"}}
    Hello
  {{/content}}
{{/extend}}

from drizzle.

gerardo-rodriguez commented on August 10, 2024

So to summarize the core issue, if you use {{#content mode="append"}}, followed by {{#content mode="replace"}} (which is the default), it will replace everything (default, appended, prepended, etc.).

@tylersticka Yes and no? So, in the example I posted today, here's what happens.

templates/my-custom-layout.hbs sets its mode to append and also has content.

pages/test-page.hbs (with only front matter and no {{#extend}}) has content and will have a mode of replace because of how drizzle-builder is setup right now.

So this means the contents of pages/test-page.hbs will replace the contents of templates/my-custom-layout.hbs, which will then be appended to the templates/blank.hbs body block. Did that make sense? I may not be articulating very well.

Keep in mind, all of this we are discussing is only for the body block since that is what drizzle-builder is looking for.

from drizzle.

erikjung commented on August 10, 2024

You would update the page from this

@mrgerardorodriguez IMO, this seems reasonable enough. Maybe I'm just struggling to come up with a more solid way to improve the template extension in general, but if this issue can be bypassed altogether with this small adjustment, maybe that's sufficient for now?

from drizzle.

tylersticka commented on August 10, 2024

I think that's sufficient for now, though as I said before, I do think adding a blank foot block to blank.hbs would be a nice usability default.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

I think that's sufficient for now

I'm fine with this as well for now.

though as I said before, I do think adding a blank foot block to blank.hbs would be a nice usability default

I'm fine by this, I can make these changes. Any objections @erikjung?

from drizzle.

erikjung commented on August 10, 2024

@mrgerardorodriguez No objections from me.

from drizzle.

gerardo-rodriguez commented on August 10, 2024

@tylersticka Should this be considered "done"?

from drizzle.

tylersticka commented on August 10, 2024

@mrgerardorodriguez I think so, especially since #53 exists now (which will carry the torch of this issue).

from drizzle.

`blank.hbs` should be easier to extend about drizzle HOT 31 CLOSED

Comments (31)

`blank.hbs`

`default.hbs`

`collection.hbs`

`blank.hbs`

`another-layout.hbs`

What I've gathered

`templates/blank.hbs`

`templates/my-custom-layout.hbs`

`pages/test-page.hbs`

The current solution

`pages/test-page.hbs`

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent