<Swing Dev> [9] Review request for 8051548 JColorChooser should have a way to disable transparency controls

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Tue Apr 21 15:32:53 UTC 2015


  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.
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
>




More information about the swing-dev mailing list