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 725345 - dialogs: always use CSD on dialogs
dialogs: always use CSD on dialogs
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2014-02-27 19:46 UTC by William Jon McCann
Modified: 2014-03-03 20:00 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
dialogs: always use CSD on dialogs (1.51 KB, patch)
2014-02-27 19:46 UTC, William Jon McCann 		committed Details | Review
before and after screenshots (70.14 KB, image/png)
2014-03-03 11:37 UTC, Allan Day 		  Details
before 1 (9.18 KB, image/png)
2014-03-03 12:09 UTC, Matthias Clasen 		  Details
before 2 (12.10 KB, image/png)
2014-03-03 12:09 UTC, Matthias Clasen 		  Details
after 1 (9.11 KB, image/png)
2014-03-03 12:10 UTC, Matthias Clasen 		  Details
after 2 (12.05 KB, image/png)
2014-03-03 12:10 UTC, Matthias Clasen 		  Details
modified screenshot (73.80 KB, image/png)
2014-03-03 14:58 UTC, Allan Day 		  Details

Description William Jon McCann 2014-02-27 19:46:24 UTC 
One of the advantages of this is so that the styling of the dialog
is completely within one theme framework. This prevents skew between
the theming expectations from the window manager and GTK+.
Comment 1 William Jon McCann 2014-02-27 19:46:26 UTC 
Created attachment 270507 [details] [review]
dialogs: always use CSD on dialogs

If we aren't using a header bar then put a fake titlebar
box on it so we can round the corners.

One of the advantages of this is so that the styling of the dialog
is completely within one theme framework. This prevents skew between
the theming expectations from the window manager and GTK+.
Comment 2 Allan Day 2014-03-03 10:55:11 UTC 
I've tested this and it seems like a big improvement. It helps with the issues described in bug 723668.
Comment 3 Frederic Peters 2014-03-03 11:15:03 UTC 
Could you post a before/after screenshot?
Comment 4 Allan Day 2014-03-03 11:37:19 UTC 
Created attachment 270771 [details]
before and after screenshots

Before is on top and after is below. Note that I seem to be suffering from an odd theme bug - the before image shouldn't look quite like that (see bug 723668).

I still feel that there's something not quite right with this; it could be something to do with the vertical spacing around the text.
Comment 5 Matthias Clasen 2014-03-03 12:09:35 UTC 
Created attachment 270776 [details]
before 1
Comment 6 Matthias Clasen 2014-03-03 12:09:56 UTC 
Created attachment 270777 [details]
before 2
Comment 7 Matthias Clasen 2014-03-03 12:10:14 UTC 
Created attachment 270778 [details]
after 1
Comment 8 Matthias Clasen 2014-03-03 12:10:36 UTC 
Created attachment 270779 [details]
after 2
Comment 9 Allan Day 2014-03-03 14:58:37 UTC 
Created attachment 270811 [details]
modified screenshot

I spoke to Jakub about the spacing of the text earlier today, and he suggested increasing the vertical padding above the heading. I've tried this out by modifying a screenshot, and the result is attached.

The top image shows the current state, which has been modified in the bottom image - here the spacing between the two lines of text has been very slightly reduced, and additional padding has been added above the heading.

Jakub thinks this is an improvement, and I agree.

That said, the original patch is already better than the state in master, so I'd be happy with landing it as is.
Comment 10 Matthias Clasen 2014-03-03 20:00:48 UTC 
Going with Jon's patch as-is for now. We can always iterate

Attachment 270507 [details] pushed as 322f6c7 - dialogs: always use CSD on dialogs