GNOME Bugzilla – Bug 731312
Update window layout
Last modified: 2014-07-04 13:24:11 UTC
Replace the menu bar and status bar with a header bar.
Created attachment 277984 [details] [review] app menu: standardize help/about/quit As of 3.12, we are using this style fairly consistently.
Created attachment 277985 [details] [review] Move the board size preference to the app menu We're going to treat this as an application-level preference.
Created attachment 277986 [details] [review] Depend on GTK+ 3.13.2 For RTL icon theme support
Created attachment 277987 [details] [review] Replace the menu bar with a header bar
Created attachment 277988 [details] [review] trivial: replace GtkVBox with GtkBox GtkVBox has been deprecated
Created attachment 277989 [details] [review] Remove status bar Center the timer beneath the grid, and use a clock icon instead of a text label. I tried placing the timer in the header bar instead, but that did not look as good as expected.
Review of attachment 277989 [details] [review]: ::: src/main.c @@ +369,3 @@ /* Translators: this is the format for the timer label. The first parameter is the number of minutes which have elapsed since the start of the * game; the second parameter is the number of seconds. */ + gchar *text = g_strdup_printf (_("%02u:%02u"), hitori->timer_value / 60, hitori->timer_value % 60); No clue if it makes sense for this to remain translatable...
Review of attachment 277984 [details] [review]: ++
Review of attachment 277985 [details] [review]: ++
Review of attachment 277986 [details] [review]: I would prefer if this were merged with the commit which actually introduces the dependency (such as the next patch in the series).
Review of attachment 277987 [details] [review]: Looks great! ++
Review of attachment 277988 [details] [review]: No need for the 'trivial:' in the commit message.
Review of attachment 277989 [details] [review]: ::: data/hitori.ui @@ +176,3 @@ + <property name="visible">True</property> + </object> + </child> The icon and the text aren't vertically aligned, which looks a little jarring. Could that be fixed? ::: src/main.c @@ +369,3 @@ /* Translators: this is the format for the timer label. The first parameter is the number of minutes which have elapsed since the start of the * game; the second parameter is the number of seconds. */ + gchar *text = g_strdup_printf (_("%02u:%02u"), hitori->timer_value / 60, hitori->timer_value % 60); I think it does. For one thing, it means we can do an 'en' locale which uses the ratio character ('∶') as the separator, instead of a colon.
(In reply to comment #10) > I would prefer if this were merged with the commit which actually introduces > the dependency (such as the next patch in the series). OK. (I normally do a separate commit immediately prior to the commit that introduces the dependency because I like to call out updated dependencies when writing the NEWS file, but the extra commit is just clutter if you don't.) (In reply to comment #13) > The icon and the text aren't vertically aligned, which looks a little jarring. > Could that be fixed? Good catch. > I think it does. For one thing, it means we can do an 'en' locale which uses > the ratio character ('∶') as the separator, instead of a colon. Hm, you're right that we're supposed to use a ratio character rather than the colon. I've been converting several colons into ratios (and hyphens into en dashes, and periods into ellipses...) throughout the GNOME games, but I've been doing this in the default locale. Isn't that standard practice throughout GNOME?
(In reply to comment #14) > > I think it does. For one thing, it means we can do an 'en' locale which uses > > the ratio character ('∶') as the separator, instead of a colon. > > Hm, you're right that we're supposed to use a ratio character rather than the > colon. I've been converting several colons into ratios (and hyphens into en > dashes, and periods into ellipses...) throughout the GNOME games, but I've been > doing this in the default locale. Isn't that standard practice throughout > GNOME? Converting en-dashes, quotation marks and ellipses is fine. IIRC ratios are more complex because they interfere with RTL locales and the two components of the time get swapped around when they shouldn’t if the game is run in an RTL locale. I can’t remember what the official solution for this is (potentially putting a ratio character plus Unicode direction indicators into the C locale), but let’s go with whatever is standard across GNOME atm. I think hadess or kalev are clued-up about this, so you could ping them.
Created attachment 278642 [details] [review] app menu: standardize help/about/quit As of 3.12, we are using this style fairly consistently.
Created attachment 278643 [details] [review] Move the board size preference to the app menu We're going to treat this as an application-level preference.
Created attachment 278644 [details] [review] Replace the menu bar with a header bar Depends on GTK+ 3.13.2 for RTL icon theme support.
Created attachment 278645 [details] [review] Replace GtkVBox with GtkBox GtkVBox has been deprecated
Created attachment 278646 [details] [review] Remove status bar Center the timer beneath the grid, and use a clock icon instead of a text label. I tried placing the timer in the header bar instead, but that did not look as good as expected.
Still need to figure out what to do about the ratio character. Attachment 278642 [details] pushed as 99d9f9a - app menu: standardize help/about/quit Attachment 278643 [details] pushed as 236fc7a - Move the board size preference to the app menu Attachment 278644 [details] pushed as 927c13a - Replace the menu bar with a header bar Attachment 278645 [details] pushed as d9d3b6c - Replace GtkVBox with GtkBox
Created attachment 278648 [details] [review] Remove status bar Center the timer beneath the grid, and use a clock icon instead of a text label. I tried placing the timer in the header bar instead, but that did not look as good as expected.
In reply to comment #15: I found Bug #726232, but the solution there does not seem reasonable to apply to all of the games (I unfortunately have to fix this in several) and other apps that use the ratio character, so instead I just changed the string to the ratio character followed by the LTR character, which seems simplest, and removed the call to gettext(). Kalev, Bastien, does this seem reasonable? If not, what is your recommendation? Note that this is a timer, not a time.
Seems reasonable to me. My understanding is that using the ratio character + the LTR direction mark should make it show up in the right way in the RTL locales, so you should be fine here. It will blow up for non-UTF8 locales of course, but I doubt that's an issue. In gnome-desktop, the code does a few additional things in order to work properly in the C locale (which is non-UTF8). It uses the the ratio character for UTF8 locales and falls back to colon for non-UTF8 locales. Frankly, I've already forgotten why we had to support the C locale there :-) Regarding whether it makes sense to make this translatable, better ask the translation team.
OK, thanks. We surely do not care about non-UTF-8 locales if we're required to use curly quotes and ellipses, so I think this last patch is good (Philip?), asides from the question of whether we want to mark the lovely string "%02u\xE2\x88\xB6\xE2\x80\x8E%02u" for translation. Yosef, can you help answer this? The translator comment would be something like: "The timer. The first %02u is the number of minutes and the last %02u is seconds, both as two digits. In the middle is a ratio character and a LTR order mark to prevent the minutes and seconds from being reversed in RTL locales."
The answere is: it needs to be differente in some language? if yes (and I think the answer is yes) so we want to mark this string as translatable. In Hebrew (RTL language) look like I need to translate this string to the original string. btw, I think we can to move this string from the statusbar to the subtitle of the headerbar.
(In reply to comment #25) > OK, thanks. We surely do not care about non-UTF-8 locales if we're required to > use curly quotes and ellipses, so I think this last patch is good (Philip?), Yes, I think so. And I think we should keep the string translatable as Yosef says. Adding the translator comment is a good idea.
Created attachment 278723 [details] [review] Remove status bar Center the timer beneath the grid, and use a clock icon instead of a text label. I tried placing the timer in the header bar instead, but that did not look as good as expected.
Review of attachment 278723 [details] [review]: ++
Attachment 278723 [details] pushed as daf4243 - Remove status bar
(In reply to comment #24) > Regarding whether it makes sense to make this translatable, better ask the > translation team. The best way to do it now is to use the colon ":" everywhere, and swap it for a ratio character with a marker: https://git.gnome.org/browse/gnome-desktop/tree/libgnome-desktop/gnome-wall-clock.c?h=gnome-3-12#n250
What's the advantage of doing that? Just to keep the translatable string more sane?
(In reply to comment #32) > What's the advantage of doing that? Just to keep the translatable string more > sane? Forgetting to use a ratio symbol, or forgetting the LTR marker.