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

Added $element pseudo-variable that allows accessing the current element #474

Merged
merged 3 commits into from Sep 28, 2012

Conversation

@mbest
Copy link
Member

@mbest mbest commented Jul 13, 2012

Added $element pseudo-variable that allows accessing the current element (and through it the DOM tree).

@mbest
Copy link
Member

@mbest mbest commented Jul 13, 2012

I've updated this pull request to base it on the latest code. I also added support for re-written templates and simplified the code a bit.

The original commit is here: studgeek/knockout@271d5b8

@studgeek
Copy link
Contributor Author

@studgeek studgeek commented Jul 13, 2012

cool :)

…ontext value. Simplify bindings string evaluator.
@@ -30,7 +30,7 @@ ko.templateRewriting = (function () {
// anonymous function, even though Opera's built-in debugger can evaluate it anyway. No other browser requires this
// extra indirection.
var applyBindingsToNextSiblingScript =
"ko.__tr_ambtns(function(){return(function(){return{" + rewrittenDataBindAttributeValue + "} })()})";
"ko.__tr_ambtns(function($context,$element){return(function(){return{ " + rewrittenDataBindAttributeValue + " } })()})";

This comment has been minimized.

@mbest

mbest Sep 3, 2012
Author Member

This includes a fix for #615.

@SteveSanderson

This comment has been minimized.

Copy link
Contributor

@SteveSanderson SteveSanderson commented on spec/templatingBehaviors.js in be8564f Sep 23, 2012

What about $context? Should there be a corresponding spec for that too?

This comment has been minimized.

Copy link
Member Author

@mbest mbest replied Sep 24, 2012

Yeah. I can add those.

@SteveSanderson
Copy link
Contributor

@SteveSanderson SteveSanderson commented Sep 23, 2012

Generally I'm pretty happy with putting this in. I really like how you simplified the binding string parsing and eliminated the unnecessarily general buildEvalWithinScopeFunction function.

Could you explain, though, what the use case for having $element and $context in binding strings is? It's not clear to me because typically the only nontrivial code you'd reference from a binding string would be a custom binding, and a custom binding already has access to both of those pieces of information. So when you you use these new pseudovariables?

@mbest
Copy link
Member

@mbest mbest commented Sep 24, 2012

Could you explain, though, what the use case for having $element and $context in binding strings is?

Regarding $element:

I've seen cases where users want to use an attribute of the element in the binding, something like this: <div id="item1" data-bind="visible: isCurrent($element.id)">...</div>.

For myself, while working on my knockout-freedom plugin, I realized that I couldn't create computed observables in a binding string because, without access to the element, I couldn't set up the disposal handler. I ended up using a "psuedo-computed" that calls a function like computed, but doesn't maintain its own subscriptions.

Regarding $context:

I don't have a specific use case in mind, but I thought that since the context object is being passed into the binding evaluator function as a parameter anyway, why not give it a meaningful name? One use-case might be to test the presence of a context variable or to access it even if it hasn't been defined, like this: data-bind="text: $context.$parentContext && $parentContext.$index".

@studgeek
Copy link
Contributor Author

@studgeek studgeek commented Sep 24, 2012

That was similar to why I wrote the $element in the first place. You can also use it to get to data that is hung on the object or use it to get to the jQuery wrapper.

@SteveSanderson SteveSanderson merged commit 5d5839b into master Sep 28, 2012
@SteveSanderson
Copy link
Contributor

@SteveSanderson SteveSanderson commented Sep 28, 2012

Thanks for the spec and explanations. Merged.

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

Successfully merging this pull request may close these issues.

None yet

3 participants