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 752963 - Boot selection issues
Boot selection issues
Status: RESOLVED FIXED
Product: gnome-logs
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gnome-logs maintainer(s)
gnome-logs maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-07-28 13:32 UTC by Allan Day
Modified: 2015-08-27 14:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for this bug (15.85 KB, patch)
2015-08-20 15:51 UTC, Jonathan Kang
needs-work Details | Review
patch updated (19.39 KB, patch)
2015-08-20 16:53 UTC, Jonathan Kang
needs-work Details | Review
patch updated (19.55 KB, patch)
2015-08-21 09:33 UTC, Jonathan Kang
needs-work Details | Review
patch (19.81 KB, patch)
2015-08-21 10:29 UTC, Jonathan Kang
none Details | Review
patch (19.81 KB, patch)
2015-08-21 10:41 UTC, Jonathan Kang
committed Details | Review

Description Allan Day 2015-07-28 13:32:14 UTC
The new boot menu is great. However, there are a few issues with it:

1. There is no persistent indication of which boot is being displayed.
2. The menu doesn't communicate that only one boot can be displayed at once.
3. The generic hamburger icon doesn't really fit with something as specific as selecting a boot to view.

This mockup suggests a way to resolve these issues:

https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/logs/logs-boot-menu.png
Comment 1 Jonathan Kang 2015-08-20 15:51:42 UTC
Created attachment 309735 [details] [review]
patch for this bug
Comment 2 David King 2015-08-20 16:06:20 UTC
Review of attachment 309735 [details] [review]:

::: src/gl-eventtoolbar.c
@@ +113,3 @@
                                     G_MENU_MODEL (boot_menu));
+
+    grid = gtk_grid_new ();

Is there any reason why it is not possible to add a custom title to a GtkHeaderBar in the UI file?

::: src/gl-window.c
@@ +43,3 @@
 
+static gchar *
+gl_window_get_current_boot_time (GlWindow *window,

This function does not belong in GlWindow. It should be moved to where the boot IDs are read. GlJournal, maybe?

@@ +353,3 @@
+
+    GActionEntry action[] = {
+        { "view-boot", on_action_radio, "s", "'s'", on_view_boot }

Is there a reason why this cannot be part of the existing actions[]?
Comment 3 Jonathan Kang 2015-08-20 16:53:49 UTC
Created attachment 309749 [details] [review]
patch updated
Comment 4 David King 2015-08-21 07:49:59 UTC
Review of attachment 309749 [details] [review]:

One thing that seems different from the mockups is that the section title ("Boot") seems missing from the popover menu. Other than that, this seems ready to merge.
Comment 5 Jonathan Kang 2015-08-21 09:33:39 UTC
Created attachment 309794 [details] [review]
patch updated

Added a label "Boot" to the menu
Comment 6 David King 2015-08-21 09:47:57 UTC
Review of attachment 309794 [details] [review]:

::: src/gl-eventtoolbar.c
@@ +112,3 @@
     }
 
+    g_menu_prepend_section (boot_menu, _("Boot"), G_MENU_MODEL (section));

A translator comment here would be useful, as boot is a noun with multiple definitions in English.

::: src/gl-util.c
@@ +236,3 @@
+                                              now, clock_format);
+
+    time_display = g_strconcat (time_first, _(" - "), time_last, NULL);

Oops, I had not noticed this before. This is not sufficient for a translatable string, and you can read the reasoning for why on the wiki: https://wiki.gnome.org/TranslationProject/DevGuidelines/Never split sentences

Also, a translator comment would be useful: https://wiki.gnome.org/TranslationProject/DevGuidelines/Use comments
Comment 7 Jonathan Kang 2015-08-21 10:29:36 UTC
Created attachment 309797 [details] [review]
patch

patch with some translator comments added.
Comment 8 Jonathan Kang 2015-08-21 10:41:59 UTC
Created attachment 309800 [details] [review]
patch
Comment 9 David King 2015-08-21 11:05:53 UTC
Review of attachment 309800 [details] [review]:

Looks good to me. We are past the freeze now, so pushing this to master will have to wait for a bit.
Comment 10 Jonathan Kang 2015-08-21 11:18:48 UTC
Pushed to master as 013a62cd56eb204d1a82dbdaef08c1528ee0b55d.
Comment 11 David King 2015-08-26 18:55:54 UTC
Approved by the release team:

https://mail.gnome.org/archives/release-team/2015-August/msg00039.html
Comment 12 Allan Day 2015-08-27 14:41:55 UTC
Thanks for pushing to get this fixed in time for the 3.18 release!