Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v15]
On Mon, 8 Jul 2024 09:17:55 GMT, Abhishek Kumar wrote: >> Abhishek Kumar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Javadoc style comment, unused import removed > >> The diff modifies the generic `SynthGraphicsUtils.paintText` so that no >> mnemonic is passed to `SwingUtilities2.drawStringUnderlineCharAt` if >> `isMnemonicHidden` returns `true`. >> >> This approach has _a performance impact_ on all UI text painting. The >> condition can be moved into `GTKGraphicsUtils` so that only GTK L&F will >> call `isMnemonicHidden`. > > Updated the code to handle the mnemonic for buttons as well as labels. > Mnemonic hide condition check moved to `GTKGraphicsUtils` to avoid the > `performance issue`. > @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. - PR Comment: https://git.openjdk.org/jdk/pull/18992#issuecomment-2214143609
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v15]
On Fri, 5 Jul 2024 11:12:50 GMT, Abhishek Kumar wrote: >> In GTK LAF, the menu mnemonics are always displayed which is different from >> the native behavior. In native application **(tested with gedit for normal >> buttons and tested with libreoffice for menu**), the menu mnemonics toggle >> on press of `ALT` key. Menu mnemonics are hidden initially and then toggles >> between show/hide on `ALT` press. >> Proposed fix is to handle the `ALT` key press for GTK LAF and mimic the >> native behavior. Fix is similar to the `ALT` key processing in Windows LAF. >> Automated test case is added to verify the fix and tested in Ubuntu and >> Oracle linux. >> >> CI testing is green and link attached in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Javadoc style comment, unused import removed > The diff modifies the generic `SynthGraphicsUtils.paintText` so that no > mnemonic is passed to `SwingUtilities2.drawStringUnderlineCharAt` if > `isMnemonicHidden` returns `true`. > > This approach has _a performance impact_ on all UI text painting. The > condition can be moved into `GTKGraphicsUtils` so that only GTK L&F will call > `isMnemonicHidden`. Updated the code to handle the mnemonic for buttons as well as labels. Mnemonic hide condition check moved to `GTKGraphicsUtils` to avoid the `performance issue`. - PR Comment: https://git.openjdk.org/jdk/pull/18992#issuecomment-2213485660
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v15]
On Fri, 5 Jul 2024 15:16:57 GMT, Alexey Ivanov wrote: >> Abhishek Kumar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Javadoc style comment, unused import removed > > src/java.desktop/share/classes/sun/swing/MnemonicHandler.java line 1: > >> 1: /* > > Git identifies this class as rename from `AquaMnemonicHandler` to > `MnemonicHandler`. > > Indeed, the files are very similar. Shall we preserve the original copyright > year: `(c) 2011, 2024`? I think it is good to preserve the original copyright year as it not considered as a **new file**. - PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1668284765
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v15]
On Fri, 5 Jul 2024 11:12:50 GMT, Abhishek Kumar wrote:
>> In GTK LAF, the menu mnemonics are always displayed which is different from
>> the native behavior. In native application **(tested with gedit for normal
>> buttons and tested with libreoffice for menu**), the menu mnemonics toggle
>> on press of `ALT` key. Menu mnemonics are hidden initially and then toggles
>> between show/hide on `ALT` press.
>> Proposed fix is to handle the `ALT` key press for GTK LAF and mimic the
>> native behavior. Fix is similar to the `ALT` key processing in Windows LAF.
>> Automated test case is added to verify the fix and tested in Ubuntu and
>> Oracle linux.
>>
>> CI testing is green and link attached in JBS.
>
> Abhishek Kumar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Javadoc style comment, unused import removed
Marked as reviewed by aivanov (Reviewer).
src/java.desktop/share/classes/sun/swing/MnemonicHandler.java line 1:
> 1: /*
Git identifies this class as rename from `AquaMnemonicHandler` to
`MnemonicHandler`.
Indeed, the files are very similar. Shall we preserve the original copyright
year: `(c) 2011, 2024`?
test/jdk/javax/swing/JMenuBar/TestMenuMnemonicLinuxAndMac.java line 59:
> 57: UIManager.getInstalledLookAndFeels()) {
> 58: if (laf.getName().contains("GTK") ||
> laf.getName().contains("Aqua")) {
> 59: System.out.println("Testing: "+laf.getName());
Suggestion:
System.out.println("Testing: " + laf.getName());
Spaces around the binary operator `+`.
-
PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2160915917
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666923069
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666925212
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v15]
> In GTK LAF, the menu mnemonics are always displayed which is different from > the native behavior. In native application **(tested with gedit for normal > buttons and tested with libreoffice for menu**), the menu mnemonics toggle on > press of `ALT` key. Menu mnemonics are hidden initially and then toggles > between show/hide on `ALT` press. > Proposed fix is to handle the `ALT` key press for GTK LAF and mimic the > native behavior. Fix is similar to the `ALT` key processing in Windows LAF. > Automated test case is added to verify the fix and tested in Ubuntu and > Oracle linux. > > CI testing is green and link attached in JBS. Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision: Javadoc style comment, unused import removed - Changes: - all: https://git.openjdk.org/jdk/pull/18992/files - new: https://git.openjdk.org/jdk/pull/18992/files/4aa6db65..81259a7b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18992&range=14 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18992&range=13-14 Stats: 12 lines in 5 files changed: 2 ins; 8 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18992.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18992/head:pull/18992 PR: https://git.openjdk.org/jdk/pull/18992
