fix(toolbar): add support in scrollshrink to ngshow/hide #5863
Conversation
`updateToolbarHeight` was not been called when `ngShow` or `ngHide` value has change which caused the toolbar to wait for the debounce to call `updateToolbarHeight` (5 sec) fixes #5706
| // If the toolbar has ngShow or ngHide we need to update height immediately as it changed | ||
| // and not wait for $mdUtil.debounce to happen | ||
|
|
||
| if (attr.ngShow) { scope.$watch(attr.ngShow, updateToolbarHeight); } |
There was a problem hiding this comment.
@EladBezalel - why use scope.$watch() instead of attr.$observer()?
There was a problem hiding this comment.
Initially it was $observe but than topher came with this jem:
a quick excerpt from here regarding $observe verses $watch and why we need $watch here: http://stackoverflow.com/questions/14876112/angularjs-difference-between-the-observe-and-watch-methods
(If you try attrs.$observe('attr1') you'll get the string myModel.some_prop, which is probably not what you want.)
The $observe was returning the string ‘shouldShowToolbar’, not the value of that expression. And it was only firing once because the expression itself didn’t change.
There was a problem hiding this comment.
As a note, I'm pretty sure the attr.$obseve('mdScrollShrink', onChangeScrollShrink); line above is also broken in the same way but I didn't get a chance to test.
Also, I highly doubt that people will be dynamically changing the scrollShrink attribute of the toolbar, so it's a VERY low priority to fix :-)
ThomasBurleson
added
in progress
ThomasBurleson
removed
the
in progress
updateToolbarHeightwas not been called whenngShoworngHidevalue has change which caused the toolbar to wait for the debounce to callupdateToolbarHeight(5 sec)KUDOS to @topherfangio for helping (and solving) me on this!
fixes #5706