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 610070 - alignment of window captions in window selector
alignment of window captions in window selector
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-02-16 05:47 UTC by William Jon McCann
Modified: 2010-02-17 17:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (615.63 KB, image/png)
2010-02-16 05:48 UTC, William Jon McCann
  Details
align the bottoms of the window previews (1.37 KB, patch)
2010-02-17 03:53 UTC, Maxim Ermilov
none Details | Review
Correct position of window previews. (2.09 KB, patch)
2010-02-17 04:18 UTC, Maxim Ermilov
none Details | Review
Correct position of window previews. (2.58 KB, patch)
2010-02-17 05:05 UTC, Maxim Ermilov
needs-work Details | Review
Correct position of window previews. (2.74 KB, patch)
2010-02-17 15:56 UTC, Maxim Ermilov
accepted-commit_now Details | Review

Description William Jon McCann 2010-02-16 05:47:42 UTC
Currently we seem to align our window previews by centering vertically.  I think this may have worked OK before we added the caption boxes underneath the windows.  But now it causes the caption boxes to be misaligned and that doesn't look so hot.

Simply aligning the caption boxes would work but that would cause a large gap between the window preview and the text in some cases.  That doesn't sound like a good thing.

Perhaps, one solution is to align the bottoms of the windows.
Comment 1 William Jon McCann 2010-02-16 05:48:12 UTC
Created attachment 153895 [details]
screenshot
Comment 2 Maxim Ermilov 2010-02-17 03:53:38 UTC
Created attachment 153993 [details] [review]
align the bottoms of the window previews
Comment 3 Maxim Ermilov 2010-02-17 04:18:34 UTC
Created attachment 153994 [details] [review]
Correct position of window previews.

align the bottoms of the window previews.
window previews always fit in the slot(Before this patch, caption can overlap border of workspace.).
Comment 4 William Jon McCann 2010-02-17 04:31:42 UTC
Looks much nicer!  One little thing I noticed with the most recent patch is that in some cases the circular close button we put over the window gets clipped slightly on the top by the edge of the workspace.  I think it might be OK for the close button to extend slightly over the edge rather than get clipped.
Comment 5 Maxim Ermilov 2010-02-17 05:05:59 UTC
Created attachment 153998 [details] [review]
Correct position of window previews.

window previews(and close button) always fit in the slot
Comment 6 Dan Winship 2010-02-17 14:27:17 UTC
Comment on attachment 153998 [details] [review]
Correct position of window previews.

It's not obvious what the two values returned from WindowOverlay.chromeHeight() are supposed to be, and just calling them chromeHeight[0] and chromeHeight[1] doesn't help.

If you're going to return both values from chromeHeight(), then you should *definitely* use a destructuring assignment:

    let [whateverTheFirstNumberIs, whateverTheSecondNumberIs] = this._windowOverlays[1].chromeHeight();

rather than assigning them to an array, and (depending on what the two numbers are) you might want to rename chromeHeight to something more obvious as well (chromeHeightAndWeight or whatever it is).

Alternatively, you could leave chromeHeight as it was before your patch, and add a new method to return the new value.
Comment 7 Maxim Ermilov 2010-02-17 15:56:36 UTC
Created attachment 154046 [details] [review]
Correct position of window previews.
Comment 8 Dan Winship 2010-02-17 16:08:52 UTC
Comment on attachment 154046 [details] [review]
Correct position of window previews.

ok.
>+        let [chromeButtonOuterHeight, chromeCaptionHeight] = this._windowOverlays[1].chromeHeights();

could also just remove the "chrome" here and call them "buttonOuterHeight" and "captionHeight" (and maybe change chromeWidth to buttonOuterWidth?) Or leave it as is.