Skip to content
This repository has been archived by the owner on Apr 30, 2019. It is now read-only.

allow for shared or global model extension when rendering all ingredi… #15

Merged
merged 1 commit into from
Feb 20, 2017

Conversation

lzilioli
Copy link
Contributor

There may be cases where you want to specify global model properties that are always included when rendering a preview, regardless of ingredient.

This change makes that possible.

@lawnsea
Copy link
Contributor

lawnsea commented Jan 9, 2017

Can you explain the motivation for this a bit more, please? I would expect this to be done via model.js in the ingredient.

@lzilioli
Copy link
Contributor Author

lzilioli commented Jan 9, 2017

@lawnsea the motivation is that certain things (icons.svg) remain as something all ingredients expect to have available to their model. It would be redundant/unnecessary to expose the contents of the icons.svg sprite by hand in every ingredient's model.

The stash PR associated with bechamel illustrates how this change is used.

@lzilioli
Copy link
Contributor Author

@lawnsea I still think we need to land this or something like it. The module allows you to override the defaultPreviewTemplatePath with a template owned by the consuming module, but roux-server provides no way to modify the model that is used to render the templates.

It is not very useful to let me override the default-preview unless I also have some level of control over the model that is thrown at that preview template.

@lawnsea
Copy link
Contributor

lawnsea commented Feb 17, 2017

certain things (icons.svg) remain as something all ingredients expect to have available to their model

Just so I'm clear, you mean that they expect them to appear at a particular absolute path in the model, right? Specifically, the templates reference @root.page.svgImagePath.

It is not very useful to let me override the default-preview unless I also have some level of control over the model that is thrown at that preview template.

This is a compelling argument. I'm in favor of landing this if you remove the unnecessary commonModel config and rename globalModel to basePreviewModel.

Copy link
Contributor

@lawnsea lawnsea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok, but I'd like to remove the commonModel code and rename globalModel to basePreviewModel.

* @param {Object} [config.globalModel] - optional model that will be extended
* into the root object before rendering. If your ingredients contain @root
* references, you'll want to specify those things here.
* @param {Object} [config.commonModel] - optional model that will be extended
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this. The ingredients should instead require a common base model in their model.js entrypoint.

@@ -208,6 +218,12 @@ function previewMiddleware(config, req, res, next) {
* is used.
* @param {Object} [config.helpers] - optional map of Handlebars helpers to
* register
* @param {Object} [config.globalModel] - optional model that will be extended
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really a base preview model. Let's call it basePreviewModel.

@lawnsea lawnsea assigned lzilioli and unassigned lawnsea Feb 17, 2017
@lzilioli lzilioli assigned lawnsea and unassigned lzilioli Feb 20, 2017
@lzilioli lzilioli merged commit 76ea996 into master Feb 20, 2017
@lzilioli lzilioli deleted the allow-for-shared-model branch February 20, 2017 17:01
@lzilioli
Copy link
Contributor Author

Published in v1.0.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants