[rfc][icedtea-web][policyeditor] Keyboard shortcuts and mnemonics touchup
Andrew Azores
aazores at redhat.com
Fri Jun 27 17:15:33 UTC 2014
On 06/27/2014 12:15 PM, Jacob Wisor wrote:
> On 06/27/2014 03:43 PM, Andrew Azores wrote:
>> On 06/27/2014 07:30 AM, Jacob Wisor wrote:
>>> On 06/26/2014 09:16 PM, Andrew Azores wrote:
>> […]
>>>> 4) Defining shortcut keys in Messages.properties is simpler (! This
>>>> one requires
>>>> help from translators to fix the key bindings. It's also not
>>>> entirely ideal
>>>> because the modifier masks are not configurable via
>>>> Messages.properties right
>>>> now either - anyone have ideas on how to do that more neatly than
>>>> having the
>>>> Messages actually specify eg "C-S" or "C-S-S" for
>>>> Ctrl-S/Ctrl-Shift-S per
>>>> shortcut, for example? This wouldn't really be so bad though, I guess,
>>>> especially since these strings would be defined solely by the
>>>> person writing the
>>>> translations, and never input by users)
>>>
>>> Have a look at javax.swing.KeyStroke.getKeyStroke(java.lang.String).
>>> The
>>> Strings that this method takes could easily be stored in a
>>> properties file,
>>> the format is relatively well defined, and it should also work for
>>> non-English
>>> alphabets. It is actually the exact same String that
>>> KeyStroke.toString()
>>> produces on a KeyStroke instance.
>>
>> Ah wow, I hadn't actually paid attention to that particular method.
>
> Because you're hasting instead of programming. What you do could be
> considered by many people — yes, including me — as dirty hacking
> instead of coding.
Gee, thanks for being a constructive member of this community.
>
>> That's pretty much exactly what I was hoping to find.
>
> Yes, it is particularly useful for accelerators, and not for mnemonics.
>
>>>> Coming up later will be accelerators/mnemonics for CustomPolicyViewer.
>>>
>>> Yeah, please do not confuse accelerators and mnemonics. Also, please
>>> keep the
>>> terminology apart.
>>
>> How do you figure I've confused them here?
>
> Because you seem to have no idea what the particular purpose of
> accelerators and mnemonics is. I was trying to explain it further
> down, but obviously I have failed.
You are wrong. I do know. You're just being extremely picky about it. I
have freely admitted many times on this mailing list that I have hardly
done any GUI work at all before, and I never pretended like these
patches were flawless. They are what I can put forth with the knowledge
I have at the time. I am always willing and eager to learn more, but I
do not like to learn from people when their comments are terse, harsh,
and low on suggestions on how to actual do things better.
>
>>> > +# Menu navigation (Alt-mask)
>>>
>>> This should rather say "mnemonics" than "Alt-mask"
>>>
>>> > +PEFileMenuMnemonic=F
>>> > +PEEditMenuMnemonic=E
>>> > +PEViewMenuMnemonic=V
>>> > +
>>> > +# Key shortcuts (Ctrl-mask)
>>>
>>> This should say "accelerators" than "Ctrl-mask".
>>>
>>> > +PEAddCodebaseMnemonic=N
>>> > +PERemoveCodebaseMnemonic=R
>>> > +PEOpenMenuItemMnemonic=O
>>> > +PESaveMenuItemMnemonic=S
>>> > +PEExitMenuItemMnemonic=Q
>>> > +PECustomPermissionsItemMnemonic=U
>>> > +PECopyCodebaseItemMnemonic=C
>>> > +PEPasteCodebaseItemMnemonic=V
>>> > +PECopyCodebaseToClipboardItemMnemonic=B
>>> > +
>>> > +# Key shortcuts (Ctrl-Shift-mask)
>>>
>>> These are accelerators too.
>>
>> The reason I've distinguished between Alt, Ctrl, and Ctrl-Shift here
>> is so that
>> multiple keys mapping to the same value here are not mistaken for
>> actually being
>> the same key binding. Although using getKeyStroke as above will make
>> that quite
>> obvious anyway, so then those comments can certainly be removed.
>
> If you understood the distinct concepts between accelerators and
> mnemonics my previous comment would have been clear.
I never said your comment wasn't clear.
>
>> […]
>>> > […]
>>> > @@ -935,14 +847,11 @@ public class PolicyEditor extends JPanel
>>> > * @param mnemonic the mnemonic to set
>>> > */
>>> > private static void setComponentMnemonic(final AbstractButton
>>> component, > final String mnemonic) {
>>> > - final int trig;
>>> > - try {
>>> > - trig = Integer.parseInt(mnemonic);
>>> > - } catch (final NumberFormatException nfe) {
>>> > - OutputController.getLogger().log(nfe);
>>> > + if (mnemonic.length() != 1 ||
>>> !mnemonic.matches("[a-zA-Z]")) {
>>>
>>> It is probably not a good idea to work with regular expressions
>>> here. For one,
>>> this expression does not cover alphabets besides English. For two,
>>> if you
>>> really want to use a regular expression in this case it should be a
>>> final
>>> static compiled pattern instead of a String.
>>
>> Hmm... fair enough.
>>
>>>
>>> > return;
>>> > }
>>> > - component.setMnemonic(trig);
>>> > + final char ch = (char) mnemonic.getBytes()[0];
>>> > + component.setMnemonic((int) ch);
>>> > }
>>> >
>>> > private static JMenuBar createMenuBar(final Window window, final
>>> > PolicyEditor editor) {
>>> > @@ -965,7 +874,7 @@ public class PolicyEditor extends JPanel
>>> >
>>> > final JMenuItem saveAsItem = new
>>> JMenuItem(R("PESaveAsMenuItem"));
>>> > setComponentMnemonic(saveAsItem,
>>> R("PESaveAsMenuItemMnemonic"));
>>> > -
>>> saveAsItem.setAccelerator(KeyStroke.getKeyStroke(saveAsItem.getMnemonic(),
>>> > ActionEvent.CTRL_MASK));
>>> > +
>>> saveAsItem.setAccelerator(KeyStroke.getKeyStroke(saveAsItem.getMnemonic(),
>>> > ActionEvent.CTRL_MASK | ActionEvent.SHIFT_MASK));
>>>
>>> This is NOT correct! Do not do this. Accelerators != mnemonics.
>>> Although
>>> accelerators' keys and mnemonics seem be equal in most cases in
>>> English, this
>>> is definitely not the case for many other languages, and in few
>>> cases not even
>>> in English. So, do not do this. Accelerators and mnemonics are two
>>> distinct
>>> concepts, so please do not mix them.
>>
>> I know they are two distinct things. What do you have in particular
>> against
>> setting the same character as the mnemonic and accelerator for an
>> action? What
>> do you propose as an alternative - would you prefer to have separate
>> keys for
>> each in Messages.properties?
>
> Again, if you weren't just hasting and have read my comment carefully
> you would perhaps understand that you cannot lump these concepts
> together. So, obviously I have to spell it out for you or spoon feed
> it to you.
>
> Accelerators are an extension or evolution of function keys
> (F1..F12...). They are supposed to directly invoke a specific
> *function* by pressing a combination of keys. Furthermore, they are
> global in scope, except for dialogs.
>
> Mnemonics are for *navigating* UI elements where usually keyboard
> navigation is the only input device or for people who cannot use a
> pointing device. Although the UI can be built to be fully tab key
> navigable, mnemonics have an advantage over tab keys which offers
> immediate navigation to a specific UI element. Lastly, mnemonics are
> _not_ supposed to invoke program functions directly, as opposed to
> accelerators, because they are local in scope.
>
> Furthermore, accelerators should be language agnostic where as
> mnemonics should not. Example: ALT+F4 is a standard accelerator on
> Windows for closing the current window or application. This
> accelerator is labeled as "Close" in English with a "C" mnemonic but
> is labeled "Schließen" in German with a "S" mnemonic. This approach
> also allows to use the same accelerators with different languages;
> which is what users want when roaming between different languages but
> using the same application.
>
> Another example are languages which use ideographic script systems.
> Just think about it, there is no key combination for Ctrl+の!
>
> This is basic UI design and building knowledge that should be taught
> during the first semester in college. Well, actually in any major
> programming language course. What do you learn at school???
Glad to see you're being mature here and not resorting to ad hominem.
>
>>> > saveAsItem.addActionListener(editor.saveAsButtonAction);
>>> > fileMenu.add(saveAsItem);
>>> > @@ -973,31 +882,47 @@ public class PolicyEditor extends JPanel
>>> > setComponentMnemonic(exitItem, R("PEExitMenuItemMnemonic"));
>>> >
>>> exitItem.setAccelerator(KeyStroke.getKeyStroke(exitItem.getMnemonic(),
>>> > ActionEvent.CTRL_MASK));
>>> > exitItem.addActionListener(editor.closeButtonAction);
>>> > - exitItem.addActionListener(new ActionListener() {
>>> > - @Override
>>> > - public void actionPerformed(final ActionEvent e) {
>>> > - window.dispose();
>>> > - }
>>> > - });
>>> > fileMenu.add(exitItem);
>>> > menuBar.add(fileMenu);
>>> >
>>> > final JMenu editMenu = new JMenu(R("PEEditMenu"));
>>> > setComponentMnemonic(editMenu, R("PEEditMenuMnemonic"));
>>> >
>>> > + final JMenuItem addNewCodebaseItem = new
>>> > JMenuItem(R("PEAddCodebase"));
>>> > + setComponentMnemonic(addNewCodebaseItem,
>>> R("PEAddCodebaseMnemonic"));
>>> > +
>>> >
>>> addNewCodebaseItem.setAccelerator(KeyStroke.getKeyStroke(addNewCodebaseItem
>>> > .getMnemonic(), ActionEvent.CTRL_MASK));
>>>
>>> Again, do not do this! Accelerator != mnemonic.
>>>
>>> > +
>>> addNewCodebaseItem.addActionListener(editor.addCodebaseButtonAction);
>>> > + editMenu.add(addNewCodebaseItem);
>>> > +
>>> > + final JMenuItem removeCodebaseItem = new
>>> > JMenuItem(R("PERemoveCodebase"));
>>> > + setComponentMnemonic(removeCodebaseItem,
>>> > R("PERemoveCodebaseMnemonic"));
>>> > +
>>> >
>>> removeCodebaseItem.setAccelerator(KeyStroke.getKeyStroke(removeCodebaseItem
>>> > .getMnemonic(), ActionEvent.CTRL_MASK));
>>>
>>> Again, do not do this! Accelerator != mnemonic.
>>>
>>> > +
>>> >
>>> removeCodebaseItem.addActionListener(editor.removeCodebaseButtonAction);
>>>
>>> > + editMenu.add(removeCodebaseItem);
>>> > +
>>> > + editMenu.addSeparator();
>>> > +
>>> > final JMenuItem renameCodebaseItem = new
>>> > JMenuItem(R("PERenameCodebaseItem"));
>>> > + setComponentMnemonic(renameCodebaseItem,
>>> > R("PERenameCodebaseItemMnemonic"));
>>> > +
>>> >
>>> renameCodebaseItem.setAccelerator(KeyStroke.getKeyStroke(renameCodebaseItem
>>> > .getMnemonic(), ActionEvent.CTRL_MASK | ActionEvent.SHIFT_MASK));
>>>
>>> Again, do not do this! Accelerator != mnemonic.
>>>
>>>
>>> >
>>> renameCodebaseItem.addActionListener(editor.renameCodebaseButtonAction);
>>>
>>> > editMenu.add(renameCodebaseItem);
>>> >
>>> > final JMenuItem copyCodebaseItem = new
>>> > JMenuItem(R("PECopyCodebaseItem"));
>>> > + setComponentMnemonic(copyCodebaseItem,
>>> > R("PECopyCodebaseItemMnemonic"));
>>> > +
>>> >
>>> copyCodebaseItem.setAccelerator(KeyStroke.getKeyStroke(copyCodebaseItem
>>> > .getMnemonic(), ActionEvent.CTRL_MASK));
>>>
>>> Again, do not do this! Accelerator != mnemonic.
>>>
>>> > copyCodebaseItem.addActionListener(editor.copyCodebaseButtonAction);
>>> > editMenu.add(copyCodebaseItem);
>>> >
>>> > final JMenuItem pasteCodebaseItem = new
>>> > JMenuItem(R("PEPasteCodebaseItem"));
>>> > + setComponentMnemonic(pasteCodebaseItem,
>>> > R("PEPasteCodebaseItemMnemonic"));
>>> > +
>>> >
>>> pasteCodebaseItem.setAccelerator(KeyStroke.getKeyStroke(pasteCodebaseItem
>>> > .getMnemonic(), ActionEvent.CTRL_MASK));
>>>
>>> Again, do not do this! Accelerator != mnemonic.
>>>
>>> >
>>> pasteCodebaseItem.addActionListener(editor.pasteCodebaseButtonAction);
>>> > editMenu.add(pasteCodebaseItem);
>>> >
>>> > final JMenuItem copyCodebaseToClipboardItem = new
>>> > JMenuItem(R("PECopyCodebaseToClipboardItem"));
>>> > + setComponentMnemonic(copyCodebaseToClipboardItem,
>>> > R("PECopyCodebaseToClipboardItemMnemonic"));
>>> > +
>>> > copyCodebaseToClipboardItem.setAccelerator(KeyStroke
>>> > .getKeyStroke(copyCodebaseToClipboardItem.getMnemonic(),
>>> > ActionEvent.CTRL_MASK));
>>>
>>> Again, do not do this! Accelerator != mnemonic.
>>
>> Please do not do this non-helpful comment repetition. If you are so
>> averse to it
>> then please provide an actual alternative suggestion rather than just
>> saying
>> "No, this sucks, never do it ever" for half the length of your mail.
>
> It does suck when people do not think... :-/
>
> Jacob
>
Please stop reviewing my patches if you aren't going to be useful or at
least not derogatory.
--
Andrew A
More information about the distro-pkg-dev
mailing list