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 655974 - Give the notification buttons the same look as those in modal dialogs
Give the notification buttons the same look as those in modal dialogs
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.0.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-08-04 14:03 UTC by Allan Day
Modified: 2011-08-19 22:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch - new notfication button style (1.83 KB, patch)
2011-08-04 14:08 UTC, Allan Day
needs-work Details | Review
patch - consistent button theming (6.80 KB, patch)
2011-08-16 18:59 UTC, Allan Day
none Details | Review
patch - consistent button theming (6.33 KB, patch)
2011-08-18 20:57 UTC, Allan Day
none Details | Review
patch - consistent button theming (6.23 KB, patch)
2011-08-18 21:05 UTC, Allan Day
committed Details | Review

Description Allan Day 2011-08-04 14:03:06 UTC
Notification buttons currently have a flat solid colour, which doesn't fit well with the rest of the theme. Using the same look as in modal dialogs would fix this and make everything more consistent.
Comment 1 Allan Day 2011-08-04 14:08:01 UTC
Created attachment 193253 [details] [review]
patch - new notfication button style

One thing to note about this patch - I struggled to get the label to be properly vertically centered, since an extra pixel to the top padding was being added. I ended up having to compensate in the theme itself - hence the uneven padding figures.
Comment 2 Florian Müllner 2011-08-04 22:11:31 UTC
Review of attachment 193253 [details] [review]:

Looks nice. Not quite sure about the top/bottom padding hackery, but CSS is in the design domain, so in the end it's you who'll have to put up with it :-)

There's one major problem though when pressing the focused button (in notifications with multiple actions) which needs fixing (see details below). Also I suppose icon actions (.notification-icon-button) should be updated as well?

::: data/theme/gnome-shell.css
@@ +1111,3 @@
+    padding-left: 42px;
+    padding-bottom: 5px;
+    padding-top: 4px;

Mmmh, a bit ugly ... maybe write this as
   padding: 5px 42p;
   padding-top: 4px;
and add a comment why top and bottom padding defer?

I'm also a bit wary that the fix might be specific for Cantarell ...

@@ +1133,2 @@
 .notification-button:active {
+    border: 1px solid #8b8b8b;

This is problematic if the button has focus, as the padding from :focus (which adjusts for the wider border) is applied here, causing the button to change size.

In particular when a notification has multiple buttons, the effect is very visible as shifting of all the buttons (try for instance the test-multi-actions test in libnotify).

To fix, you can either re-define the padding of undefined buttons here, or add something like

.notification-button:focus:active {
  border: 2px solid #8b8b8b;
}

to enforce a two pixel border in that case.

Another possibility would be to swap :focus and :active, but relying on the order in the CSS is a pretty bad idea in my opinion.
Comment 3 Allan Day 2011-08-16 18:59:53 UTC
Created attachment 193988 [details] [review]
patch - consistent button theming

Here's an updated (and much expanded) patch. It updates all the buttons to use the same style (including search buttons) and backports a few of the new changes (the vertical padding, reducing the button size when focused) to modal dialog buttons.

The only problem I have right now is that I've been unable to test the notification item button changes, since I never seem to see them in the wild.
Comment 4 Allan Day 2011-08-18 20:57:21 UTC
Created attachment 194170 [details] [review]
patch - consistent button theming

Updated patch. This version applies the same style to dash search buttons, adds the necessary comment, consolidates some shared class properties and does some general tidy up.
Comment 5 Allan Day 2011-08-18 21:05:05 UTC
Created attachment 194171 [details] [review]
patch - consistent button theming

Apologies, there was a slight mistake in the previous patch.

I still haven't been able to test the notification-icon-button style, by the way.
Comment 6 Jasper St. Pierre (not reading bugmail) 2011-08-19 12:45:04 UTC
Review of attachment 194171 [details] [review]:

LGTM.