-
Notifications
You must be signed in to change notification settings - Fork 541
feat(templatesConfig): helpers and options transferred to Template #136
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
Conversation
ae02d82
to
f561d08
Compare
I believe it's Also we should use If you agree, update :D |
Do you mean |
@vvo Fixed both. |
var content; | ||
|
||
if (template === null || typeof template === 'undefined') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typeof template === 'undefined'
is only needed when the variable may not have been defined in the first place. Which is not the case here because it is a parameter of the function. I would suggest that instead :
template === undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the edge case here (people redefining undefined
global var). But those websites have other issues that we do not want to fix here.
But we are not template engine agnostic. We use hogan and thus should not hide it (and we do not right now). I think there are little chances that we switch to another template engine. And if we do then it would be a major breakage otherwise it means there's no added value to the change. It will be easier to grasp for people what are theses options and to what they map (the hogan documentation). The only place where we would benefit from another templating engine would be to use something more featureful than hogan and such we would break compat. |
@@ -2,6 +2,7 @@ | |||
<label> | |||
<input type="checkbox" {{#isRefined}}checked{{/isRefined}} /> | |||
{{label}} | |||
{{_refined}}!{{/refined}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where do _refined comes from and is it different from "isRefined"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just to illustrate that the options worked, and I didn't want to do it with the delimiters
options, because it would have meant changing all delimiters in example/templates/*. It's no different from isRefined
, the only difference is that it's using the section tags of Hogan
(you can see there is no #
in the opening tag).
We can remove it.
I am worried about the "defaultTemplate" being passed everywhere. Because it adds some good code to solve the "For some widgets I want to use my options, for others I want to use my options" usecase. I do not have a solution for this thought right now. |
Îf we really want to state that we're using hoganConfig = {
helpers,
compileOptions
} |
About the |
New parameter passed to the `init` and `render` method: `templatesConfig`. It replaces `templateHelpers`. This parameter embeds both the `helpers` and the `Hogan` options. It uses the same chain that the `templateHelpers` previously used to be passed from a widget to the `Template` component. The `Template` component now also accepts a `templateDefault` parameter which allows us to ignore `Hogan`'s `options` if there is no `template` provided Closes #99.
f561d08
to
456d781
Compare
8d433c0
to
40fe8ad
Compare
+ using a shared behavior to handle all template* instantsearch instance config dependencies + renamed templatesConfig.options to templatesConfig.compileOptions + added `templates` (header, body, footer) to `stats` BREAKING CHANGE + jsDoc for `stats`
40fe8ad
to
3c28aac
Compare
feat(templatesConfig): helpers and options transferred to Template
The main goal of this PR is to let the
Template
component handle the logic related to itself instead of its parents.New parameter passed to the
init
andrender
method:templatesConfig
.It replaces
templateHelpers
and is defined inInstantsearch.js
:To define templating options, you only need to use:
or
This parameter embeds both the
helpers
and theHogan
options.It uses the same chain that the
templateHelpers
previously used to be passed from a widget to theTemplate
component.The
Template
component now also accepts atemplateDefault
parameter which allows us to ignore the templates options onHogan.compile
calls if there is notemplate
provided.The
template
prop is not required anymore down the chain, butrenderTemplate
will throw an error if both the default template and the passed templated arenull
orundefined
(but in case we want to display nothing, the empty string works).Closes #99.
Alternatives:
context
feature ofReact
to avoid passing the config down the chain, and only declare it as a context parameter on the firstrender
call.