GNOME Bugzilla – Bug 694740
Improve shortcut preferences
Last modified: 2013-03-04 21:22:59 UTC
The mnemonic and accelerator shortcut options should be insensitive when the menubar is disabled because they have no effect in that case. The options should be in the General tab instead of the Keybindings tab because their behaviour is dependent on whether the menubar is enabled or disabled, which is set in the General tab so their sensitivity should be immediately apparent to the user. The Keybindings tab will contain only the Shortcut Keys, so it should be renamed to Keyboard Shortcuts which is the same string as is used for the corresponding menuitem in 3.6 and the rest of GNOME. The checkbox labels should reflect that the mnemonics and the menu accelerators are different and discrete from the keyboard shortcuts. At the moment, the labels use non-standard terms to describe mnemonics and menu accelerators, and the meanings are explained in the label as well. Instead of using non-standard terms and an explanation, the label should contain precise terms (as used in Gtk) and an explanation. There is no harm in using the precise terms because these are advanced options. The first, third and fourth patches do not break the UI freeze. I would like to see the second patch applied for this release because the current behaviour is not user friendly and is awkward to document in the help. I am happy to ask for a freeze break if you agree with the changes.
Created attachment 237437 [details] [review] patch 1: insensitive labels
Created attachment 237438 [details] [review] patch 2: move checkboxes to General tab
Created attachment 237439 [details] [review] patch 3: rename Keybindings to Keyboard Shortcuts
Created attachment 237440 [details] [review] patch 4: change label strings
Comment on attachment 237437 [details] [review] patch 1: insensitive labels The menubar option is a global option, and the menubar can be shown in a window even if the option is disabled, so these prefs still need to be settable.
Comment on attachment 237439 [details] [review] patch 3: rename Keybindings to Keyboard Shortcuts That's the tab label, right? I think two words is too long for it...
Created attachment 237446 [details] [review] patch two: move checkboxes to General tab (In reply to comment #5) > The menubar option is a global option, and the menubar can be shown in a window > even if the option is disabled, so these prefs still need to be settable. Thanks for explaining it to me, that makes sense and my solution is not the right one. I still think that it is a problem that those two options can be enabled but do not have any effect if the menubar is disabled for a specific terminal window. Unfortunately, I cannot think of a nice way around this right now. I have removed the extra indentation for the two strings in patch two as they are not dependent on show_menubar_button. Is this ok? (In reply to comment #6) > (From update of attachment 237439 [details] [review]) > That's the tab label, right? I think two words is too long for it... What about something like Shortcuts?
(In reply to comment #7) > Created an attachment (id=237446) [details] [review] > patch two: move checkboxes to General tab > > (In reply to comment #5) > > The menubar option is a global option, and the menubar can be shown in a window > > even if the option is disabled, so these prefs still need to be settable. > > Thanks for explaining it to me, that makes sense and my solution is not the > right one. I still think that it is a problem that those two options can be > enabled but do not have any effect if the menubar is disabled for a specific > terminal window. Unfortunately, I cannot think of a nice way around this right > now. Maybe just have some small "Note: ..." text explaining that these options only apply when the menubar is actually shown? > I have removed the extra indentation for the two strings in patch two as they > are not dependent on show_menubar_button. Is this ok? Ok. > (In reply to comment #6) > > (From update of attachment 237439 [details] [review] [details]) > > That's the tab label, right? I think two words is too long for it... > > What about something like Shortcuts? Ok.
Created attachment 237488 [details] [review] patch three: rename Keybindings to Shortcuts (In reply to comment #8) > (In reply to comment #7) > > I still think that it is a problem that those two options can be > > enabled but do not have any effect if the menubar is disabled for a specific > > terminal window. > > Maybe just have some small "Note: ..." text explaining that these options only > apply when the menubar is actually shown? That would be good compromise for now, I will come up with some text in the next couple of days. Freeze break has been granted, I will push patches 2-4 tomorrow.
Comment on attachment 237446 [details] [review] patch two: move checkboxes to General tab Pushed to master in commit 9df4780f5204c7f944bb34d3e4ff496ed491768f
Comment on attachment 237488 [details] [review] patch three: rename Keybindings to Shortcuts Pushed to master in commit 4cab756d56e327f4749dea5547ed0369810a31d1
Comment on attachment 237440 [details] [review] patch 4: change label strings Pushed to master in commit 6cc98352555fe6adf737ca530ab9a98c216b742a
This is FIXED, right?