+1
>
>
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 3/3/2017 8:55 PM, Alexandr Scherbatiy wrote:
>>
>> I have uploaded the webrev to the http://cr.openjdk.java.net
>> <http://cr.openjdk.java.net/>:
>> http://cr.openjdk.java.net/~alexsch/mraz.martin/6490753/webrev.01
>> <http://cr.openjdk.java.net/%7Ealexsch/mraz.martin/6490753/webrev.01>
>>
>> Thanks,
>> Alexandr.
>>
>> On 2/17/2017 2:41 PM, Martin M wrote:
>>> AnimationController.java
>>> 386 if (skin.haveToSwitchStates()) {
>>> 387 skin.paintSkinRaw(g, dx, dy, dw, dh, state);
>>> 388 g.setComposite(AlphaComposite.SrcOver.derive(1 -
>>> progress));
>>> 389 skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
>>> 390 } else {
>>>
>>> - Could the '1 - progress' value in AlphaComposite.SrcOver.derive(1 -
>>> progress) be out of range?
>>>
>>> Value 'progress' is checked in method 'private void updateProgress()' so it
>>> always is between 0 and 1.
>>>
>>>
>>> WindowsComboBoxUI.java
>>> 221 if (this.borderChecked == false) // check border of
>>> component
>>> 222 replaceBorder();
>>>
>>> It is better to use 'if (!this.borderChecked) {replaceBorder();}'.
>>> There are Java Style Guidelines [1] which we need to follow to.
>>>
>>> I removed this part and replaced it with one line into method 'public void
>>> installUI( JComponent c )'. It has the same effect.
>>> WindowsComboBoxUI.java
>>> 159 // set empty border as default to see vista animated border
>>> 160 comboBox.setBorder(new EmptyBorder(0,0,0,0));
>>>
>>>
>>> 454 if (comboBox.isPopupVisible())
>>> 455 getModel().setPressed(true);
>>> 456 else
>>> 457 getModel().setPressed(false);
>>>
>>> This can be simplified to getModel().setPressed(comboBox.isPopupVisible()).
>>>
>>> Corrected. this was lame :(
>>>
>>>
>>> 508 @Deprecated
>>> 509 @SuppressWarnings("serial") // Superclass is not serializable
>>> across versions
>>> 510 protected class WindowsComboPopup extends BasicComboPopup {
>>> 511
>>> 512 private class comboPopUpBorder extends EmptyBorder {
>>>
>>> The class WindowsComboPopup is marked as deprecated with comments '* This
>>> class is now obsolete and doesn't do anything.'
>>> Is it possible to move the popup border class outside and do not use the
>>> WindowsComboPopup at all?
>>> The comboPopUpBorder class name should start from capital char.
>>>
>>> I removed comboPopUpBorder class because painting of this border was
>>> hardcoded and it looked the same in win7 and win10.
>>> The border is now painted with system skin and looks properly in both
>>> windows versions.
>>> And I am not sure if it is ok, but I created new class WinComboPopUp which
>>> extends BasicComboPopup
>>> to avoid using deprecated WindowsComboPopup. But deprecated class also
>>> extends BasicComboPopup. So....
>>>
>>>
>>> 566 Border border =
>>> (Border)UIManager.get("ComboBox.editorBorder");
>>> 567
>>> 568 // correct space on the left side of text (for editable
>>> combobox)
>>> 569 Insets i = border.getBorderInsets(editor);
>>> 570 border = BorderFactory.createEmptyBorder(i.top, 4,
>>> i.bottom, i.right);
>>> 571
>>> 572 if (border != null) {
>>> 573 editor.setBorder(border);
>>> 574 }
>>>
>>> - The border.getBorderInsets(editor) is called before checking the border
>>> to null.
>>> - It seems that if a user sets a custom "ComboBox.editorBorder" it should
>>> not be changed.
>>> Is it possible just to update the dafult "ComboBox.editorBorder" in the
>>> com.sun.java.swing.plaf.windows.WindowsLookAndFeel class?
>>>
>>> I updated default ComboBox.editorBorder.
>>>
>>>
>>> XPStyle.java
>>> 517 public boolean haveToSwitchStates() {
>>> 518 return switchStates;
>>> 519 }
>>> 520
>>> 521 public void switchStates(boolean b) {
>>> 522 switchStates = b;
>>> 523 }
>>>
>>> Could you change the methods access from the public to package? Making some
>>> API public, even in com.sun.* package may require some additional process.
>>>
>>> I changed access modifier from public to package.
>>>
>>> And I also decreased shifting of list items to right from 3px to 2px.
>>> If I see correctly native comboBox items have 2 px intendation from left
>>> side of the border, when system font is used.
>>> WindowsComboBoxUI.java
>>> 600 return new Insets(0,2,0,0); (previous value
>>> was 3)
>>> 609 setBorder(new EmptyBorder(0, 2, 0, i.right)); (previous value was 3
>>> as well)
>>>
>>> br
>>> Martin
>>>
>>>
>>>
>>> 2017-02-03 13:31 GMT+01:00 Alexandr Scherbatiy
>>> <alexandr.scherba...@oracle.com <mailto:alexandr.scherba...@oracle.com>>:
>>> On 2/1/2017 3:41 PM, Martin M wrote:
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-6490753
>>>> <https://bugs.openjdk.java.net/browse/JDK-6490753>
>>>> webrev: http://cr.openjdk.java.net/~alexsch/mraz.martin/6490753/webrev.00
>>>> <http://cr.openjdk.java.net/%7Ealexsch/mraz.martin/6490753/webrev.00>
>>>>
>>>> Problem description:
>>>> Swing JComboBox looks different than native one in states like e.g.
>>>> focused state or mouse rollover state and so on.
>>>>
>>>> The fix solves following issues:
>>>> - Editable combobox border is regular dark rectangle in all states now.
>>>> After the fix border will have light gray color in normal state, light
>>>> blue in rollover or hot state and dark blue in pressed or focused state.
>>>> - Editable combobox button is painted almost properly, but not animated as
>>>> it should be. The fix will correct animation of the button.
>>>> - popup with list of choices has black border. The fix will correct the
>>>> border to proper colors.
>>>> - All texts were rendered very close to left side of borders. The fix
>>>> shifts texts few screen points to the left.
>>>
>>> AnimationController.java
>>> 386 if (skin.haveToSwitchStates()) {
>>> 387 skin.paintSkinRaw(g, dx, dy, dw, dh, state);
>>> 388 g.setComposite(AlphaComposite.SrcOver.derive(1 -
>>> progress));
>>> 389 skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
>>> 390 } else {
>>>
>>> - Could the '1 - progress' value in AlphaComposite.SrcOver.derive(1 -
>>> progress) be out of range?
>>>
>>> WindowsComboBoxUI.java
>>> 221 if (this.borderChecked == false) // check border of
>>> component
>>> 222 replaceBorder();
>>>
>>> It is better to use 'if (!this.borderChecked) {replaceBorder();}'.
>>> There are Java Style Guidelines [1] which we need to follow to.
>>>
>>>
>>> 454 if (comboBox.isPopupVisible())
>>> 455 getModel().setPressed(true);
>>> 456 else
>>> 457 getModel().setPressed(false);
>>>
>>> This can be simplified to getModel().setPressed(comboBox.isPopupVisible()).
>>>
>>>
>>> 508 @Deprecated
>>> 509 @SuppressWarnings("serial") // Superclass is not serializable
>>> across versions
>>> 510 protected class WindowsComboPopup extends BasicComboPopup {
>>> 511
>>> 512 private class comboPopUpBorder extends EmptyBorder {
>>>
>>> The class WindowsComboPopup is marked as deprecated with comments '* This
>>> class is now obsolete and doesn't do anything.'
>>> Is it possible to move the popup border class outside and do not use the
>>> WindowsComboPopup at all?
>>> The comboPopUpBorder class name should start from capital char.
>>>
>>>
>>> 566 Border border =
>>> (Border)UIManager.get("ComboBox.editorBorder");
>>> 567
>>> 568 // correct space on the left side of text (for editable
>>> combobox)
>>> 569 Insets i = border.getBorderInsets(editor);
>>> 570 border = BorderFactory.createEmptyBorder(i.top, 4,
>>> i.bottom, i.right);
>>> 571
>>> 572 if (border != null) {
>>> 573 editor.setBorder(border);
>>> 574 }
>>>
>>> - The border.getBorderInsets(editor) is called before checking the border
>>> to null.
>>> - It seems that if a user sets a custom "ComboBox.editorBorder" it should
>>> not be changed.
>>> Is it possible just to update the dafult "ComboBox.editorBorder" in the
>>> com.sun.java.swing.plaf.windows.WindowsLookAndFeel class?
>>>
>>>
>>> XPStyle.java
>>> 517 public boolean haveToSwitchStates() {
>>> 518 return switchStates;
>>> 519 }
>>> 520
>>> 521 public void switchStates(boolean b) {
>>> 522 switchStates = b;
>>> 523 }
>>>
>>> Could you change the methods access from the public to package? Making some
>>> API public, even in com.sun.* package may require some additional process.
>>>
>>> [1] http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html
>>> <http://cr.openjdk.java.net/%7Ealundblad/styleguide/index-v6.html>
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>>
>>>
>>>
>>
>
