Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nested layout: layout with its own layout #14

Open
wants to merge 4 commits into
base: master
from

Conversation

@echaozh
Copy link
Contributor

echaozh commented Oct 9, 2013

Test is included.

Layout for layout is useful, and is supported in Jekyll. I refactored the layout finding code out and call it again in partial() before the first layout is rendered.

All tests are passing.

The earlier layout explicit set to false fix is included too. The code change in lib is moved to helpers/raw.js with the whole layout finding logic. The test is still where it was, though.

echaozh added 4 commits Oct 8, 2013
1. layout with `layout` defined in _data.json will be rendered
inside that layout after including its own yielded content
2. layout's layout has to be explicitly defined, so nearest
layout will not be used in mistake. Also, layout with itself as
layout will be ignored the second time
3. layout finding code is yanked from `render()` into `helpers` as
a standalone function. `partial()` calls `findLayout()` to find
an explicit layout for the to-be-rendered layout itself.
When explicitly set layout to false in _data.json and put a file
in the outside directory with the same name as the directory name
and a template extension, that file will be used as layout. In the
test case, the file is false.jade outside the false/ directory.

This is because false is not explicitly checked in the layout
lookup, and is treated as empty string.
@kennethormandy

This comment has been minimized.

Copy link
Collaborator

kennethormandy commented Oct 9, 2013

Very interesting! This has definitely come up before, even the day before yesterday actually: https://twitter.com/axis12345678/status/387053584065757184

I’m a little unclear on how you actually use it, though. And did it make changes to how layout is used in the _data.json?

@echaozh

This comment has been minimized.

Copy link
Contributor Author

echaozh commented Oct 10, 2013

It doesn't change how layout is used in _data.json. You just need to set a value to layout for a layout template. For demo usage, you can take a look at the added test case.

@edrex edrex mentioned this pull request Nov 6, 2013
@edrex

This comment has been minimized.

Copy link
Contributor

edrex commented Nov 16, 2013

Worth noting that Jade already supports nested layouts via extends, block, and prepend/append.

http://www.devthought.com/code/use-jade-blocks-not-layouts/
http://www.learnjade.com/tour/block-append-and-prepend/

@cfjedimaster

This comment has been minimized.

Copy link

cfjedimaster commented Feb 13, 2014

Any reason why this isn't in source yet? (For those of us not using Jade. ;)

@kennethormandy

This comment has been minimized.

Copy link
Collaborator

kennethormandy commented Feb 13, 2014

At least as far as I could determine, it overwrites the default behaviour of layout in a _data.json file. That said, we shouldn’t have left this unresolved, so apologies for that. You can accomplish this now using partial(), but there has definitely been a demand for some kind of nested layouts. We’ll either improve the documentation around an alternate method or take another look at this.

@cfjedimaster

This comment has been minimized.

Copy link

cfjedimaster commented Feb 13, 2014

Fair enough. I'm going to blog an example for EJS tomorrow. I'm assuming
the thought is something like this (pseudo-code):

top level layout:

partial a header
yield
partial a footer

lower layout:
partial the top header
yield
partial the top footer

On Thu, Feb 13, 2014 at 5:08 PM, Kenneth Ormandy
notifications@github.comwrote:

At least as far as I could determine, it overwrites the default behaviour
of layout in a _data.json file. That said, we shouldn't have left this
unresolved, so apologies for that. You can accomplish this now using
partial(), but there has definitely been a demand for some kind of nested
layouts. We'll either improve the documentation around an alternate method
or take another look at this.

Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-35037826
.

Raymond Camden, Web Developer for Adobe

Email : raymondcamden@gmail.com
Blog : www.raymondcamden.com
Twitter: cfjedimaster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.