On Tue, 25 Jun 2024 14:58:53 GMT, Alexey Ivanov <[email protected]> wrote:

>>> 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.

> > 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`.

No, I didn't refer to this particular case. I referred to my suggestion where 
you always add `altProcessor` as the keyboard listener and keep track of the 
value of the `RootPane.altPress` property.

Anyway this approach seems to be rejected.

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

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

Reply via email to