On Tue, 28 Jan 2025 03:40:26 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
> When JRadioButtonMenuItem is called with imageIcon, then only imageIcon is > shown without radiobutton in WIndowsLookAndFeel as there was no provision of > drawing the radiobutton alongside icon. > If icon is not there, the radiobutton is drawn. Added provision of drawing > the radiobutton windows Skin even when imageIcon is present. src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 860: > 858: State state; > 859: if (isEnabled(c, null)) { > 860: backgroundState = State.NORMAL; Do you happen to know why `State.BITMAP` was originally here? I tried locating where it originated from, but can't pinpoint exactly why it's either `State.BITMAP` or `State.NORMAL` based on the icon being `null`. Curious only because we're completely eliminating it here and there might be a dependency. However, I haven't encountered any issues when testing yet. src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 882: > 880: } else { > 881: skin = xp.getSkin(c, part); > 882: skin.paintSkin(g, x - 3*OFFSET, y + > OFFSET, state); Suggestion: skin.paintSkin(g, x - 3 * OFFSET, y + OFFSET, state); test/jdk/javax/swing/JMenuItem/TestImageIconWithJRadioButtonMenuItem.java line 51: > 49: Verify that for JRadioButtonMenuItem with imageicon, > 50: radiobutton is been shown alongside the imageicon. > 51: If radiobutton is shown, test passes else fails."""; Suggestion: One JRadioButtonMenuItem is with imageicon and another one without imageicon. Verify that for JRadioButtonMenuItem with imageicon, radiobutton is been shown alongside the imageicon. If radiobutton is shown, test passes. Else fails."""; Just for consistency, I see theres one capital `i`. I guess you can also just title-case all of the UI components. Ex: ImageIcon and RadioButton. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1931734330 PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1931715704 PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1931731265