Re: [9] Review request for 8051548 JColorChooser should have a way to disable transparency controls

[Alexander Scherbatiy]

 Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8051548/webrev.02/
On 4/20/2015 6:34 PM, Sergey Bylokhov wrote:
Hi, Alexander.
A few notes:
 - Does this property control the "enable/disable" status or 
"visible/invisible" status?
      Both. Setting invisible status means the getColor() returns color 
ignoring alpha value.
 - Why the string property is soo long? Can it be simplified? Probably 
it will be good to wrap it using new String()?
     The property is renamed to TRANSPARENCY_ENABLED_PROPERTY
 - Why SlidingSpinner.isVisible does not check the status of 
label/spinner?
     Slider, label and spinner should always have the same visible state.
 - What about a new constructor which was requested in the CR also?
    I have added the colorTransparencySelectionEnabled argument to the 
JColorChooser.showDialog() method and updated the test.

  Thanks,
  Alexandr.


On 20.04.15 18:10, Alexander Scherbatiy wrote:

 Hello,

 Could you review the updated fix:
    http://cr.openjdk.java.net/~alexsch/8051548/webrev.01/

    - the state changing check is added in the 
ColorPanel.setColorTransparencySelectionEnabled() method

 Thanks,
 Alexandr.

On 4/16/2015 11:27 PM, Phil Race wrote:

 210
 211     void setColorTransparencySelectionEnabled(boolean b) {
 212         spinners[model.getCount() - 1].setVisible(b);
 213         colorChanged();
 214     }

do you want to check if there's a state change here before firing 
the events ?

Other than that it looks OK to me.

-phil.

On 4/16/15 1:14 AM, Alexandr Scherbatiy wrote:
15.04.2015 22:22, Sergey Bylokhov пишет:
HI, Alexander.
What is the status of this fix?

The fix requires at least two reviewers.

Thanks,
Alexandr.


On 25.07.14 16:45, Alexander Scherbatiy wrote:

Hello,

Could you review the fix:
bug: https://bugs.openjdk.java.net/browse/JDK-8051548
webrev: http://cr.openjdk.java.net/~alexsch/8051548/webrev.00

The following public methods are added to the 
AbstractColorChooserPanel:
- setColorTransparencySelectionEnabled(boolean b)
- isColorTransparencySelectionEnabled()

The request to the public API change will be created after the 
technical review.

Thanks,
Alexandr.