GNOME Bugzilla – Bug 756377
00:xx times displayed in 12hour clock mode
Last modified: 2017-04-17 18:20:40 UTC
Created attachment 313045 [details] [review] A simple patch fixing this problem The 12hour clock in gnome-calendar displays the time "12:30pm" as "00:30pm" which is not the standard(at least that is not the standard here in north america). This patch fixes this in the edit box for both the entering and display of values.
(In reply to Trevor Gale from comment #0) > Created attachment 313045 [details] [review] [review] > A simple patch fixing this problem > > The 12hour clock in gnome-calendar displays the time "12:30pm" as "00:30pm" > which is not the standard(at least that is not the standard here in north > america). This patch fixes this in the edit box for both the entering and > display of values. If you post the patch properly formatted I can review it. A really fast look at it, tells me it will show 12:05AM too, and we want 00:05 AM, right?
Created attachment 313150 [details] [review] 12h clock time-selector fix
The gnome-calendar app currently(without the patch) shows the time 5 minutes after midnight as 00:05AM. The standard way of displaying that time is 12:05AM. The same thing happens 5 minutes after noon(00:05 becomes 12:05PM). Hopefully my re-submitted patch is correctly formatted, if it's not please let me know and I will try to fix the problem(whether it is with the submission method or coding style).
(In reply to Trevor Gale from comment #3) > The gnome-calendar app currently(without the patch) shows the time 5 minutes > after midnight as 00:05AM. The standard way of displaying that time is > 12:05AM. The same thing happens 5 minutes after noon(00:05 becomes 12:05PM). > > Hopefully my re-submitted patch is correctly formatted, if it's not please > let me know and I will try to fix the problem(whether it is with the > submission method or coding style). I'll take a look tonight, tomorrow morning you'll have your answer.
Review of attachment 313150 [details] [review]: Improve it, and we'll push it ::: src/gcal-time-selector.c @@ +143,3 @@ gtk_adjustment_set_upper (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 23.0); else + gtk_adjustment_set_upper (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 12.0); If you're going to set this, then we need to call GtkAdjustment.set_lower() with 1. Otherwise we'll have 13 hours from [0, 12] both included. @@ +241,3 @@ + if(hours == 0){ + hours = 12; + } I don't like the if, I rather have this: `hours = hours % 12` + (hours == 0 ? 12 : 0);` It's not a big deal tho.
(In reply to Trevor Gale from comment #3) > The gnome-calendar app currently(without the patch) shows the time 5 minutes > after midnight as 00:05AM. The standard way of displaying that time is > 12:05AM. The same thing happens 5 minutes after noon(00:05 becomes 12:05PM). > > Hopefully my re-submitted patch is correctly formatted, if it's not please > let me know and I will try to fix the problem(whether it is with the > submission method or coding style). Sorry, it took me more than what I expected to get to it. Thank you for your work on this.
Created attachment 313511 [details] [review] time selector fix for 12h setting
I included the changes as you suggested.
Review of attachment 313511 [details] [review]: ::: src/gcal-time-selector.c @@ +144,3 @@ else + gtk_adjustment_set_lower (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 1.0); + gtk_adjustment_set_upper (gtk_spin_button_get_adjustment (GTK_SPIN_BUTTON (priv->hour_spin)), 12.0); You're missing two pairs of curly brackets here, right? Otherwise the else part of the if will just execute the first statement. Then for code styling issues, if you put curly brackets in the else part, and you need them, you'll have to add them to the then part.
Created attachment 332245 [details] [review] time-selector: fix 12-hour format Currently, the date selector allows hours in range [0; 11], which is not the convention. This commit fixes the conversion logic.
Comment on attachment 332245 [details] [review] time-selector: fix 12-hour format Sure.
Attachment 332245 [details] pushed as c16864f - time-selector: fix 12-hour format