On Tue, 14 Feb 2023 19:11:20 GMT, Damon Nguyen <[email protected]> wrote:

>> Before the fix, a JComboBox in Nimbus L&F would have normal black text even 
>> when the JComboBox was disabled if SynthComboBoxRenderer was replaced with a 
>> DefaultListCellRenderer. This text should be greyed out like in other L&F's. 
>> When looking into the defaults for Nimbus L&F files for attributes and 
>> states of a JComboBox, it confirm that the intention for disabled 
>> JComboBoxes is nimbusDisabledText (which is grey text).
>> 
>> SynthComboBoxes have an additional check in its default 
>> SynthComboBoxRenderer that enables/disables the renderer itself. The 
>> SynthComboBoxRenderer inherits its enabled state from the parent ComboBox. 
>> Since the renderer with DefaultListCellRenderer is in a separate class 
>> without a reference to the comboBox, a listener was added to SynthComboBoxUI.
>> 
>> An additional issue occurred in DefaultListCellRenderer because the renderer 
>> overrode the listener's re-assigned enabled state. In testing, setting the 
>> enabled state in DefaultListCellRenderer is redundant for all L&F's and is 
>> not needed here. However, instead of removing it altogether, a conditional 
>> was added specifically to allow ComboBoxes to skip setting enabled state 
>> here.
>> 
>> After the fix, the Nimbus JComboBox with DLCR set matches the appearance of 
>> a normal Nimbus JComboBox. I can enable/disable the JComboBoxes in the test, 
>> and the UI elements behave and appear as expected.
>
> Damon Nguyen has updated the pull request incrementally with four additional 
> commits since the last revision:
> 
>  - Remove extra bufferedImage
>  - Fix crlf
>  - Add automated test. Cycle thru all LAF
>  - Add testing for other LAFs

test/jdk/javax/swing/JComboBox/DisabledComboBoxFontTestAuto.java line 100:

> 98:         ImageIO.write(disabledImage2, "png", new File(testDir
> 99:                 + "/" + lafName + "DisabledDLCR.png"));
> 100:         System.out.println("DIR: " + testDir);

Is this needed? I know it is just a test but please remove the debug output 
when not needed.

test/jdk/javax/swing/JComboBox/DisabledComboBoxFontTestAuto.java line 173:

> 171:             
> SwingUtilities.invokeAndWait(DisabledComboBoxFontTestAuto::createCombo);
> 172:             
> SwingUtilities.invokeAndWait(DisabledComboBoxFontTestAuto::paintCombo);
> 173:             testMethod();

So test will fail whenever it encounters the first wrong color? Ideally we 
would run test for all the installed LAFs anyways and report which ones are 
failed. But in this case you would need to pass the LAF name to the test method 
so it saves screenshots for all the LAFs for future analysis.

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

PR: https://git.openjdk.org/jdk/pull/12390

Reply via email to