After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 682286 - Overview as BoxLayouts
Overview as BoxLayouts
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 688501 (view as bug list)
Depends on:
Blocks: 682050
 
 
Reported: 2012-08-20 18:14 UTC by Tanner Doshier
Modified: 2012-12-13 15:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
overview: overview as boxlayouts (12.00 KB, patch)
2012-08-20 18:14 UTC, Tanner Doshier
needs-work Details | Review
overview: overview as boxlayouts (11.98 KB, patch)
2012-08-21 00:28 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
workspace: Don't recalculate window positions when leaving the overview (834 bytes, patch)
2012-08-21 00:32 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
overview: overview as boxlayouts (11.85 KB, patch)
2012-12-11 15:01 UTC, Cosimo Cecchi
needs-work Details | Review
overview: overview as boxlayouts (11.75 KB, patch)
2012-12-11 20:54 UTC, Cosimo Cecchi
needs-work Details | Review
overview: overview as boxlayouts (14.09 KB, patch)
2012-12-13 14:40 UTC, Cosimo Cecchi
committed Details | Review

Description Tanner Doshier 2012-08-20 18:14:26 UTC
Avoid a bunch of manual calculations and use St.BoxLayouts to manage the
layout of the overview.
Comment 1 Tanner Doshier 2012-08-20 18:14:28 UTC
Created attachment 221869 [details] [review]
overview: overview as boxlayouts
Comment 2 Florian Müllner 2012-08-20 18:20:40 UTC
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!
Comment 3 Allan Day 2012-08-21 00:18:56 UTC
(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? ;)
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-08-21 00:28:38 UTC
Created attachment 221944 [details] [review]
overview: overview as boxlayouts
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-08-21 00:32:35 UTC
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.
Comment 6 Allan Day 2012-11-28 10:01:26 UTC
*** Bug 688501 has been marked as a duplicate of this bug. ***
Comment 7 Cosimo Cecchi 2012-12-11 15:01:15 UTC
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 8 Cosimo Cecchi 2012-12-11 19:49:28 UTC
Comment on attachment 231265 [details] [review]
overview: overview as boxlayouts

I was being too optimistic. This actually has some regressions.
Comment 9 Cosimo Cecchi 2012-12-11 20:54:30 UTC
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.
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-12-13 14:24:36 UTC
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.
Comment 11 Cosimo Cecchi 2012-12-13 14:28:14 UTC
(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.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-12-13 14:29:23 UTC
(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.
Comment 13 Cosimo Cecchi 2012-12-13 14:40:23 UTC
Created attachment 231481 [details] [review]
overview: overview as boxlayouts

--

You're right, this isn't needed anymore.
Comment 14 Jasper St. Pierre (not reading bugmail) 2012-12-13 14:49:35 UTC
Review of attachment 231481 [details] [review]:

OK. Let's put this in and wait for the regressions.
Comment 15 Cosimo Cecchi 2012-12-13 15:27:26 UTC
Attachment 231481 [details] pushed as 04d68c6 - overview: overview as boxlayouts

Pushed, thanks for the review!