On Thu, 22 Jun 2023 03:44:19 GMT, Prasanta Sadhukhan <[email protected]> 
wrote:

>> Title buttons under Widows Classic L&F got their sizes from the XP desktop 
>> theme in which button width can be bigger than height. It is construed as XP 
>> bug where sizes aren't updated properly so it uses height units for width 
>> for XP and later windows. The proposed fix uses the [same 
>> technique](https://github.com/openjdk/jdk/blob/a0595761ef35c4eec8cb84326a869b9473cd5bba/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsInternalFrameTitlePane.java#L78-L82)
>>  for Classic and forces title buttons to be square and to fit the frame 
>> title in height.
>> 
>> Before fix SwingSet2 demo (Windows Classic InternalFrame)
>> ![image](https://github.com/openjdk/jdk/assets/43534309/3d6ec539-5e17-46ce-aba5-e724af6085fe)
>> 
>> After fix
>> ![image](https://github.com/openjdk/jdk/assets/43534309/db0135e4-d7f3-41a4-bbfa-e8e95cbd071d)
>
> Prasanta Sadhukhan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Review comments

We have exactly the same code in both `WindowsIconFactory.java` and 
`WindowsInternalFrameTitlePane.java`. Can it moved into a helper method to 
avoid duplication?

Would be enough to have it in `WindowsIconFactory.java` only?

Can `WindowsInternalFrameTitlePane.java` rely on the value set to UI Manager?

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java
 line 297:

> 295:                 }
> 296:             } else {
> 297:                 width = 
> UIManager.getInt("InternalFrame.titleButtonHeight") - 2;

Suggestion:

                width = UIManager.getInt("InternalFrame.titleButtonHeight") -2;

I agree it looks better with the space on either side but it's consistently 
used without the space after `-`. So I'd rather keep the original formatting.

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

PR Review: https://git.openjdk.org/jdk/pull/14555#pullrequestreview-1493939458
PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1238992267

Reply via email to