Bug 197639

Don't seek for a workaround, provide an API improvement. Check for xxx24_checked.png and if present used it while the checkbox is checked.

After looking at the code, here's a rough outline that seems to fit with how things are partitioned. All in package org.openide.awt. Comments?

- In Actions.ButtonBridge, detect a AlwaysEnabledAction$CheckBox. If a xxx24_checked.png is a available, then invoke
        CheckBox.connect(button, defaultIcon, checkedIcon).

- CheckBox.connect(button, defaultIcon, checkedIcon)
    Saves the button and icons.
    If either icon is null, then forget button and both icons.
        class ButtonIcons { button, defaultIcon, checkedIcon }
    so only one pointer needed.
    If there might be more than one button ArrayList<ButtonIcons>

- In CheckBox.updateItemsSelected add a
        if(button != null) {
            do button.setIcon(selected ? checkedIcon, defaultIcon);
        }

That should be ButtonBridge.updateButtonIcon where CheckBox.connect is called from. I assume updateButtonIcon always gets called at least once.

Right all the changes should be in openide.awt module. I was also expecting the change to be done in ButtonBridge.updateButtonIcon. Add something like:

button.setCheckedIcon(icon) 

as that method probably does not exist, then do button.putClientProperty("checkedIcon", icon). The CheckBox action would need then to implement Presenter.Toolbar and create a button subclass that would react to this property (remember that there can be multiple toolbar buttons for a single action).

Reopen when you have an API change patch.

Created attachment 107795 [details]
pick up the rest of the button icons

Fortunately, as I started implementing the button subclass, I discovered that "selected" (what we called checked) is handled by AbstractButton. There are three related icons.

In the comment I considered making a <ul> for the methods/icons. The WeakSet is a little heavy, but it gets created on demand and the code is simple.

I just noticed Toolbar.DefaultIconButton. Probably should use that to return from CheckBox.getToolbarPresenter? Since it's not public, just copy it into AlwaysEnabledAction?

I'm putting together pngs to test all the combos. There seems to be an opacity issue with the background or something with some of the icons. I'll dig into that this week.

Is there any particular unit test you think is needed?

The patch looks simple, that is good.

Y01 versioning: increment spec version in manifest, write apichanges.xml entry, in javadoc describe since when the behavior works

The DefaultButton is in different module and openide.awt cannot have dependency on it.

The test should check that proper icons are really used and that the button changes them when it is selected and deselected.

Created attachment 107825 [details]
handle all button icons, use JToggleButton subclass

Created attachment 107826 [details]
module showing all the button icons

> test should check that proper icons are really used and that the button
> changes them when it is selected and deselected.

I have no idea how, in NB, this would be done in an automated fashion.

The attached module puts two buttons on the toolbar, named "toggle" and "disablable". The toggle button enables/disables the other button. With toggle you can see 5 of the icons.  The remaining two icons, disabled and disabledSelected, can be seen in the "disablable" icon.

The icons themselves are useful since they are initialed with the state they represent.

> Y01 versioning: increment spec version in manifest, write 
> apichanges.xml entry, in javadoc describe since when the behavior works

I believe the patch covers everything

Y02 You cannot use org/openide/loaders resources from openide.awt. Should you write the unit test, you'd find that out.

Y03 Re. "test" - have you looked at ActionsTest.testIconsAction? Just mimic that.

Created attachment 107924 [details]
patch with Y02,Y03 fix

> Y02 cannot use org/openide/loaders resources

copied gif to openide.awt

> Y03 Re. "test" 

- Copied existing icons to "_selected" version.
  Changed previously unreferenced pixel (upper right) to black for selected.
  Incorporated into existing tests.
- Made AlwaysEnabledAction.DefaultIconToggleButton package scope
  so can reference it from new test.

Created attachment 107925 [details]
patch with Y02,Y03 fix; against latest source

Rebuilt patch against today's main-silver (spec version had changed)

Created attachment 107928 [details]
patch with Y02,Y03 fix; all files and against latest source

try again, this time include manifest (pesky 'qref -s' option)

Is this stalled for any technical reason?

I'll apply the latest patch if things work. I am still waiting for 7.0.1 to be branched.

Applied as ergonomics#b99d9a6cf077, thanks for the contribution.

Integrated into 'main-golden', will be available in build *201105270401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/b99d9a6cf077
User: err@netbeans.org
Log: #197639: Support "selected" icons in Actions and Actions.checkbox