On Fri, 28 Feb 2025 03:45:29 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.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Test rename
Ran test on windows 11 machine and the menu items layout seems ok to me.
Don't have windows 10 machine to test.
Few test changes are required for consistency.
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicMenuItemUI.java line
664:
> 662: if (UIManager.getLookAndFeel().getName().equals("Windows")
> 663: && (Integer.parseInt(System.getProperty("os.name").
> 664: replaceAll("[^0-9]", "")) >= 11)
Suggestion:
&& (Integer.parseInt(System.getProperty("os.name")
.replaceAll("[^0-9]", "")) >= 11)
May shift `. operator` on next line for continuation.
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 28:
> 26: * @bug 8348760
> 27: * @summary Verify if RadioButton is shown if JRadioButtonMenuItem
> 28: * is rendered with ImageIcon in WindowsLookAndFeel
JCheckBoxMenuItem is missing in summary. Should be rephrased.
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 32:
> 30: * @library /java/awt/regtesthelpers
> 31: * @build PassFailJFrame
> 32: * @run main/manual
> TestImageIconWithJRadioButtonMenuItemAndJCheckBoxMenuItem
File name is different than the class name, jtreg test failed with `Error.
can't find TestImageIconWithJRadioButtonMenuItemAndJCheckBoxMenuItem in test
directory or libraries`
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 38:
> 36: import java.awt.Graphics;
> 37: import java.awt.image.BufferedImage;
> 38: import javax.swing.AbstractButton;
May add a blank line between awt and swing imports.
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 47:
> 45:
> 46: import javax.swing.JMenu;
> 47: import javax.swing.JMenuBar;
Please sort the imports.
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 60:
> 58:
> 59: Clicking on the Menu will show a
> 60: RadioButtonMenuItem group with 3 radiobutton menuitems
Suggestion:
JRadioButtonMenuItem group with 3 radiobutton menuitems
keep it same as JCheckBoxMenuItem below.
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 61:
> 59: Clicking on the Menu will show a
> 60: RadioButtonMenuItem group with 3 radiobutton menuitems
> 61: and a JCheckBoxMenuItem group with 3 checkbox menuitems
For consistency, keep period everywhere else remove it.
Suggestion:
and a JCheckBoxMenuItem group with 3 checkbox menuitems.
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 65:
> 63: First radio button menuitem is selected with imageicon of a red
> square.
> 64: Second radiobutton menuitem is unselected with imageicon.
> 65: Third radiobutton menuItem is unselected without imageicon.
`Either radio button or radiobutton`, should be consistent
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 74:
> 72: a bullet is shown alongside the imageicon and
> 73: for first JCheckBoxMenuItem with imageicon
> 74: a checkmark is shown alongside the image icon.
for consistency
`either imageicon or image icon`
-------------
PR Review: https://git.openjdk.org/jdk/pull/23324#pullrequestreview-2658519093
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979982189
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979969302
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979963728
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979971880
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979970932
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979973758
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979966221
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979967125
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1979976178
