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 675471 - add an app menu with a help item in it
add an app menu with a help item in it
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: shell
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
: 653591 (view as bug list)
Depends on:
Blocks: 674957
 
 
Reported: 2012-05-04 20:48 UTC by William Jon McCann
Modified: 2012-09-12 09:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
shell: Make sure the "active-panel" property is updated (4.33 KB, patch)
2012-05-08 18:07 UTC, Florian Müllner
needs-work Details | Review
cc-panel: Add get_help_uri() vfunc (1.49 KB, patch)
2012-05-08 18:07 UTC, Florian Müllner
needs-work Details | Review
panels: Implement get_help_uri() if applicable (15.41 KB, patch)
2012-05-08 18:07 UTC, Florian Müllner
reviewed Details | Review
shell: Add application menu (3.15 KB, patch)
2012-05-08 18:08 UTC, Florian Müllner
reviewed Details | Review
shell: Pass the object to show_overview_page(), not its private (3.03 KB, patch)
2012-05-18 15:29 UTC, Florian Müllner
committed Details | Review
shell: Make sure the "active-panel" property is updated (1.78 KB, patch)
2012-05-18 15:29 UTC, Florian Müllner
committed Details | Review
cc-panel: Add get_help_uri() vfunc (1.46 KB, patch)
2012-05-18 15:29 UTC, Florian Müllner
committed Details | Review
background: Add get_help_uri() implementation (1.47 KB, patch)
2012-05-18 15:29 UTC, Florian Müllner
committed Details | Review
bluetooth: Add get_help_uri() implementation (1.14 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
color: Add get_help_uri() implementation (1.42 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
datetime: Add get_help_uri() implementation (1.14 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
display: Add get_help_uri() implementation (1.32 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
keyboard: Add get_help_uri() implementation (1.29 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
mouse: Add get_help_uri() implementation (1.26 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
network: Add get_help_uri() implementation (1.33 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
online-accounts: Add get_help_uri() implementation (1.03 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
power: Add get_help_uri() implementation (1.29 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
printers: Add get_help_uri() implementation (1.14 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
region: Add get_help_uri() implementation (1.23 KB, patch)
2012-05-18 15:30 UTC, Florian Müllner
committed Details | Review
screen: Add get_help_uri() implementation (1.36 KB, patch)
2012-05-18 15:31 UTC, Florian Müllner
committed Details | Review
sound: Add get_help_uri() implementation (1.11 KB, patch)
2012-05-18 15:31 UTC, Florian Müllner
committed Details | Review
universal-access: Add get_help_uri() implementation (1.28 KB, patch)
2012-05-18 15:31 UTC, Florian Müllner
committed Details | Review
user-accounts: Add get_help_uri() implementation (1.20 KB, patch)
2012-05-18 15:31 UTC, Florian Müllner
committed Details | Review
shell: Make the main window a GtkApplicationWindow (3.10 KB, patch)
2012-05-18 15:31 UTC, Florian Müllner
committed Details | Review
shell: Add application menu (2.53 KB, patch)
2012-05-18 15:31 UTC, Florian Müllner
committed Details | Review

Description William Jon McCann 2012-05-04 20:48:10 UTC
We should probably add an app menu with Help in it.
Comment 1 Allan Day 2012-05-05 12:25:06 UTC
Agree.
Comment 2 Florian Müllner 2012-05-08 12:45:47 UTC
Just Help and no Quit? Also: should the launched help page always be the same or should it depend on the currently active settings panel?
Comment 3 Cosimo Cecchi 2012-05-08 16:21:16 UTC
Quit should always be there IMO
Comment 4 William Jon McCann 2012-05-08 16:31:59 UTC
Yeah we have Quit so I was ignoring that :)
Comment 5 Florian Müllner 2012-05-08 16:33:15 UTC
OK, so what about the second question?
Comment 6 William Jon McCann 2012-05-08 16:39:12 UTC
I guess it might be interesting to provide a hint to the help system about where the user is coming from. I think it is up to them to decide if it should be tuned to that context or not. Probably want to ask them.
Comment 7 Florian Müllner 2012-05-08 18:07:47 UTC
Created attachment 213692 [details] [review]
shell: Make sure the "active-panel" property is updated

The (currently unused) cc_shell_get_active_panel() method is useless
unless "active-panel" is actually updated correctly.
Comment 8 Florian Müllner 2012-05-08 18:07:51 UTC
Created attachment 213693 [details] [review]
cc-panel: Add get_help_uri() vfunc

We want to allow panels to point to a specific help page, so add
a vfunc for that.
Comment 9 Florian Müllner 2012-05-08 18:07:56 UTC
Created attachment 213694 [details] [review]
panels: Implement get_help_uri() if applicable

Add a get_help_uri() implementation to all panels but info and wacom.
Comment 10 Florian Müllner 2012-05-08 18:08:01 UTC
Created attachment 213695 [details] [review]
shell: Add application menu

Add a simple application menu containing just "Help" and "Quit".
Comment 11 Florian Müllner 2012-05-08 18:11:05 UTC
(In reply to comment #6)
> I guess it might be interesting to provide a hint to the help system about
> where the user is coming from. I think it is up to them to decide if it should
> be tuned to that context or not.

I don't think we can currently do hints, so I made help unconditionally depend on context. We can still revisit that as necessary.

(If we decide that a "fixed" help item is better, only the last patch is needed (with some simplifications))
Comment 12 Bastien Nocera 2012-05-18 10:34:31 UTC
Review of attachment 213692 [details] [review]:

Could you please split the changes to pass the object itself rather than its private member?
Also, the rest of the patch doesn't apply now.
Comment 13 Bastien Nocera 2012-05-18 10:35:09 UTC
Review of attachment 213693 [details] [review]:

::: shell/cc-panel.c
@@ +255,3 @@
+  if (class->get_help_uri)
+    return class->get_help_uri (panel);
+g_warning ("wort mit x ...");

Interesting.
Comment 14 Bastien Nocera 2012-05-18 10:36:05 UTC
Review of attachment 213694 [details] [review]:

I'd prefer one patch per panel instead, but the code itself is fine.
Comment 15 Bastien Nocera 2012-05-18 10:38:10 UTC
Review of attachment 213695 [details] [review]:

::: shell/control-center.c
@@ +190,3 @@
+  GtkWidget *window = cc_shell_get_toplevel (CC_SHELL (shell));
+
+  gtk_widget_destroy (window);

We unref it when handling Ctrl+Q.
Comment 16 Bastien Nocera 2012-05-18 10:41:31 UTC
*** Bug 653591 has been marked as a duplicate of this bug. ***
Comment 17 Florian Müllner 2012-05-18 15:28:57 UTC
(In reply to comment #12)
> Could you please split the changes to pass the object itself rather than its
> private member?

Sure.


(In reply to comment #13)
> +g_warning ("wort mit x ...");
> 
> Interesting.

Woops, sorry.


(In reply to comment #14)
> I'd prefer one patch per panel instead, but the code itself is fine.

It felt shamelessly boosting my commit stats, but sure :-)
Comment 18 Florian Müllner 2012-05-18 15:29:31 UTC
Created attachment 214340 [details] [review]
shell: Pass the object to show_overview_page(), not its private

We will need it to update the "active-panel" property properly.
Comment 19 Florian Müllner 2012-05-18 15:29:42 UTC
Created attachment 214341 [details] [review]
shell: Make sure the "active-panel" property is updated

The (currently unused) cc_shell_get_active_panel() method is useless
unless "active-panel" is actually updated correctly.
Comment 20 Florian Müllner 2012-05-18 15:29:53 UTC
Created attachment 214342 [details] [review]
cc-panel: Add get_help_uri() vfunc

We want to allow panels to point to a specific help page, so add
a vfunc for that.
Comment 21 Florian Müllner 2012-05-18 15:29:59 UTC
Created attachment 214343 [details] [review]
background: Add get_help_uri() implementation
Comment 22 Florian Müllner 2012-05-18 15:30:04 UTC
Created attachment 214344 [details] [review]
bluetooth: Add get_help_uri() implementation
Comment 23 Florian Müllner 2012-05-18 15:30:08 UTC
Created attachment 214345 [details] [review]
color: Add get_help_uri() implementation
Comment 24 Florian Müllner 2012-05-18 15:30:12 UTC
Created attachment 214346 [details] [review]
datetime: Add get_help_uri() implementation
Comment 25 Florian Müllner 2012-05-18 15:30:17 UTC
Created attachment 214347 [details] [review]
display: Add get_help_uri() implementation
Comment 26 Florian Müllner 2012-05-18 15:30:24 UTC
Created attachment 214348 [details] [review]
keyboard: Add get_help_uri() implementation
Comment 27 Florian Müllner 2012-05-18 15:30:30 UTC
Created attachment 214349 [details] [review]
mouse: Add get_help_uri() implementation
Comment 28 Florian Müllner 2012-05-18 15:30:35 UTC
Created attachment 214350 [details] [review]
network: Add get_help_uri() implementation
Comment 29 Florian Müllner 2012-05-18 15:30:40 UTC
Created attachment 214351 [details] [review]
online-accounts: Add get_help_uri() implementation
Comment 30 Florian Müllner 2012-05-18 15:30:46 UTC
Created attachment 214352 [details] [review]
power: Add get_help_uri() implementation
Comment 31 Florian Müllner 2012-05-18 15:30:51 UTC
Created attachment 214353 [details] [review]
printers: Add get_help_uri() implementation
Comment 32 Florian Müllner 2012-05-18 15:30:56 UTC
Created attachment 214354 [details] [review]
region: Add get_help_uri() implementation
Comment 33 Florian Müllner 2012-05-18 15:31:05 UTC
Created attachment 214355 [details] [review]
screen: Add get_help_uri() implementation
Comment 34 Florian Müllner 2012-05-18 15:31:14 UTC
Created attachment 214356 [details] [review]
sound: Add get_help_uri() implementation
Comment 35 Florian Müllner 2012-05-18 15:31:21 UTC
Created attachment 214357 [details] [review]
universal-access: Add get_help_uri() implementation
Comment 36 Florian Müllner 2012-05-18 15:31:28 UTC
Created attachment 214358 [details] [review]
user-accounts: Add get_help_uri() implementation
Comment 37 Florian Müllner 2012-05-18 15:31:33 UTC
Created attachment 214359 [details] [review]
shell: Make the main window a GtkApplicationWindow

This is a prerequisite of using the new GMenu API. Also move the
check for small screen sizes introduced in commit 22ed5a9fd5, as
GtkApplicationWindows cannot be realized unless their application
property has been set.
Comment 38 Florian Müllner 2012-05-18 15:31:42 UTC
Created attachment 214360 [details] [review]
shell: Add application menu

Add a simple application menu containing just "Help" and "Quit".
Comment 39 Florian Müllner 2012-05-18 16:49:36 UTC
Patch series approved on IRC, pushing.

Attachment 214340 [details] pushed as ce8e120 - shell: Pass the object to show_overview_page(), not its private
Attachment 214341 [details] pushed as 7871703 - shell: Make sure the "active-panel" property is updated
Attachment 214342 [details] pushed as 6827068 - cc-panel: Add get_help_uri() vfunc
Attachment 214343 [details] pushed as f863747 - background: Add get_help_uri() implementation
Attachment 214344 [details] pushed as bc18595 - bluetooth: Add get_help_uri() implementation
Attachment 214345 [details] pushed as 186d615 - color: Add get_help_uri() implementation
Attachment 214346 [details] pushed as afe99e3 - datetime: Add get_help_uri() implementation
Attachment 214347 [details] pushed as 4e374a2 - display: Add get_help_uri() implementation
Attachment 214348 [details] pushed as 7adcd01 - keyboard: Add get_help_uri() implementation
Attachment 214349 [details] pushed as 164d8b5 - mouse: Add get_help_uri() implementation
Attachment 214350 [details] pushed as 1bbe93c - network: Add get_help_uri() implementation
Attachment 214351 [details] pushed as 4b72311 - online-accounts: Add get_help_uri() implementation
Attachment 214352 [details] pushed as e38491e - power: Add get_help_uri() implementation
Attachment 214353 [details] pushed as 0c353fb - printers: Add get_help_uri() implementation
Attachment 214354 [details] pushed as 79b1189 - region: Add get_help_uri() implementation
Attachment 214355 [details] pushed as dc38101 - screen: Add get_help_uri() implementation
Attachment 214356 [details] pushed as 45b7938 - sound: Add get_help_uri() implementation
Attachment 214357 [details] pushed as fa4a957 - universal-access: Add get_help_uri() implementation
Attachment 214358 [details] pushed as a901cc1 - user-accounts: Add get_help_uri() implementation
Attachment 214359 [details] pushed as ad4da16 - shell: Make the main window a GtkApplicationWindow
Attachment 214360 [details] pushed as c626ba5 - shell: Add application menu
Comment 40 Michael Hill 2012-09-12 00:14:43 UTC
wacom: refer to wacom.page instead of prefs.page in 3.6 user docs.
Comment 41 Bastien Nocera 2012-09-12 09:25:17 UTC
(In reply to comment #40)
> wacom: refer to wacom.page instead of prefs.page in 3.6 user docs.

Seriously, you could have filed a new bug here. This comment doesn't even apply to GNOME 3.4 which the original patch was targeting.
Comment 42 Bastien Nocera 2012-09-12 09:26:10 UTC
Done that in master for gnome 3.6.

commit 2a88736cf61395b731b3d3f8f9615ec11248d004
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed Sep 12 10:24:19 2012 +0100

    wacom: Add link to wacom docs
    
    And not the generic control-center docs.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=675471#c40