On Mon, 8 Jul 2024 13:12:33 GMT, Abhishek Kumar <abhis...@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
