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 778146 - Huge AM/PM button in date/time selection dialog
Huge AM/PM button in date/time selection dialog
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Date and Time
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: Kalev Lember
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-03 15:30 UTC by Allan Day
Modified: 2017-05-17 15:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (214.49 KB, image/png)
2017-02-03 15:30 UTC, Allan Day
  Details
Fix dialog style (3.37 KB, patch)
2017-05-15 17:04 UTC, slatchurie
none Details | Review
patch v2 (3.33 KB, patch)
2017-05-15 18:19 UTC, slatchurie
committed Details | Review

Description Allan Day 2017-02-03 15:30:29 UTC
Created attachment 344872 [details]
screenshot

See the attached screenshot - the label on the AM/PM button is way too big.

There also seems to be issues with the time widgets - the text is too big for the field, making it feel cramped. The time widgets need to be wider, and there needs to be more padding around the controls as a whole.

See the mockups for layout guidance:

https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/system-settings/date-and-time/date-and-time.png
Comment 1 slatchurie 2017-05-15 17:04:42 UTC
Created attachment 351911 [details] [review]
Fix dialog style
Comment 2 Rui Matos 2017-05-15 17:26:52 UTC
Review of attachment 351911 [details] [review]:

Thanks for the patch. Can you fix the code style to be similar to the surrounding code?

::: panels/datetime/cc-datetime-panel.c
@@ +1130,3 @@
   GtkAdjustment *adjustment;
+  GdkDisplay* display;
+  GdkScreen* screen;

please follow the code style: the * goes with the variable, not the type

@@ +1133,2 @@
   GtkCssProvider *provider;
   GtkStyleContext *context;

this variable is no longer needed

@@ +1149,2 @@
                                    "}", -1, NULL);
+  display = gdk_display_get_default();

leave a space between the function name and (

@@ +1150,3 @@
+  display = gdk_display_get_default();
+  screen = gdk_display_get_default_screen(display);
+  gtk_style_context_add_provider_for_screen(screen,

could simply use gdk_screen_get_default () here

@@ +1152,3 @@
+  gtk_style_context_add_provider_for_screen(screen,
+                                            GTK_STYLE_PROVIDER(provider),
+                                            GTK_STYLE_PROVIDER_PRIORITY_USER);

why are you changing from application priority to user?
Comment 3 slatchurie 2017-05-15 18:19:32 UTC
Created attachment 351915 [details] [review]
patch v2

I have updated the patch.
Thank you for your guidance.
Comment 4 Rui Matos 2017-05-16 16:00:17 UTC
Review of attachment 351915 [details] [review]:

looks good, do you have git push access?
Comment 5 slatchurie 2017-05-16 16:55:56 UTC
(In reply to Rui Matos from comment #4)
> Review of attachment 351915 [details] [review] [review]:
> 
> looks good, do you have git push access?

Newcomer here and not familiar with git, so I guess I don't.
Comment 6 Rui Matos 2017-05-17 15:42:39 UTC
2c95c6a..c890872  master -> master