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 733372 - Inline composer's close and detach buttons should follow theme for placement
Inline composer's close and detach buttons should follow theme for placement
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: composer
master
Other Linux
: Normal normal
: ---
Assigned To: Geary Maintainers
Geary Maintainers
review
Depends on:
Blocks:
 
 
Reported: 2014-07-18 16:50 UTC by Robert Schroll
Modified: 2014-07-23 02:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Set the side of the window buttons in the HeaderBar from the GTK theme (5.38 KB, patch)
2014-07-20 02:57 UTC, Robert Schroll
none Details | Review
Set the side of the window buttons in the HeaderBar from the GTK theme (5.77 KB, patch)
2014-07-21 02:33 UTC, Robert Schroll
committed Details | Review

Description Robert Schroll 2014-07-18 16:50:12 UTC
Currently, we hard-code the close and detach buttons to appear at the end of the headerbar.  But the built-in close button appears either at the start or the end, depending on the Gtk theme.  (Not the window manager theme, because being able to be inconsistent is important.)  The property to look for is "-GtkWindow-decoration-button-layout".  It's value is a comma-separated list of buttons to appear, with a colon separating those to appear at the start from those to appear at the end.  We can probably just check on which side of the colon "close" appears.
Comment 1 Robert Schroll 2014-07-20 02:57:07 UTC
Created attachment 281216 [details] [review]
Set the side of the window buttons in the HeaderBar from the GTK theme

Make two sets of the buttons, one for the start and one for the end of
the HeaderBar, and show one or the other based on the the theme.  Watch
the style context for changes in the theme and update accordingly.  Once
the composer is detached, the default close button is used, so hide both
and stop watching for style updates.  This assumes we never re-attach a
composer.
Comment 2 Robert Schroll 2014-07-21 02:33:32 UTC
Created attachment 281277 [details] [review]
Set the side of the window buttons in the HeaderBar from the GTK theme

This adds a bit of space between the separator and the detach button.  
The HeaderBar has a spacing of 6, but the separator and detach button 
are in a Box with 0 spacing.  So I added a margin to the detach button 
to mimic this.  I think the close and detach buttons look good with no 
space between them so I couldn't do this with spacing in the Box.

What we really want are margin_start and margin_end, but they aren't 
available on GTK 3.10, so I implement it myself.
Comment 3 Jim Nelson 2014-07-22 19:51:31 UTC
Review of attachment 281277 [details] [review]:

The logic in set_win_buttons_side() is scary, but if that's how they're doing it in GTK, then I guess that's how we need to do it too.

Commit!
Comment 4 Robert Schroll 2014-07-23 02:30:57 UTC
Yeah, I think the logic is that way is because you can do that pithily 
with pointer tricks.  (Not that that's a *good* reason.)  As long as the 
layout string is correctly formatted, you'll always get what you want.  
But the behavior when it's not is a bit odd.

Attachment 281277 [details] pushed as 02eca78 - Set the side of the window buttons in the HeaderBar from the GTK theme