search

cloudfour / drizzle

A streamlined tool for developing, documenting and presenting UI pattern libraries.
https://cloudfour.com/thinks/introducing-drizzle/
MIT License
666 stars 32 forks source link

Feature: Restructure `blank.hbs` #52

Closed gerardo-rodriguez closed 8 years ago

gerardo-rodriguez commented 8 years ago

Per the comment here, this PR restructures the blank.hbs template and adds a blank foot block.

cc: @tylersticka @erikjung

erikjung commented 8 years ago

Should the foot block be inside of wrapper?

gerardo-rodriguez commented 8 years ago

Should the foot block be inside of wrapper?

Great question @erikjung!

I initially put the foot block inside the wrapper. In doing so, though, I realized that as soon as you extend the blank.hbs template and pass along content via {{#content "wrapper"}}, this content replaces everything within the blank.hbs wrapper block, including the foot block. This means I can no longer use {{#content "foot"}} in my extended template which I assumed was the goal.

If this is an incorrect assumption, we can treat the foot block the same way we treat the body block. As we currently have it setup, when we extend the blank.hbs template and pass content using the {{#content "wrapper"}} we then create a new body block within this wrapper. We could also create a new foot block a second time in the extended template as well. I guess, to me, it feels a bit odd to ditch the foot block from the blank.hbs template only to recreate a foot block in the extended template a second time.

What are your thoughts? If this isn't making sense, let me know and I can paste in some code examples. Thanks! :)

erikjung commented 8 years ago

@mrgerardorodriguez I'm not sure if it's correct or not anymore 😄

I'll defer to @tylersticka for an opinion on that placement of that foot block.

tylersticka commented 8 years ago

I think foot should go after wrapper, for the reasons @mrgerardorodriguez mentioned. If we put it inside of wrapper, we may as well just remove it altogether.

(I actually wish wrapper were named body and body were named main. But I'm pretty sure that change is too low-level to prioritize now.)

gerardo-rodriguez commented 8 years ago

(I actually wish wrapper were named body and body were named main. But I'm pretty sure that change is too low-level to prioritize now.)

Good point @tylersticka. I considered a similar change like this as well. It may actually not be too bad to make that change. That being said, perhaps we should make that a separate issue/task?

tylersticka commented 8 years ago

It may actually not be too bad to make that change.

I assumed it would be larger because it would require a change to the builder, but I'd love to be wrong about that.

That being said, perhaps we should make that a separate issue/task?

Couldn't agree more! 👍

gerardo-rodriguez commented 8 years ago

I assumed it would be larger because it would require a change to the builder, but I'd love to be wrong about that.

It does require a change in to the builder, but the way it was written, it doesn't seem like a huge task. :)

Couldn't agree more! 👍

Was this a "LGTM" or a "I agree enthusiastically? I couldn't tell. 😁

tylersticka commented 8 years ago

Both, I guess? ¯_(ツ)_/¯