RFR: 8348760: RadioButton is not shown if JRadioButtonMenuItem is rendered with ImageIcon in WindowsLookAndFeel [v4]

[Alexey Ivanov]

On Thu, 30 Jan 2025 09:59:33 GMT, Prasanta Sadhukhan <psadhukhan at 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:
> 
>   optimise

I am also for generating an `ImageIcon` on the fly _to avoid adding a binary file_.

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

> 885:                 }
> 886:                 if (icon != null) {
> 887:                     if (!((AbstractButton) c).isSelected()) {

This new block of code 887–918 copies that from above 855–884.

You should create a helper method instead copying the same code in the two branches of code, or the code should be re-arranged somehow.

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

> 889:                         Part part;
> 890:                         if (type == JRadioButtonMenuItem.class) {
> 891:                             part = Part.BP_RADIOBUTTON;

The part  `Part.BP_RADIOBUTTON` doesn't look right to me.

The selected case above always uses the part `Part.MP_POPUPCHECK` — and it looks correct because this code paints a menu item (and radio or check menu items are usually in popup menus rather than on the menu bar).

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

> 892:                         } else {
> 893:                             part = Part.MP_POPUPCHECK;
> 894:                         }

Suggestion:

                        Part part = (type == JRadioButtonMenuItem.class)
                                    ? Part.BP_RADIOBUTTON
                                    : Part.MP_POPUPCHECK;

Moreover, you use this syntax below.

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

> 905:                                     (type == JRadioButtonMenuItem.class)
> 906:                                             ? State.BULLETDISABLED
> 907:                                             : State.CHECKMARKDISABLED;

Suggestion:

                            state = (type == JRadioButtonMenuItem.class)
                                    ? State.BULLETDISABLED
                                    : State.CHECKMARKDISABLED;

For consistency with the code in the block above `else`.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23324#pullrequestreview-2583633590
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1935463611
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1935467489
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1935452320
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1935454098