GNOME Bugzilla – Bug 690643
Glitch when the dash expands
Last modified: 2013-02-20 19:52:41 UTC
How to reproduce: start removing favorites or closing applications until the dash jumps to the next size.
Created attachment 232113 [details] [review] dash: turn DashItemContainer into a proper St.Widget This removes a number of unneeded ._delegate accesses and cleans up the code.
Created attachment 232114 [details] [review] dash: fix allocation loop when increasing icon size The DashActor will known to allocate the show apps button only if the icon size is (temporarily) too big for the containing box, therefore it should request just that as the minimum size. This solves a glitch that happened when removing a favorite and at the same time causing the dash to expand.
Created attachment 232116 [details] [review] dash: fix icon animation animateIn and animateOut should not reset the scale, otherwise extra animate calls (which are possible because the diff algorithm in _redisplay is not optimal) cause unneeded movement. Therefore, create new items hidden, and have the creator call animateIn or set the scale/opacity properties manually. adjustIconSize must be changed to always set the icon size temporarily, otherwise the first time it computes the icon size with 0 scale.
Review of attachment 232116 [details] [review]: ::: js/ui/dash.js @@ +604,1 @@ + let firstIcon = iconChildren[0].child._delegate.icon; bad rebase @@ +607,3 @@ + // Enforce the current icon size during the size request + { bad @@ +639,3 @@ let scale = oldIconSize / newIconSize; for (let i = 0; i < iconChildren.length; i++) { + let icon = iconChildren[i].child._delegate.icon; bad rebase
Created attachment 232175 [details] [review] dash: turn DashItemContainer into a proper St.Widget This removes a number of unneeded ._delegate accesses and cleans up the code.
Created attachment 232176 [details] [review] dash: fix icon animation animateIn and animateOut should not reset the scale, otherwise extra animate calls (which are possible because the diff algorithm in _redisplay is not optimal) cause unneeded movement. Therefore, create new items hidden, and have the creator call animateIn or set the scale/opacity properties manually. adjustIconSize must be changed to always set the icon size temporarily, otherwise the first time it computes the icon size with 0 scale. The second one was not bad, actually.
Review of attachment 232176 [details] [review]: ::: js/ui/dash.js @@ +607,3 @@ + // Enforce the current icon size during the size request + { We do not use JS blocks anywhere else in shell code, and I don't see a reason to start now.
Created attachment 233822 [details] [review] dash: fix icon animation animateIn and animateOut should not reset the scale, otherwise extra animate calls (which are possible because the diff algorithm in _redisplay is not optimal) cause unneeded movement. Therefore, create new items hidden, and have the creator call animateIn or set the scale/opacity properties manually. adjustIconSize must be changed to always set the icon size temporarily, otherwise the first time it computes the icon size with 0 scale.
See bug 692744 for another layout issue with the Dash, which I believe lies in Clutter.
Created attachment 234669 [details] [review] dash: add a workaround for Clutter bug 692744
*** Bug 693571 has been marked as a duplicate of this bug. ***
Review of attachment 232175 [details] [review]: OK.
Review of attachment 233822 [details] [review]: ::: js/ui/dash.js @@ +161,3 @@ this.add_actor(this.child); + + this.child.set_scale_with_gravity(this._childScale, this._childScale, Gravity is deprecated. Set the pivot point? @@ +788,3 @@ + for (let i = 0; i < addedItems.length; i++) { + if (animate) { + addedItems[i].item.animateIn(); addedItems[i].item.show(animate); maybe?
Review of attachment 234669 [details] [review]: OK.
(In reply to comment #13) > Review of attachment 233822 [details] [review]: > > ::: js/ui/dash.js > @@ +161,3 @@ > this.add_actor(this.child); > + > + this.child.set_scale_with_gravity(this._childScale, this._childScale, > > Gravity is deprecated. Set the pivot point? I just copypasted the code from the childScale property setter. If you want to remove the deprecated call, I'll do with a separate patch.
OK.
Created attachment 236957 [details] [review] dash: fix icon animation animateIn and animateOut should not reset the scale, otherwise extra animate calls (which are possible because the diff algorithm in _redisplay is not optimal) cause unneeded movement. Therefore, create new items hidden, and have the creator call animateIn or set the scale/opacity properties manually. adjustIconSize must be changed to always set the icon size temporarily, otherwise the first time it computes the icon size with 0 scale.
Review of attachment 236957 [details] [review]: I like this a lot better, yeah.
Created attachment 236960 [details] [review] dash: fix allocation loop when increasing icon size The DashActor will known to allocate the show apps button only if the icon size is (temporarily) too big for the containing box, therefore it should request just that as the minimum size. This solves a glitch that happened when removing a favorite and at the same time causing the dash to expand.
Review of attachment 236960 [details] [review]: The comment makes this much clearer.
Attachment 232175 [details] pushed as ef1e279 - dash: turn DashItemContainer into a proper St.Widget Attachment 234669 [details] pushed as 8bcb103 - dash: add a workaround for Clutter bug 692744 Attachment 236957 [details] pushed as 8e7d74b - dash: fix icon animation Attachment 236960 [details] pushed as 629b6fa - dash: fix allocation loop when increasing icon size