Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Can't set css props while using {{}}-expression within style-attribute #12763

Closed
dhilt opened this issue Sep 4, 2015 · 10 comments
Closed

Can't set css props while using {{}}-expression within style-attribute #12763

dhilt opened this issue Sep 4, 2015 · 10 comments

Comments

@dhilt
Copy link

dhilt commented Sep 4, 2015

Greetings! I've made a little repro: http://jsfiddle.net/dhilt/8tceyw2w/3/ on Angular 1.4.4.

My problem is that I can't set css property via .css() while I'm playing with visibility of the element like this:

<div class="myElement" style="visibility:{{visible ? 'visible' : 'hidden' }}">

And there is no problem when I switch to classes-playing:

<div class="myElement" class="myElement {{visible ? 'visible' : 'hidden'}}">
.visible { visibility: visible; }
.hidden { visibility: hidden; }

So we have two similar situations with two different behaviour. In the demo (http://jsfiddle.net/dhilt/8tceyw2w/3/) I try to set some property syncronous for both cases right after the visibility is switched, and my expectation is that the values of this property of both elements must be the same. And they are not the same.

Please advise!

@m-amr
Copy link
Contributor

m-amr commented Sep 5, 2015

@dhilt you are trying to set element css in two place
one in html

<div class="myElement" style="visibility:{{visible ? 'visible' : 'hidden' }}"></div>

and second one in javascript
var changeProperty = function (element) {
if(element[0].style.overflowY === 'auto')
element.css({'overflow-y': 'inherit'});
else
element.css({'overflow-y': 'auto'});
};

The problem is in the second element because you are setting it's style. i think what happen is that
when you set it css property using javascript element.css() function
angular overrides that when it run the digestive cycle
because it compute the expression in the html style property style="visibility:{{visible ? 'visible' : 'hidden' }}"> and remove the 'overflow-y': 'auto' that you set in the javascript
if you try to log element[0].style.overflowY it will always return nothing so it will enter the else state
and set to element.css({'overflow-y': 'auto'}); at all the times

Do you understand ?

@dhilt
Copy link
Author

dhilt commented Sep 6, 2015

when you set it css property using javascript element.css() function angular overrides that when it run the digestive cycle

@m-amr But don't you think it is a bug? cause in my demo angular-digest has to deal only with 'visibility' property while .css() deals with 'overflow-y' property; looks like a strange collision; these two settings have to be independent, isn't it?

@gkalpak
Copy link
Member

gkalpak commented Sep 7, 2015

I think might be related to/the same as #9109.
I believe both the interpolation and .css() are trying to set the value of the style attribute, effectively overwriting each other.

My speculation from #9109 (comment) (point D) seems valid:

Basically, I believe that this problem is likely to arise any time there is an attribute whose value is an interpolated string and someone (a directive) also changes the value of that attribute (not replacing it, but modifying it).

Unfortunately, I can't think of a good solution 😞

@dhilt
Copy link
Author

dhilt commented Sep 7, 2015

@gkalpak Your investigation is realy good! but your suggestion

Generally, I think attribute interpolation should check (how?) that there has been no modification to the original template string (based on which the new value has been computed). If the original template string has been modified, then the computed value is useless and interpolation should rerun.

I guess could be a bit irrelevant to situation discussed here. Cause by my demo we can obtain the next sequence:

  1. changing property on $scope ($scope.visible) through ng-model binding
  2. triggering watcher on this property ($scope.$watch('visible'))
  3. get html-node attribute data (element[0].style.overflowY) within this watcher
  4. set a new value of this attr via .css() within the watcher based on (3) result

And the problem is that on (3) we get wrong value, we always get "" (as it was shown by @m-amr).
Despite the next setting (4) is incorrect cause it based on the result of (3), its value becomes non-empty.
But during the next round of $scope.visible manual changing we get an empty value on (3) again.

So no modifications to the original template string have been made from (1) to (3), no .css() execution, but on (3) we see wrong data. May be I should change the title cause .css() works properly as I see now...

@dhilt
Copy link
Author

dhilt commented Sep 7, 2015

I've updated the issue demo: http://jsfiddle.net/dhilt/8tceyw2w/5/. It became simplier.
It demonstrates that {{}}-expression within style-attribute breaks other inline-style data.

<div id="myElement" style="overflowX: auto; visibility:{{visible ? 'visible' : 'hidden' }}">
document.getElementById('myElement').style.overflowY = 'auto';
  1. Before $scope.visible changes, {{}}-expression breaks inlined overflowX property but breaks not overflowY property which was set within controller code.
  2. After $scope.visible changes, {{}}-expression breaks overflowY property too.

@dhilt dhilt changed the title Can't set css property via .css() in some special case Can't set css props while using {{}}-expression within class-attribute Sep 7, 2015
@gkalpak
Copy link
Member

gkalpak commented Sep 7, 2015

@dhilt, here is what is going on:

  1. #myElement has style="overflow-x: auto; visibility:{{visible ? 'visible' : 'hidden' }}".
  2. The (implicit) attribute-interpolation directive, sees that there is an interpolation in the value of the style attribute and says: Let me replace the value of the style attribute with the result of the evaluation of overflow-x: auto; visibility:{{visible ? 'visible' : 'hidden' }}.
    But it doesn't replace it right away.
  3. The controller is instantiated and sets myElementStyle.overflowY = 'auto'.
    At this point, both overflowX and overflowY are set to auto.
    (The value of style attribute is changed to reflect that. It may be either overflow-x: auto; visibility:{{visible ? 'visible' : 'hidden' }}; overflow-y: auto or overflow: auto; visibility:{{visible ? 'visible' : 'hidden' }}, based on the browser; but that's unimportant.)
  4. The watchAction (of $scope.$watch('visible', ...)) is triggered and stores the values of overflowX/Y to myElementProp1/2 respectively.
    At this point, both are still set to auto.
  5. Later on, the interpolation directive deterines that overflow-x: auto; visibility:{{visible ? 'visible' : 'hidden' }} (with visible === true) gets evaluated to overflow-x: auto; visibility: visible and sets the value of the style attribute to that.
    Now, myElementStyle.overflowY is no longer set to 'auto'.

BTW, I think that what I proposed as a possible solution in #9109 (comment) won't actually work, but I believe the analysis of the problem is still valid.

@dhilt
Copy link
Author

dhilt commented Sep 7, 2015

@gkalpak I can't agree with p.4:

a) when watchAction is triggered on app start (without manual changing of $scope.visible) I see that overflowX is "" while overflowY is "auto"; it is so when I come into angular $digest() for the first time

b) when watchAction is triggered by $scope.visible manual changing (and I come into the body of angular $digest() for the second time) both of them are empty

@gkalpak
Copy link
Member

gkalpak commented Sep 7, 2015

@dhilt, it is probably because in your latest example you have a style="overflowX: auto; ..." instead of style="overflow-x: auto; ...".

Updated fiddle

@dhilt
Copy link
Author

dhilt commented Sep 7, 2015

@gkalpak got it, thanks) thus we have a contradiction between dom-node attr internal recalculation when we are using {{}}-expression and dom-node attr external modifying (for example .css() style modifying). This problem is buried under a bunch of comments (on my #12763 and your #9109 issues I mean), may be it would be better to open another issue and close this one to increase the probability that somebody will fix it?

@gkalpak
Copy link
Member

gkalpak commented Sep 8, 2015

@dhilt, sure, feel free to close this one and open a new "more-to-the-actual-point" issue (with links to comments you think provide useful context).
Thx !

@dhilt dhilt closed this as completed Sep 10, 2015
@dhilt dhilt changed the title Can't set css props while using {{}}-expression within class-attribute Can't set css props while using {{}}-expression within style-attribute Sep 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants