On Mon, 24 Jun 2024 07:21:49 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:

>>>Requesting the value of RootPane.altPress from UIManager each time 
>>>postProcessKeyEvent is called is inefficient, so you can store the value in 
>>>altProcessor when look and feel is installed.
>> 
>> I guess you are pointing out the code in SynthGraphicsUtils.paintText method 
>> where RootPane.altPress value is retrieved from UIManager each time. Storing 
>> the value in it's constructor doesn't reflect the correct value and is 
>> always `false`.
>
>> If RootPane.altPress can be changed dynamically, you can install a 
>> PropertyChangeListener to UIManager.
> 
> I think it can be changed but is it really required to handle it ?
> I mean why does a user change it dynamically ?

> In my initial fix, I added the `altProcessor` handler in 
> `SynthLookAndFeel.initialize` with condition check for GTK L&F. Phil has 
> suggested not to check for GTK L&F instead look for some alternate way like 
> mentioned 
> [here](https://github.com/openjdk/jdk/pull/18992#discussion_r1595782003).
> 
> So, the handler implementation is moved to `SynthRootPaneUI`.

Thus, out of Synth-derived Look and Feels, only GTK needs the feature. Can't 
the listener be installed in `GTKLookAndFeel.initialize`?

At the same time, if you install the listener in `SynthLookAndFeel`, other L&F 
based on Synth could also use this feature, thus your way is more flexible.

However, `SynthRootPaneUI` is still part of the base class… I still don't 
understand how it's different from what I'm proposing.

I'd like to avoid keeping a flag whether the listener is installed or not… But 
there could be no other way since not all Synth-based L&Fs enable hiding 
mnemonics.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1652996654

Reply via email to