<Swing Dev> Review request for JDK-6490753 JComboBox doesn't look as native combobox in different states of component
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Fri Mar 3 17:55:28 UTC 2017
I have uploaded the webrev to the http://cr.openjdk.java.net:
http://cr.openjdk.java.net/~alexsch/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.scherbatiy at oracle.com <mailto:alexandr.scherbatiy at 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.
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170303/7ccb9fbc/attachment.html>
More information about the swing-dev
mailing list