On Mon, 10 Jul 2023 09:49:21 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) >>  >> >> After fix >>  > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Review comments In general, looks good except for the couple of comments. I also think the test can be simplified, more on this below. src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsInternalFrameTitlePane.java line 59: > 57: import javax.swing.UIManager; > 58: import javax.swing.plaf.basic.BasicInternalFrameTitlePane; > 59: import javax.swing.plaf.UIResource; I got a different list of imports: see [commit 389cd03](https://github.com/aivanov-jdk/jdk/commit/389cd03c5630e41bc20c60579d6a339590d36fbc). The list of imports should be sorted, but it's not sorted here. The imports from `java.beans` should be after `java.awt` rather inside `javax` packages. In OpenJDK, we decided to put internal packages below public packages, so `SwingUtilities2` goes below `javax.*` packages. There's still wildcard import: `TMSchema.*`. The [complete sorted list of imports](https://github.com/aivanov-jdk/jdk/blob/389cd03c5630e41bc20c60579d6a339590d36fbc/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsInternalFrameTitlePane.java#L28-L68) as produced by *Optimize Imports* command in IDEA. test/jdk/javax/swing/JInternalFrame/InternalFrameTitleButtonTest.java line 105: > 103: BasicInternalFrameTitlePane title = > 104: ((BasicInternalFrameTitlePane)((BasicInternalFrameUI) > 105: iframe.getUI()).getNorthPane()); You don't use the `BasicInternalFrameTitlePane` type, the cast can be removed. ------------- Changes requested by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14555#pullrequestreview-1522410250 PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1258473594 PR Review Comment: https://git.openjdk.org/jdk/pull/14555#discussion_r1258474781
