alphagov / govuk_template

❗️GOV.UK Template is deprecated, and will only receive major bug fixes and security patches. A template containing the GOV.UK header and footer, and associated assets.
https://alphagov.github.io/govuk_template/
MIT License
112 stars 67 forks source link

Rendering pages requires multiple render steps (mustache) #84

Closed michaeldfallen closed 10 years ago

michaeldfallen commented 10 years ago

Currently to render a page I first need to render any content I want to place into the content areas (htmlLang, topOfPage, pageTitle, assetPath, head, bodyClasses, headerClass, insideHeader, propositionHeader, afterHeader, cookieMessage, footerTop, footerSupportLinks, bodyEndContent, content).

12 of those allow html to be rendered, using {{{ to render literal raw html. So in the "best worst" case, where we only render our content for each of the 12 raw html variables once then render the template with those html blocks, we end up doing 13 mustache render passes. On top of this the rendering is nested, render these blocks, then render them into the template, so we need to control our render code and have multiple mustache models.

To fix this I'd propose splitting the template into partials around the {{{content}}} block, two partials Header and Footer. This allows us to import the partials above and below our content inside our template. Also we only need one mustache model, so long as we provide content for the different parameters listed above. This only reduces the number of render passes slightly but it makes the render process much simpler.

Thoughts?

edds commented 10 years ago

The mustache inheritance version gets around this and allows you to only have to call render once.

It allows you to specify all of the content for the content areas in a mustache file which you can pass your own variables to and then let the template call the super (govuk_template) mustache file. It means you only need to call mustache render once. You can see an example of how that works here: https://gist.github.com/spullara/1854699

michaeldfallen commented 10 years ago

Interesting, though that looks like it's only a proposal. Is it implemented anywhere?

michaeldfallen commented 10 years ago

Looking through the spec actually it seems lambdas might be the solution: https://github.com/mustache/spec/blob/master/specs/~lambdas.yml#L28

I think that spec means that:

template:

{{foo}}

model:

{
  "foo" : "<h1>{{bar}}</h1>",
  "bar" : "baz"
}

should result in html:

<h1>baz</h1>
edds commented 10 years ago

Is it implemented anywhere?

It is supported in both the mustache.java and the hogan.js implementations. It is how we were including the template on an exemplar I was working on.

michaeldfallen commented 10 years ago

Could you link me to an example? Sounds like a really good way to deal with this issue and simplify our rendering.

edds commented 10 years ago

Thinking about it, it is also how @tombye's prototype thing works:

govuk_template template: https://github.com/tombye/express_prototype/blob/master/govuk/views/govuk_template.html Layout which defers to govuk_template: https://github.com/tombye/express_prototype/blob/master/views/layout.html View which defers to the main layout: https://github.com/tombye/express_prototype/blob/master/views/sample.html

michaeldfallen commented 10 years ago

Nice, I'm switching us to this approach now. Surprisingly easy to implement, only took an hour to get it up to scratch with what we previously had.