Bug 197639 - Actions.checkbox has no icon for "off" state
Actions.checkbox has no icon for "off" state
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Actions
7.0
PC Windows XP
: P3 (vote)
: 7.1
Assigned To: Jaroslav Tulach
issues@platform
: API_REVIEW_FAST
Depends on:
Blocks: 179047
  Show dependency treegraph
 
Reported: 2011-04-12 03:07 UTC by err
Modified: 2011-05-27 11:53 UTC (History)
3 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
pick up the rest of the button icons (5.11 KB, patch)
2011-04-17 04:49 UTC, err
Details | Diff
handle all button icons, use JToggleButton subclass (7.92 KB, patch)
2011-04-19 05:11 UTC, err
Details | Diff
module showing all the button icons (12.28 KB, text/plain)
2011-04-19 05:19 UTC, err
Details
patch with Y02,Y03 fix (17.44 KB, patch)
2011-04-24 19:44 UTC, err
Details | Diff
patch with Y02,Y03 fix; against latest source (17.09 KB, patch)
2011-04-24 20:09 UTC, err
Details | Diff
patch with Y02,Y03 fix; all files and against latest source (17.44 KB, patch)
2011-04-24 20:19 UTC, err
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description err 2011-04-12 03:07:22 UTC
My module has a Actions.checkbox defined with xml only; it appears in the Tools menu as a checkbox, this works fine. I drag it to the menu and get a button there from xxx24.png. Ater pressing it, the state toggles as expected.

I don't visual feedback from the toolbar button for whether the checkbox is on or off. I added xxx24_pressed.png, but that only appears while the mouse button is pressed. I tried copying the pressed.png to xxx24_disabled.png, but no change (not surprising).

I catch the preference for the checkbox change, is there some workaround to manually change the image? How do I get hold of the button? Should I provide a custom ButtonActionConnector and look for my Action and capture the button? How do I know its for the toolbar?
Comment 1 Jaroslav Tulach 2011-04-12 08:42:29 UTC
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.
Comment 2 err 2011-04-12 20:26:55 UTC
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);
        }
Comment 3 err 2011-04-12 20:35:58 UTC
That should be ButtonBridge.updateButtonIcon where CheckBox.connect is called from. I assume updateButtonIcon always gets called at least once.
Comment 4 Jaroslav Tulach 2011-04-13 07:02:54 UTC
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).
Comment 5 Jaroslav Tulach 2011-04-15 11:11:53 UTC
Reopen when you have an API change patch.
Comment 6 err 2011-04-17 04:49:05 UTC
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.
Comment 7 err 2011-04-17 05:07:31 UTC
I just noticed Toolbar.DefaultIconButton. Probably should use that to return from CheckBox.getToolbarPresenter? Since it's not public, just copy it into AlwaysEnabledAction?
Comment 8 err 2011-04-18 00:41:59 UTC
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?
Comment 9 Jaroslav Tulach 2011-04-18 09:59:13 UTC
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.
Comment 10 err 2011-04-19 05:11:13 UTC
Created attachment 107825 [details]
handle all button icons, use JToggleButton subclass
Comment 11 err 2011-04-19 05:19:03 UTC
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.
Comment 12 err 2011-04-19 05:20:27 UTC
> 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
Comment 13 Jaroslav Tulach 2011-04-21 17:39:16 UTC
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.
Comment 14 err 2011-04-24 19:44:35 UTC
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.
Comment 15 err 2011-04-24 20:09:59 UTC
Created attachment 107925 [details]
patch with Y02,Y03 fix; against latest source

Rebuilt patch against today's main-silver (spec version had changed)
Comment 16 err 2011-04-24 20:19:43 UTC
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)
Comment 17 err 2011-05-10 15:38:53 UTC
Is this stalled for any technical reason?
Comment 18 Jaroslav Tulach 2011-05-11 13:30:14 UTC
I'll apply the latest patch if things work. I am still waiting for 7.0.1 to be branched.
Comment 19 Jaroslav Tulach 2011-05-24 06:11:56 UTC
Applied as ergonomics#b99d9a6cf077, thanks for the contribution.
Comment 20 Quality Engineering 2011-05-27 11:53:45 UTC
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


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo