You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@netbeans.apache.org by "eirikbakke (via GitHub)" <gi...@apache.org> on 2023/01/20 22:34:46 UTC

[GitHub] [netbeans] eirikbakke opened a new pull request, #5336: Don't show the '+' separator in keyboard accelerators on MacOS

eirikbakke opened a new pull request, #5336:
URL: https://github.com/apache/netbeans/pull/5336

   Keyboard accelerator strings, such as "Ctrl+R", are shown to the user in various places throughout NetBeans, e.g. in editor toolbar tooltips. On MacOS, the traditional display format for these is to use a special single-character symbol for modifier keys, and no "+" symbol. E.g. "⌘R" for Command+R. On NetBeans, though, we currently show this as "⌘+R".
   
   This PR gets rid of the plus symbol on MacOS, to match the native convention. This is already how shortcuts are displayed in the main menu (where the accelerator display strings are generated by the OS).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] eirikbakke merged pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.

eirikbakke merged PR #5336:
URL: https://github.com/apache/netbeans/pull/5336


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] eirikbakke commented on pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.

eirikbakke commented on PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#issuecomment-1484238035

Thanks for these comments! I have pushed an improved version of this PR now, which gathers the required logic in a single new public API method Actions.keyStrokeToString, and uses it in more places. This includes the editor toolbar tooltips.

In the Keymap settings code, changing accelerator strings is a bit risky, since there may be parsing code that assumes the old format. So I've left the options.keymap module untouched for now.

There are also a few other places calling KeyEvent.getKeyText/getKeyModifiersText where presentation strings are generated in more complicated ways; I have left these for future work.

> KeyEvent.getKeyModifiersText() and InputEvent.getModifiersExText() on macOS have the problem that the order of the modifier symbols is different to the order used by macOS

Fixed, by using logic similar to the one you linked to in FlatLAF.

> BTW would suggest to replace deprecated KeyEvent.getKeyModifiersText() with InputEvent.getModifiersExText().

Done.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] eirikbakke commented on a diff in pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.

eirikbakke commented on code in PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#discussion_r1150656139


##########
platform/openide.awt/src/org/openide/awt/Actions.java:
##########
@@ -128,25 +129,100 @@ private static String findKey(Action action) {
             return null;
         }
 
-        KeyStroke accelerator = (KeyStroke) action.getValue(Action.ACCELERATOR_KEY);
+        KeyStroke stroke = (KeyStroke) action.getValue(Action.ACCELERATOR_KEY);
 
-        if (accelerator == null) {
+        if (stroke == null) {
             return null;
         }
 
-        int modifiers = accelerator.getModifiers();
-        String acceleratorText = ""; // NOI18N
+        return keyStrokeToString(stroke);
+    }
 
-        if (modifiers > 0) {
-            acceleratorText = KeyEvent.getKeyModifiersText(modifiers);
-            acceleratorText += "+"; // NOI18N
-        } else if (accelerator.getKeyCode() == KeyEvent.VK_UNDEFINED) {
-            return ""; // NOI18N
+    // Based on com.formdev.flatlaf.getMacOSModifiersExText.

Review Comment:
   Meant com.formdev.flatlaf.ui.FlatMenuItemRenderer.getMacOSModifiersExText here in the comment... will fix before merging.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] eirikbakke commented on pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.

eirikbakke commented on PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#issuecomment-1501222365

   Thanks for reviewing, @DevCharly! Will wait a few days to see if there are any other comments since there is an API change involved. (One utility method added at Actions.keyStrokeToString.)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] eirikbakke commented on pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.

eirikbakke commented on PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#issuecomment-1509405137

   I pushed a change to fix the typo "com.formdev.flatlaf.getMacOSModifiersExText." to "com.formdev.flatlaf.FlatMenuItemRenderer.getMacOSModifiersExText" in a comment. Otherwise no changes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] mbien commented on pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "mbien (via GitHub)" <gi...@apache.org>.

mbien commented on PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#issuecomment-1509936861

   this has no milestone assigned. If you want to see this in NB 18, now would be a good time to merge :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] eirikbakke commented on pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.

eirikbakke commented on PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#issuecomment-1509975835

   Great; merged!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

[GitHub] [netbeans] DevCharly commented on pull request #5336: Don't show the '+' separator in keyboard accelerators on MacOS

Posted by "DevCharly (via GitHub)" <gi...@apache.org>.

DevCharly commented on PR #5336:
URL: https://github.com/apache/netbeans/pull/5336#issuecomment-1429553540

   `KeyEvent.getKeyModifiersText()` and `InputEvent.getModifiersExText()` on macOS have the problem that the order of the modifier symbols is different to the order used by macOS (left is Java, right is macOS:
   
   ![Untitled](https://user-images.githubusercontent.com/5604048/218698550-06ea3af3-6671-4b7e-8881-0b6530ad0332.png) ![Untitled 2](https://user-images.githubusercontent.com/5604048/218698517-47a6408e-d7bd-4263-9ace-1895230116bd.png) 
   
   That's why FlatLaf and Aqua L&F use own methods.
   See: [FlatMenuItemRenderer.getMacOSModifiersExText()](https://github.com/JFormDesigner/FlatLaf/blob/546382e47170aa5b7132620b2cba32583d6011cc/flatlaf-core/src/main/java/com/formdev/flatlaf/ui/FlatMenuItemRenderer.java#L621-L644) and [AquaMenuPainter.getKeyModifiersUnicode()](https://github.com/openjdk/jdk/blob/92474f13f03afacc48541b0de17998998f70eb65/src/java.desktop/macosx/classes/com/apple/laf/AquaMenuPainter.java#L91-L126)
   
   For right-to-left component orientation, Aqua L&F (and FlatLaf) reverses the order. E.g. Swing Aqua L&F:
   
   ![Untitled 7](https://user-images.githubusercontent.com/5604048/218707775-d20e0cf2-bcbf-4d56-8b6d-adc677461607.png)
   
   BTW would suggest to replace deprecated `KeyEvent.getKeyModifiersText()` with `InputEvent.getModifiersExText()`.
   
   Tried this out. I see the change in the main toolbar, but not in editor/view toolbars and other areas. I fear that this kind of code is used in many places. It is also surprising that text "Ctrl" or "Shift" (instead of symbols) is used in some places on macOS.
   
   Editor toolbar:
   
   ![Untitled 3](https://user-images.githubusercontent.com/5604048/218715607-4a27cd88-6cb3-4fb0-9b2d-fff95fd3fccb.png)
   
   Options Keymap:
   
   ![Untitled 5](https://user-images.githubusercontent.com/5604048/218715717-efcf1dcd-e369-4c74-a03a-85f0eebc8f3e.png)
   
   Editor macro shortcut:
   
   ![Untitled 6](https://user-images.githubusercontent.com/5604048/218715726-2591d501-d594-47d2-ac28-b03957ba8e99.png)
   
   Editor popup:
   
   ![Untitled 4](https://user-images.githubusercontent.com/5604048/218716355-ac14b0de-9f70-4623-906f-1dd8958b1772.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists