On Tue, 5 Mar 2024 06:08:00 GMT, Abhishek Kumar <[email protected]> wrote:
>> Menu mnemonic doesn't toggle between show and hide state when F10 is
>> pressed. Behavior is not similar to windows native application. Fix is to
>> ensure that menu mnemonic state toggles between show and hide.
>>
>> Can be verified with SwingSet2 application.
>> CI tests are green with the fix. Link posted in JBS.
>
> Abhishek Kumar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Review comment update
Looks good to me except for a couple minor comments.
test/jdk/javax/swing/JMenuBar/TestMenuMnemonic.java line 58:
> 56: public static void main(String[] args) throws Exception {
> 57:
> UIManager.setLookAndFeel("com.sun.java.swing.plaf.windows.WindowsLookAndFeel");
> 58: final int EXPECTED = 5;
I meant making it `private static final` outside of main.
I suggest something like this:
private static JFrame frame;
private static JMenuBar menuBar;
private static JMenu fileMenu;
private static final AtomicInteger mnemonicHideCount = new AtomicInteger(0);
private static final AtomicInteger mnemonicShowCount = new AtomicInteger(0);
private static final int EXPECTED = 5;
The blank lines separate the blocks of related fields.
test/jdk/javax/swing/JMenuBar/TestMenuMnemonic.java line 97:
> 95: if (WindowsLookAndFeel.isMnemonicHidden()) {
> 96: mnemonicHideCount.getAndIncrement();
> 97: // check if selection is cleared when mnemonics are hidden
We capitalise ‘C‘ since it starts a sentence:
Suggestion:
// Check if selection is cleared when mnemonics are hidden
See [a comment on the
subject](https://github.com/openjdk/jdk/pull/18067#discussion_r1511570584).
-------------
Marked as reviewed by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17961#pullrequestreview-1916657969
PR Review Comment: https://git.openjdk.org/jdk/pull/17961#discussion_r1512686263
PR Review Comment: https://git.openjdk.org/jdk/pull/17961#discussion_r1512689884
