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

consider adding templating #357

Closed
gkatsev opened this issue Mar 4, 2013 · 5 comments
Closed

consider adding templating #357

gkatsev opened this issue Mar 4, 2013 · 5 comments

Comments

@gkatsev
Copy link
Member

gkatsev commented Mar 4, 2013

Seems like there is a lot of string manipulation going on in the creation of the controls. Would be nicer if we were to use a templating system.
Even one as simple as:

function template(str, arrSubs){
  var i, l;
  i = 0;
  l = arrSubs.length;
  for (; i < l; i++) {
    str = str.replace("{"+i+"}", arrSubs[i]);
  }
  return str;
}
template('foo {0} {1} {2}', [1,2,3]);
// "foo 1 2 3"

For example, we currently have the same string in the content and updateContent of the durationDisplay.
Instead, each component, could have a template associated with it that can be changed.

@heff
Copy link
Member

heff commented Mar 5, 2013

Yeah, the current way definitely isn't the most optimal. In this specific case it might be cleaner to just build a span for the time specifically and reference the innerHTML of that instead of overwriting the whole content el.

On Mar 4, 2013, at 3:34 PM, Gary Katsevman notifications@github.com wrote:

Seems like there is a lot of string manipulation going on in the creation of the controls. Would be nicer if we were to use a templating system.
Even one as simple as:

function template(str, arrSubs){
var i, l;
i = 0;
l = arrSubs.length;
for (; i < l; i++) {
str = str.replace("{"+i+"}", arrSubs[i]);
}
return str;
}
template('foo {0} {1} {2}', [1,2,3]);
// "foo 1 2 3"
For example, we currently have the same string in the content and updateContent of the durationDisplay.
Instead, each component, could have a template associated with it that can be changed.


Reply to this email directly or view it on GitHub.

@heff
Copy link
Member

heff commented Mar 1, 2014

To clarify this more, every component has a contentEl property that returns the el() by default, but you can have them be different, for instance if you want children to be embedded in a sub-element of the component's main element.

In this specific case we could set up the span as a contentEl, so we don't have to create a new element each time.

But there still use for templates other places. If we can decide on how to pull in lodash we could make use of those templates.

This could probably be broken up into two feature issue, but confirming this either way.

@mmcc
Copy link
Member

mmcc commented Feb 26, 2015

Just to revisit this one, if we're going to use an ES6 transpiler (#1812), this is somewhat moot. Even something as simple as string substition would improve things.

@mmcc
Copy link
Member

mmcc commented Apr 23, 2015

I think we can consider this closed now that we have things like #2015 pulled in.

@mmcc mmcc closed this as completed Apr 23, 2015
@gkatsev
Copy link
Member Author

gkatsev commented Apr 23, 2015

There is probably some extra stuff we can do to increase DRY, but yeah, we did get a lot of stuff for this via template strings.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants