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

Bound handlebars helpers #1274

Merged
merged 1 commit into from Dec 14, 2012

Conversation

Projects
None yet
@ghempton
Member

ghempton commented Aug 15, 2012

This PR introduces Ember.Handlebars.registerBoundHelper.

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Aug 15, 2012

This pull request passes (merged f61455c7 into a2caaa3).

travisbot commented Aug 15, 2012

This pull request passes (merged f61455c7 into a2caaa3).

@pangratz

This comment has been minimized.

Show comment
Hide comment
@pangratz

pangratz Aug 15, 2012

Member

👍 Nice! Definitely useful!!

Member

pangratz commented Aug 15, 2012

👍 Nice! Definitely useful!!

@trek

This comment has been minimized.

Show comment
Hide comment
@trek

trek Aug 15, 2012

Member

👍 I find myself need this often and am not happy with other patterns of getting transformed data into my views

Member

trek commented Aug 15, 2012

👍 I find myself need this often and am not happy with other patterns of getting transformed data into my views

@raycohen

This comment has been minimized.

Show comment
Hide comment
@raycohen

raycohen Aug 15, 2012

Contributor

Looks great. I'd like to see specs that show it works with keywords and in nested contexts.

Contributor

raycohen commented Aug 15, 2012

Looks great. I'd like to see specs that show it works with keywords and in nested contexts.

@wagenet

This comment has been minimized.

Show comment
Hide comment
@wagenet

wagenet Aug 22, 2012

Member

This looks really promising. Will have to review a bit more.

Member

wagenet commented Aug 22, 2012

This looks really promising. Will have to review a bit more.

@krisselden

This comment has been minimized.

Show comment
Hide comment
@krisselden

krisselden Aug 22, 2012

Member

just quickly reviewing the code, it does not look like it would support keywords

Member

krisselden commented Aug 22, 2012

just quickly reviewing the code, it does not look like it would support keywords

@joliss

This comment has been minimized.

Show comment
Hide comment
@joliss

joliss Aug 28, 2012

Contributor

As a workaround until this is merged, you can instantiate views in your helpers to the same effect: http://techblog.fundinggates.com/blog/2012/08/ember-handlebars-helpers-bound-and-unbound/

Contributor

joliss commented Aug 28, 2012

As a workaround until this is merged, you can instantiate views in your helpers to the same effect: http://techblog.fundinggates.com/blog/2012/08/ember-handlebars-helpers-bound-and-unbound/

@ghempton

This comment has been minimized.

Show comment
Hide comment
@ghempton

ghempton Sep 4, 2012

Member

@kselden, @raycohen, I just added support for keywords

Member

ghempton commented Sep 4, 2012

@kselden, @raycohen, I just added support for keywords

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Sep 4, 2012

This pull request passes (merged 505698e4 into 72b0f3d).

travisbot commented Sep 4, 2012

This pull request passes (merged 505698e4 into 72b0f3d).

@ghempton

This comment has been minimized.

Show comment
Hide comment
@ghempton

ghempton Sep 4, 2012

Member

@kselden I think another issue with this approach is a memory leak with the context. Any thoughts on how that could be cleaned up?

Member

ghempton commented Sep 4, 2012

@kselden I think another issue with this approach is a memory leak with the context. Any thoughts on how that could be cleaned up?

@ghempton

This comment has been minimized.

Show comment
Hide comment
@ghempton

ghempton Dec 10, 2012

Member

I just updated this PR to use Ember._SimpleHandlebarsView per a conversation with @wycats and others.

Member

ghempton commented Dec 10, 2012

I just updated this PR to use Ember._SimpleHandlebarsView per a conversation with @wycats and others.

@trek

This comment has been minimized.

Show comment
Hide comment
@trek

trek Dec 10, 2012

Member

ZOMG can we finally get this pulled in?

Member

trek commented Dec 10, 2012

ZOMG can we finally get this pulled in?

@wildchild

This comment has been minimized.

Show comment
Hide comment
@wildchild

wildchild Dec 13, 2012

Any status update on this must-have feature?

wildchild commented Dec 13, 2012

Any status update on this must-have feature?

@ghempton

This comment has been minimized.

Show comment
Hide comment
@ghempton

ghempton Dec 13, 2012

Member

would be good to have @kselden take a look and comment

Member

ghempton commented Dec 13, 2012

would be good to have @kselden take a look and comment

@krisselden

This comment has been minimized.

Show comment
Hide comment
@krisselden

krisselden Dec 14, 2012

Member

The dependentKeys feature isn't tested by any of the tests, otherwise looks good.

Member

krisselden commented Dec 14, 2012

The dependentKeys feature isn't tested by any of the tests, otherwise looks good.

@krisselden

This comment has been minimized.

Show comment
Hide comment
@krisselden

krisselden Dec 14, 2012

Member

I was wrong, it is tested, just not in a way that makes the use case very clear.

Member

krisselden commented Dec 14, 2012

I was wrong, it is tested, just not in a way that makes the use case very clear.

krisselden added a commit that referenced this pull request Dec 14, 2012

@krisselden krisselden merged commit 4646983 into emberjs:master Dec 14, 2012

1 check passed

default The Travis build passed
Details
@kurko

This comment has been minimized.

Show comment
Hide comment
@kurko

kurko commented Dec 14, 2012

@bestie

This comment has been minimized.

Show comment
Hide comment
@bestie

bestie Dec 14, 2012

Contributor

👍

Contributor

bestie commented Dec 14, 2012

👍

@panayi

This comment has been minimized.

Show comment
Hide comment
@panayi

panayi commented Dec 14, 2012

orsonclapping

@machty

This comment has been minimized.

Show comment
Hide comment
@machty

machty Dec 14, 2012

Member

Yessssss <3

Member

machty commented Dec 14, 2012

Yessssss <3

@kmdsbng

This comment has been minimized.

Show comment
Hide comment
@kmdsbng

kmdsbng commented Dec 17, 2012

Great!

@dmzza

This comment has been minimized.

Show comment
Hide comment
@dmzza

dmzza Dec 17, 2012

Contributor

👍

Contributor

dmzza commented Dec 17, 2012

👍

@chrisdugne

This comment has been minimized.

Show comment
Hide comment
@chrisdugne

chrisdugne Dec 20, 2012

excellent !!

chrisdugne commented Dec 20, 2012

excellent !!

@josepjaume

This comment has been minimized.

Show comment
Hide comment
@josepjaume

josepjaume Jan 17, 2013

Contributor

I'm trying to create a I18n helper with the following syntax:

{{t 'hello.world' nameBinding="name"}}

Am I wrong or this helper wouldn't keep track of changes in "name"? I know setting a property in the view bound to "name" would be pretty straightforward, but shouldn't this use case be considered in registerBoundHelper?

Contributor

josepjaume commented Jan 17, 2013

I'm trying to create a I18n helper with the following syntax:

{{t 'hello.world' nameBinding="name"}}

Am I wrong or this helper wouldn't keep track of changes in "name"? I know setting a property in the view bound to "name" would be pretty straightforward, but shouldn't this use case be considered in registerBoundHelper?

@machty

This comment has been minimized.

Show comment
Hide comment
@machty

machty Jan 17, 2013

Member

@josepjaume I'm going to try and address this for #1768

Member

machty commented Jan 17, 2013

@josepjaume I'm going to try and address this for #1768

@ZenCocoon

This comment has been minimized.

Show comment
Hide comment
@ZenCocoon

ZenCocoon Apr 14, 2013

Contributor

toUpperCase() is not the same as capitalize actually. And in this case, you could use directly capitalize() from http://emberjs.com/api/classes/Ember.String.html#method_capitalize

Am I missing something?

Contributor

ZenCocoon commented on packages/ember-handlebars/lib/ext.js in 91869b1 Apr 14, 2013

toUpperCase() is not the same as capitalize actually. And in this case, you could use directly capitalize() from http://emberjs.com/api/classes/Ember.String.html#method_capitalize

Am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment