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

Expose {{concat}} helper publicly. #11264

Merged
merged 1 commit into from
Jun 6, 2015
Merged

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented May 24, 2015

No description provided.

@mmun
Copy link
Member

mmun commented May 24, 2015

Seems bad to call it concat since that's not what it is in JS... It should be join if anything. Arguably it would take an array instead of positional args. What's the point of making this public?

@rwjblue
Copy link
Member Author

rwjblue commented May 24, 2015

Seems bad to call it concat since that's not what it is in JS

I can remove the separator (which would make it a proper concat instead of a join + separator).

What's the point of making this public?

It is a very commonly needed helper, that many folks end up writing themselves. With angle-bracket component invocation this is much less needed, but until angle-bracket invocation is ready to go I would like to bake this in.

One specific example:

<label for="{{elementId}}-input">{{label}}</label>
{{input id=(concat elementId '-input') value=value}}

Another example #11263:

{{! this is deprecated }}
{{foo-bar classBinding=":truthy isStuff"}}

{{! this is how we silence the deprecation internally }}
{{foo-bar class=(-concat "truthy" (if isStuff "is-stuff") separator=" ")}}

As far as I can tell, it is not possible for users to satisfy this deprecation notice (I actually think we should disable the deprecation until angle bracket invocation is a bit better) without either writing your own concat helper or using the private one.

@mmun
Copy link
Member

mmun commented May 24, 2015

I'd rather write (add elementId '-input') in the first case, which is what you'd write in JS. concat is fine though.

For the second case, assuming we don't deal with the deprecation another way, I'd rather have a classes helper that deals with spaces nicely. An Interpolation helper would be fine too (interpolate "truthy ${}" (if isStuff "is-stuff")). (fmt might be better).

@rwjblue rwjblue force-pushed the add-concat-helper branch 2 times, most recently from 4bfa772 to 7631324 Compare May 24, 2015 19:46
@rwjblue
Copy link
Member Author

rwjblue commented May 24, 2015

Updated to remove separator.

@rwjblue rwjblue force-pushed the add-concat-helper branch from 7631324 to 9e8fd04 Compare May 29, 2015 22:25
@rwjblue
Copy link
Member Author

rwjblue commented May 29, 2015

rebased

rwjblue added a commit that referenced this pull request Jun 6, 2015
Expose `{{concat}}` helper publicly.
@rwjblue rwjblue merged commit 55f02b1 into emberjs:master Jun 6, 2015
@rwjblue rwjblue deleted the add-concat-helper branch June 6, 2015 22:09
@mmun
Copy link
Member

mmun commented Jun 6, 2015

👍

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

Successfully merging this pull request may close these issues.

2 participants