RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v17]
Abhishek Kumar
abhiscxk at openjdk.org
Mon Jul 8 14:26:38 UTC 2024
On Mon, 8 Jul 2024 13:12:33 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:
>> Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision:
>>
>> wild import expand. SynthGraphicsUtils revert back
>
> Adding a brief details for the changes in PR as it got multiple commits and will be hard to track those changes.
>
> Initial proposed fix was to handle the **mnemonic show/hide on Alt press for GTK in Synth based classes** where
>
> - `SynthLookAndFeel` class installed the AltProcessor handler after the conditional check of current L&F is GTK. public APIs `setMnemonicHidden and isMnemonicHidden` were added as part of fix.
> - `SynthRootPaneUI` class is having the AltProcessor class to handle the press and release of ALT key press.
> - `SynthGraphicsUtils` class is responsible for rendering the mnemonics based on the `show/hide status of isMnemonicHidden flag` for GTK L&F.
>
> @prrace suggested that checking for GTK L&F is not an ideal way of implementation and we should look for other alternative mentioned [here](https://github.com/openjdk/jdk/pull/18992#discussion_r1595782003).
>
> Based on the suggestion, `AltPressProperty` was added in `GTKLookAndFeel` to ensure the ALT key press event is handled only for GTK and thus moving the installation / uninstallation of ALTProcessor handler code in SynthRootPaneUI class.
> **This fix required a CSR because of few public APIs which were added as a part of fix.**
>
> Later on @aivanov-jdk suggested to move the **public APIs in a separate class MnemonicHandler (under sun.swing package) as the code was duplicated among Windows/Mac/Linux OSes** and a separate `AltProcessor class to handle the Alt key press/release for GTK and Aqua L&F` (same logic was implemented for them), Windows is having it's own version of AltProcessor.
>
> Current implementation details are:
>
> - Duplicate code removed, helper methods are moved to one common file (MnemonicHandler.java)
> - Common AltProcessor handler class for GTK and Aqua (AltProcessor.java)
> - Refactoring code for Windows and Aqua to access the helper methods from MnemonicHandler class and remove unused code.
> - Mnemonic hidden condition check moved to `GTKGraphicsUtils` so that only GTK L&F will call `isMnemonicHidden` to avoid the performance issue.
> - No need of CSR as [sun.swing package is not public](https://github.com/openjdk/jdk/pull/18992#discussion_r1654417685)
>
> **The fix is extended for Labels and Buttons as well along with Menus for GTK L&F.**
> Further the wild imports were expanded wherever applicable.
>
> Automated test enhanced to test menu mnemonic behavior for Aqua L&F also. Few existing regression tests also modified to access the helper methods from MnemonicHandler class.
> > @kumarabhi006 Setting summary to:
>
> It's not what I meant. This summary will be included in the commit message, its purpose is to provide additional details of the changeset.
>
> Something like this should be enough:
>
> ```
> Hides mnemonics on menus, buttons, and labels for GTK L&F.
>
> Moved shared code for hiding mnemonics into
> sun/swing/MnemonicHandler and AltProcessor to avoid code duplication.
> ```
oh ok.. I misunderstood that. Updated now.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/18992#issuecomment-2214223498
More information about the client-libs-dev
mailing list