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 688234 - Window thumbnail caption width allocation
Window thumbnail caption width allocation
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-11-13 11:09 UTC by Allan Day
Modified: 2013-02-19 10:37 UTC
See Also:
GNOME target: ---
GNOME version: 3.7/3.8


Attachments
screenshot (606.98 KB, image/png)
2012-11-13 11:09 UTC, Allan Day
  Details
Workspace: fix window clone slot reporting (5.12 KB, patch)
2012-12-16 23:48 UTC, Giovanni Campagna
committed Details | Review
WindowOverlay: fix title sizing (1.13 KB, patch)
2012-12-16 23:49 UTC, Giovanni Campagna
none Details | Review
WorkspacesView: don't use clutter_actor_reparent() (1016 bytes, patch)
2012-12-16 23:49 UTC, Giovanni Campagna
committed Details | Review
WorkspacesDisplay: clean up workspacesView lists (3.44 KB, patch)
2012-12-16 23:49 UTC, Giovanni Campagna
committed Details | Review
WindowOverlay: fix title sizing (1.66 KB, patch)
2012-12-17 00:01 UTC, Giovanni Campagna
committed Details | Review

Description Allan Day 2012-11-13 11:09:27 UTC
Created attachment 228871 [details]
screenshot

The window selector currently switches over to a grid layout when there are more than two rows of thumbnails. If you stay in the overview and add windows, you can see this switch happen. When it does, the captions beneath the thumbnails don't update properly. They end up overlapping (see attached screenshot).

Steps to reproduce:

 * Open the overview with two rows of windows
 * Drag app launchers to the window area to open more windows
 * Keep doing this until the window selector switches to grid layout
 * Observe
Comment 1 Allan Day 2012-11-13 11:16:52 UTC
If it isn't too much work, it might be interesting to experiment with not using the grid layout before tackling this.
Comment 2 Allan Day 2012-12-14 12:42:05 UTC
Let's make this a general bug about the layout of the window captions - there seems to be a few generic issues here.

When the thumbnails aren't in a grid, the captions should be restricted to the width of the window thumbnails. When the thumbnails are in a grid, the captions should be able to extend to the width of the grid cell (but shouldn't become wider than the cell).
Comment 3 Allan Day 2012-12-14 12:44:49 UTC
Let's make this a general bug about the layout of the window captions - there seems to be a few generic issues here.

When the thumbnails aren't in a grid, the captions should be restricted to the width of the window thumbnails. When the thumbnails are in a grid, the captions should be able to extend to the width of the grid cell (but shouldn't become wider than the cell).
Comment 4 Giovanni Campagna 2012-12-16 23:48:53 UTC
Created attachment 231685 [details] [review]
Workspace: fix window clone slot reporting

WindowOverlay was at times seeing bogus values reported as WindowClone
sizes. Fix that by storing and passing the value from the authoritative
source, that is, the LayoutStrategy.
Comment 5 Giovanni Campagna 2012-12-16 23:49:02 UTC
Created attachment 231686 [details] [review]
WindowOverlay: fix title sizing

After the first time the title was placed, we were setting its width,
thus forcing get_preferred_width() to return that as the minimum and
natural width.
To workaround that, explicitly reset the width to -1, -1, causing
StLabel->get_preferred_width() to be called, which would give us a meaningful
value for minimum and natural width.
Comment 6 Giovanni Campagna 2012-12-16 23:49:23 UTC
Created attachment 231687 [details] [review]
WorkspacesView: don't use clutter_actor_reparent()

It's deprecated and in this case is not needed because at that point
the actor hasn't been parented yet.
Comment 7 Giovanni Campagna 2012-12-16 23:49:32 UTC
Created attachment 231688 [details] [review]
WorkspacesDisplay: clean up workspacesView lists

Rather than sometimes having a list and sometimes null, keep an array
always and check for its length.
Comment 8 Giovanni Campagna 2012-12-16 23:55:45 UTC
The attached patches fix the first part of the bug, that is, getting the window overlay to follow the clone size. Well, patch #231686 is the important one, the two on WorkspacesView are code cleanups and the first one covers an edge case during animation.
They are on top of *everything*, in particular if you want to test them you probably need bugs 650843 and 689116, in that order.
Comment 9 Giovanni Campagna 2012-12-17 00:01:23 UTC
Created attachment 231689 [details] [review]
WindowOverlay: fix title sizing

After the first time the title was placed, we were setting its width,
thus forcing get_preferred_width() to return that as the minimum and
natural width.
To workaround that, explicitly reset the width to -1, -1, causing
StLabel->get_preferred_width() to be called, which would give us a meaningful
value for minimum and natural width.

This one is better...
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-12-17 00:04:30 UTC
Review of attachment 231685 [details] [review]:

OK.
Comment 11 Jasper St. Pierre (not reading bugmail) 2012-12-17 00:04:46 UTC
Review of attachment 231687 [details] [review]:

OK.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-12-17 00:05:29 UTC
Review of attachment 231688 [details] [review]:

I made this exact cleanup before, but OK
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-12-17 00:07:51 UTC
Review of attachment 231689 [details] [review]:

Yikes.

OK, but on the condition we talk to ebassi about better solutions for this in the future.
Comment 14 Giovanni Campagna 2012-12-17 00:14:44 UTC
(In reply to comment #13)
> Review of attachment 231689 [details] [review]:
> 
> Yikes.
> 
> OK, but on the condition we talk to ebassi about better solutions for this in
> the future.

Eh... I don't think there are many better options. Maybe clutter_actor_get_preferred_width_full (for_height, CLUTTER_SIZE_REQUEST_FLAG_IGNORE_FIXED)...

Anyway, I hate you because now I have to rebase everything!
Comment 15 Giovanni Campagna 2012-12-17 00:19:41 UTC
Umpf, I really don't want to remove the dependency on the misc cleanup patch from bug 650843. Could you review that too, please?
Thank you!
Comment 16 Giovanni Campagna 2012-12-18 15:05:59 UTC
Attachment 231685 [details] pushed as c66068c - Workspace: fix window clone slot reporting
Attachment 231687 [details] pushed as 5487f8c - WorkspacesView: don't use clutter_actor_reparent()
Attachment 231688 [details] pushed as 176daa1 - WorkspacesDisplay: clean up workspacesView lists
Attachment 231689 [details] pushed as 944762a - WindowOverlay: fix title sizing
Not closing because the special handling for grid layout is not yet implemented.
Comment 17 Allan Day 2013-01-15 09:49:15 UTC
This is fixed?
Comment 18 Giovanni Campagna 2013-01-15 14:35:08 UTC
(In reply to comment #17)
> This is fixed?

Partially: the special handling in grid mode you proposed in comment #3 is not implemented, and captions are always bounded by the size of the window, but now they update properly.
Comment 19 Allan Day 2013-01-15 14:44:05 UTC
(In reply to comment #18)
> (In reply to comment #17)
> > This is fixed?
> 
> Partially: the special handling in grid mode you proposed in comment #3 is not
> implemented, and captions are always bounded by the size of the window, but now
> they update properly.

Thanks for the update. That's the worst of it fixed, at least. Having wider captions for the grid state would be nice, of course. :)
Comment 20 Allan Day 2013-02-19 10:37:12 UTC
Let's close this. We might want to revisit the behaviour, but it will depend on where we decide to go with the window selector in general.