Skip to content

Dynamically add menu mnemonics to all menus #2382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 18, 2014
Merged

Conversation

GKFX
Copy link
Contributor

@GKFX GKFX commented Mar 1, 2014

Added and called Toolkit.setMenuMnemonics(JMenuItem...) and Toolkit.setMenuMnemonics(JMenuBar). These automatically set menu mnemonics based on the KDE, Windows, and Gnome human-interface guidelines, except on Mac, where as per Mac HIGs, it does nothing.
Really, there should be a Toolkit.setMenuMnemonics(JPopupMenu), but I couldn't make one, so I've called
Toolkit.setMenuMnemonics(firstItem, secondItem, thirdItem, ...) instead.
#51

GKFX added 2 commits February 25, 2014 19:51
Added Toolkit.setMenuMnemonics(JMenuItem...) and
Toolkit.setMenuMnemonics(JMenuBar).
@GKFX
Copy link
Contributor Author

GKFX commented Mar 1, 2014

Human Interface Guidelines

Mac

No mnemonics. (https://developer.apple.com/library/mac/documentation/Java/Conceptual/Java14Development/07-NativePlatformIntegration/NativePlatformIntegration.html)

Gnome

(https://developer.gnome.org/hig-book/3.10/hig-book.html#choosing-access-keys)

  • Assign access keys to the most frequently-used controls first. If it's not clear which controls will be the most frequently used, assign access keys from left to right, top to bottom (for Western locales).
  • Use the first letter of the label, or of one of its other words if it has more than one. If another letter provides a better association (e.g. "x" in Extra Large) however, consider using that letter instead.
  • If the first letter is not available, choose an easy to remember consonant from the label, for example, "p" in Replace.
  • If no such consonants are available, choose any available vowel from the label.
    If duplication of access keys in a window is unavoidable, you should still refrain from duplicating the access keys for any of these buttons that appear in the same window: OK, Cancel, Close, Apply or Help.
    Also, it is better not to assign access keys to "thin" letters (such as lowercase i or l), or letters with descenders (such as lowercase g or y) unless it is unavoidable. The underline does not show up very well on those characters in some fonts.
Microsoft

I couldn't find anything except Windows Mobile guidance, but it's sensible stuff anyway: (http://msdn.microsoft.com/en-us/library/bb158536.aspx)

Consider the following guidelines:

  • Choose the mnemonic in the following order of preference:
    1. Use the first letter of the command name unless another letter provides a better mnemonic association.
    2. If the first letter is already assigned to another command, use a letter that is as close as possible to the first letter of the command name.
    3. Use a distinctive consonant in the command name.
    4. Use a vowel in the command name.
  • Avoid duplication of mnemonic assignments or use of a letter that is difficult to see. Otherwise, choose the mnemonic in the following order of preference:
    1. Letter that is next to a letter with a descender.
    2. Letter with descender, such as p, g, q, or y.
    3. Letter that is narrow, such as i or l.
  • Do not assign mnemonics to commands that are not included on menus, such as the Finish, Cancel, or Next command buttons.
KDE

There's a massive list of global accelerators here (http://techbase.kde.org/Projects/Usability/HIG/Keyboard_Accelerators).

@aengelke
Copy link
Contributor

aengelke commented Mar 1, 2014

There is a syntax error in Toolkit.java:236. The line should be commented out.

@GKFX
Copy link
Contributor Author

GKFX commented Mar 1, 2014

@aengelke I compiled it! algorithmicAssaignment: is a label, for my continue statements. I used labeled continues because it was a tangled mess of if{if{if{else{if{if{if{else{if{if{if{else{}}}}}}}}}}} before.

@GKFX
Copy link
Contributor Author

GKFX commented Mar 9, 2014

OK, I've found the proper MS stuff: http://msdn.microsoft.com/en-us/library/windows/desktop/bb545460.aspx#accessKeys. There's a table of defaults, and then:

  • Prefer characters with wide widths, such as w, m, and capital letters.
  • Prefer a distinctive consonant or a vowel, such as "x" in "Exit."
  • Avoid using characters that make the underline difficult to see, such as (from most problematic to least problematic):
    • Characters that are only one pixel wide, such as i and l.
    • Characters with descenders, such as g, j, p, q, and y.
    • Characters next to a letter with a descender.
  • When assigning access keys in wizard pages, remember to reserve "B" for Back and "N" for Next.
  • When assigning access keys in property pages, remember to reserve "A" for Apply, if used.
    Menu access keys
  • Assign access keys to all menu items. No exceptions.
  • For dynamic menu items (such as recently used files), assign access keys numerically.
    Screen shot of menu items with numeric access keys
    • In this example, the Paint program in Windows assigns numeric access keys to recently used files.
  • Assign unique access keys within a menu level. You can reuse access keys across different menu levels.
  • Make access keys easy to find:
  • For the most frequently used menu items, choose characters at the beginning of the first or second word of the label, preferably the first character.
  • For less frequently used menu items, choose letters that are a distinctive consonant or a vowel in the label.

@@ -2671,6 +2670,10 @@ public void actionPerformed(ActionEvent e) {
}
});
this.add(referenceItem);

Toolkit.setMenuMnemonics(cutItem, copyItem, discourseItem,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than making all these messy local variables, why not just call Toolkit.setMenuMnemonics(this)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to, but I couldn't figure out how to get JMenuItems out of a JPopupMenu without dubious casting. If you know how to, feel free to do it.

@benfry
Copy link
Contributor

benfry commented Mar 30, 2014

Thanks for the patch—I've added a handful of comments but we should be able to get an alternate version of this incorporated soon.

* Disabled on Mac, per Apple guidelines.
* <tt>menu</tt> may contain nulls.
*
* Author: George Bateman. Initial work Myer Nore.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this involves someone else's work, are we sure that the code can be licensed as Processing code? (i.e. the license of the original source isn't incompatible)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They submitted it as a changed form of one of your files (Editor.java). That sounded like they were implying you had permission to take it as part of the project.
(http://processing.org/bugs/bugzilla/attachments/304 from http://processing.org/bugs/bugzilla/26.html)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just making sure it wasn't a KDE dev or something like that...

GKFX added 2 commits March 31, 2014 16:53
Null-check.
Spacing as requested
@GKFX
Copy link
Contributor Author

GKFX commented Mar 31, 2014

OK. Thanks for reading over it.

GKFX added 2 commits April 2, 2014 17:16
Javadoc bug
I just found out that jmi.getText() can be null, so I added null-checking.
@benfry
Copy link
Contributor

benfry commented Jul 30, 2014

We'll need to re-evaluate this once #2084 is integrated.

@GKFX
Copy link
Contributor Author

GKFX commented Jul 30, 2014

I designed this with internationalization in mind (but didn't test or work too hard on that aspect). Obviously it accepts arbitrary text in any language, and it will also only associate ASCII mnemonics, to ensure they can be typed (no ctrl+á). But I could have missed things so we'll need to double-check it all.

@GKFX
Copy link
Contributor Author

GKFX commented Aug 2, 2014

I have read the code again. I think it is internationalization ready, but I will test it in a few languages to check.

@GKFX GKFX changed the title Dynamically add menu mnemonics to all menus [wip] Dynamically add menu mnemonics to all menus Aug 13, 2014
…o mnem-patch

Not quite done yet - temporary commit.
Conflicts:
	app/src/processing/app/Editor.java
	app/src/processing/app/Mode.java
	app/src/processing/app/Toolkit.java
@GKFX
Copy link
Contributor Author

GKFX commented Aug 24, 2014

OK. I managed to implement Toolkit.setMenuMnemonics(JPopupMenu) in the end, which helped a lot with Java 3 mode integration. I tested it in German:
Fast perfekt!
Only problem here is that the Germans seem to use a smaller set of frequent letters, leaving Beenden without a mnemonic. It does have Ctrl+Q though, and there is no repeat of the problem in other menus.
In non-Latin alphabets, I'm out of my depth, and I can't possibly test anything, so the only mnemonics are on snippets of ASCII that do appear.
All Greek to me.

Merge menu mnemonics back with current code and integrate with PDE-X.
@GKFX
Copy link
Contributor Author

GKFX commented Aug 24, 2014

Only thing left is a bit of the sketch menu.

Added Toolkit.setMenuMnemsInside(); removed Editor.resetMenuMnemonics(); added mnemonics to sketches in Sketch menu.
@GKFX
Copy link
Contributor Author

GKFX commented Aug 25, 2014

@benfry Done!

@GKFX GKFX changed the title [wip] Dynamically add menu mnemonics to all menus Dynamically add menu mnemonics to all menus Aug 25, 2014
@GKFX GKFX mentioned this pull request Oct 28, 2014
@benfry
Copy link
Contributor

benfry commented Nov 14, 2014

Sorry, once more...

GKFX added 3 commits November 18, 2014 18:14
…o mnem-patch

Conflicts:
	pdex/src/processing/mode/experimental/DebugEditor.java
@GKFX
Copy link
Contributor Author

GKFX commented Nov 18, 2014

I fixed a little bug in #2834, so to avoid complications I've tagged it on the end of this, seeing as the issues are interlinked.

benfry added a commit that referenced this pull request Nov 18, 2014
Dynamically add menu mnemonics to all menus
@benfry benfry merged commit f895ad0 into processing:master Nov 18, 2014
@benfry
Copy link
Contributor

benfry commented Nov 18, 2014

Thanks... merging this now so it can have some time to bake as we prepare for the next release. Thanks very much for your help and for keeping the patch up to date.

@GKFX GKFX deleted the mnem-patch branch July 3, 2015 17:29
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants