fix(ngSrc, ngSrcset): only interpolate if all expressions are defined #7290
Conversation
|
✨ wow ✨ |
|
Ping @IgorMinar @caitp: thoughts? |
I am not too keen on this, a small api change to $interpolate would be easier, I think. Like a 3rd (2nd?) parameter to the returned function that says "allow undefined expression results" or something. Having multiple implementations of interpolation seems like a bad idea to me (IMO) |
|
Right; it's probably useful to expose this behavior on I started reimplementing this feature like @caitp described. See: |
|
@tbosch Can you take a look at this? I want to merge it soon~ |
|
I resolved with @tbosch to expose this behavior to |
|
@IgorMinar @caitp I significantly refactored my implementation and improved tests and docs. I still don't have a good name for this new feature. I'm tentatively calling it Can you please take a look again when you have a minute? Thanks! |
|
It still LGTM, although I'm not totally sure setAllOrNothing() is a great API. I would personally prefer if we had some other notation for this that authors were in control of. But, I'm not sure what that would look like, and it would probably complicate things. So it works for me. |
|
well in that case separators.join('') === '', so no update will happen. that's the desired behavior, no? |
|
Arg. bad example. Consider |
|
@IgorMinar updated with the changes we discussed |
btford
changed the title
| @@ -1856,7 +1863,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |||
|
|
|||
| // we need to interpolate again, in case the attribute value has been updated | |||
| // (e.g. by another directive's compile function) | |||
| interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name)); | |||
| interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name), | |||
| allOrNothingAttrs.indexOf(name) > -1); | |||
There was a problem hiding this comment.
instead of indexOf you could make allOrNothingAttrs a map of boolean values. I think we even have a helper method that can turn an array into such map. then you can simply do allOrNothingAttrs[name] and you are done.
There was a problem hiding this comment.
👍 any idea what the helper is called/where it lives?
|
otherwise this looks good |
|
Made that change; can I land this? |
BREAKING CHANGE
If `bar` is `undefined`, before `<img src="foo/{{bar}}.jpg">` yields
`<img src="foo/.jpg">`. With this change, the binding will not set `src`.
If you want the old behavior, you can do this: `<img src="foo/{{bar || ''}}.jpg">`.
The same applies for `srcset` as well.
Closes angular#6984
|
Landed as 8d18038. |
BREAKING CHANGE
If
barisundefined, before<img src="foo/{{bar}}.jpg">yields<img src="foo/.jpg">. With this change, the binding will not setsrc.If you want the old behavior, you can do this:
<img src="foo/{{bar || ''}}.jpg">.The same applies for
srcsetandhref.Closes #6984