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 684280 - RFE: jump to the shortcut settings from link directly
RFE: jump to the shortcut settings from link directly
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Region & Language
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-09-18 09:53 UTC by Akira TAGOH
Modified: 2012-09-21 19:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
keyboard: Allow switching to a specific section (5.74 KB, patch)
2012-09-18 11:27 UTC, Bastien Nocera
accepted-commit_now Details | Review
region: Switch to the input sources section (932 bytes, patch)
2012-09-18 11:27 UTC, Bastien Nocera
accepted-commit_now Details | Review
keyboard: Allow switching to a specific section (5.87 KB, patch)
2012-09-18 13:46 UTC, Bastien Nocera
none Details | Review
keyboard: Allow switching to a specific section (8.03 KB, patch)
2012-09-18 13:59 UTC, Bastien Nocera
reviewed Details | Review
region: Switch to the input sources section (929 bytes, patch)
2012-09-18 13:59 UTC, Bastien Nocera
committed Details | Review
keyboard: Add unique IDs for each shortcuts section (6.29 KB, patch)
2012-09-18 17:03 UTC, Bastien Nocera
accepted-commit_now Details | Review
keyboard: Add ability to switch to a shortcuts section (4.56 KB, patch)
2012-09-18 17:03 UTC, Bastien Nocera
accepted-commit_now Details | Review
keyboard: Handle arguments for the shortcuts tab (1.93 KB, patch)
2012-09-18 17:03 UTC, Bastien Nocera
accepted-commit_now Details | Review

Description Akira TAGOH 2012-09-18 09:53:16 UTC
There are a link to the shortcut settings on the input sources tab though, that would be intuitive and give us a good experience if it jumps to "Typing" section in the shortcut settings directly.
Comment 1 Bastien Nocera 2012-09-18 10:28:44 UTC
Works correctly here, and jumps to the correct tab. Any special way to reproducing the problem? Which versions did you test with?
Comment 2 Rui Matos 2012-09-18 10:35:46 UTC
The request is to jump directly to the Typing section I think.
Comment 3 Bastien Nocera 2012-09-18 10:46:30 UTC
Oh, right. Yes, we could do that actually.
Comment 4 Bastien Nocera 2012-09-18 11:27:03 UTC
Created attachment 224608 [details] [review]
keyboard: Allow switching to a specific section
Comment 5 Bastien Nocera 2012-09-18 11:27:13 UTC
Created attachment 224609 [details] [review]
region: Switch to the input sources section

When clicking on the link.
Comment 6 Rui Matos 2012-09-18 13:32:40 UTC
Review of attachment 224609 [details] [review]:

++
Comment 7 Rui Matos 2012-09-18 13:33:45 UTC
Review of attachment 224608 [details] [review]:

Looks good. Just some cosmetic nits below.

::: panels/keyboard/keyboard-shortcuts.c
@@ +669,3 @@
   append_sections_from_gsettings (builder);
 
   /* Select the first item */

This comment is wrong now.

@@ +678,3 @@
+    selection = gtk_tree_view_get_selection (section_treeview);
+    gtk_tree_selection_select_iter (selection, &iter);
+  }

I know the code style is inconsistent in g-c-c but this file uses braces-on-their-own-line everywhere else.

@@ +1811,3 @@
+
+  builder = g_object_get_data (G_OBJECT (panel), "builder");
+  if (builder == NULL) {

braces
Comment 8 Bastien Nocera 2012-09-18 13:46:50 UTC
Created attachment 224627 [details] [review]
keyboard: Allow switching to a specific section
Comment 9 Bastien Nocera 2012-09-18 13:59:41 UTC
Created attachment 224630 [details] [review]
keyboard: Allow switching to a specific section
Comment 10 Bastien Nocera 2012-09-18 13:59:52 UTC
Created attachment 224631 [details] [review]
region: Switch to the input sources section

When clicking on the link.
Comment 11 Rui Matos 2012-09-18 14:30:23 UTC
Review of attachment 224630 [details] [review]:

See the comment below, up to you if you want to split this or go with the previous approach.

::: panels/keyboard/keyboard-shortcuts.c
@@ +73,3 @@
 {
   SECTION_DESCRIPTION_COLUMN,
+  SECTION_ID_COLUMN,

This would be better, but I think it should go in a separate patch before this one.

Also, if we want to do this, I think we should also change the kb_*_sections hash tables to use this ID instead of the translated description.

And in section_selection_changed() we could then also use the ID instead of the description to match the Typing section to add the XKB options.
Comment 12 Bastien Nocera 2012-09-18 17:03:06 UTC
Created attachment 224650 [details] [review]
keyboard: Add unique IDs for each shortcuts section

Rather than relying on comparison of translated strings.
Comment 13 Bastien Nocera 2012-09-18 17:03:19 UTC
Created attachment 224651 [details] [review]
keyboard: Add ability to switch to a shortcuts section

With some new API.
Comment 14 Bastien Nocera 2012-09-18 17:03:31 UTC
Created attachment 224652 [details] [review]
keyboard: Handle arguments for the shortcuts tab

So it's possible to switch directly to a section of the shortcuts
tab, rather than have users look for particular sections when redirected
there through a link.
Comment 15 Rui Matos 2012-09-18 21:04:08 UTC
Review of attachment 224650 [details] [review]:

Yes
Comment 16 Rui Matos 2012-09-18 21:09:50 UTC
Review of attachment 224651 [details] [review]:

Looks good
Comment 17 Rui Matos 2012-09-18 21:12:35 UTC
Review of attachment 224652 [details] [review]:

Fine
Comment 18 Rui Matos 2012-09-18 21:12:53 UTC
Review of attachment 224631 [details] [review]:

++
Comment 19 Matthias Clasen 2012-09-18 22:41:32 UTC
A bit confusing that we have a 'Typing' tab, and a 'Typing' section in the 'Shortcuts' tab. But the patches look fine to me, otherwise.
Comment 20 Bastien Nocera 2012-09-18 22:56:46 UTC
Pushed all those patches by mistake while fixing another bug. Reopen if I should revert them.
Comment 21 Rui Matos 2012-09-21 19:47:14 UTC
Attachment 224631 [details] pushed as c8b4efd - region: Switch to the input sources section