GNOME Bugzilla – Bug 682286
Overview as BoxLayouts
Last modified: 2012-12-13 15:27:30 UTC
Avoid a bunch of manual calculations and use St.BoxLayouts to manage the layout of the overview.
Created attachment 221869 [details] [review] overview: overview as boxlayouts
Comment on attachment 221869 [details] [review] overview: overview as boxlayouts 1. The window picker should not move around when the dash item size changes (at least this used to be a design constraint, I assume it still applies) 2. overview transition!
(In reply to comment #2) > (From update of attachment 221869 [details] [review]) > 1. The window picker should not move around when the dash item size changes (at > least this used to be a design constraint, I assume it still applies) I'm not familiar with the history, but I'm not entirely sure about this. We already dynamically adjust the window picker when the workspace switcher slides in and out, and we should be trying to give as much space to the window thumbnails as possible. Allocating a fixed size to the dash can also create some odd layouts, with the window thumbnails seemingly being pulled off to one side. Or am I missing something? ;)
Created attachment 221944 [details] [review] overview: overview as boxlayouts
Created attachment 221945 [details] [review] workspace: Don't recalculate window positions when leaving the overview This fixes the issue of the leaving the overview transition. Investigating the first transition to the overview, it's because we now may get spurious allocations and we just have to deal with it. I don't know what causes the multiple allocations of the uiGroup when animating to the overview, but we need to just deal with it. The existing code just animates all clones with the assumption that they'll be in the original states when they start out, so what we end up with is that the clones are being animated twice. On the second time, they're being pulled from their partially animated states, so you see them appear and thrust from stage left into their positions.
*** Bug 688501 has been marked as a duplicate of this bug. ***
Created attachment 231265 [details] [review] overview: overview as boxlayouts -- This patch is still needed for future developments of the search relayout branch. Attached is a new version that works fine with no allocation regressions in my testing.
Comment on attachment 231265 [details] [review] overview: overview as boxlayouts I was being too optimistic. This actually has some regressions.
Created attachment 231306 [details] [review] overview: overview as boxlayouts -- This one seems to work fine; it was a matter of finding the right combination of fill/expand StBoxLayout flags.
Review of attachment 231306 [details] [review]: ::: js/ui/overview.js @@ +142,3 @@ + this._overview._delegate = this; + + this._spacing = 0; this._spacing is gotten and updated but never used.
(In reply to comment #10) > Review of attachment 231306 [details] [review]: > > ::: js/ui/overview.js > @@ +142,3 @@ > + this._overview._delegate = this; > + > + this._spacing = 0; > > this._spacing is gotten and updated but never used. No, it's still used in js/ui/workspacesView.js.
(In reply to comment #11) > No, it's still used in js/ui/workspacesView.js. It shouldn't be, is the thing. You should be able to set it to 0 without losing anything.
Created attachment 231481 [details] [review] overview: overview as boxlayouts -- You're right, this isn't needed anymore.
Review of attachment 231481 [details] [review]: OK. Let's put this in and wait for the regressions.
Attachment 231481 [details] pushed as 04d68c6 - overview: overview as boxlayouts Pushed, thanks for the review!