<Swing Dev> Review request for JDK-6490753 JComboBox doesn't look as native combobox in different states of component
Martin M
mraz.martin.dev at gmail.com
Fri Feb 17 11:41:41 UTC 2017
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("ComboBo
x.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>:
> On 2/1/2017 3:41 PM, Martin M wrote:
>
> bug: https://bugs.openjdk.java.net/browse/JDK-6490753
> webrev: http://cr.openjdk.java.net/~alexsch/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
>
> Thanks,
> Alexandr.
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170217/9fda88f8/attachment.html>
-------------- next part --------------
diff -r 7f92506142d1 src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/AnimationController.java
--- a/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/AnimationController.java Thu Feb 16 23:56:51 2017 +0300
+++ b/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/AnimationController.java Fri Feb 17 12:36:28 2017 +0100
@@ -36,7 +36,7 @@
import javax.swing.*;
-
+import sun.swing.UIClientPropertyKey;
import com.sun.java.swing.plaf.windows.TMSchema.State;
import static com.sun.java.swing.plaf.windows.TMSchema.State.*;
import com.sun.java.swing.plaf.windows.TMSchema.Part;
@@ -383,18 +383,25 @@
updateProgress();
if (! isDone()) {
Graphics2D g = (Graphics2D) _g.create();
- skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
- float alpha;
- if (isForward) {
- alpha = progress;
+ if (skin.haveToSwitchStates()) {
+ skin.paintSkinRaw(g, dx, dy, dw, dh, state);
+ g.setComposite(AlphaComposite.SrcOver.derive(1 - progress));
+ skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
} else {
- alpha = 1 - progress;
+ skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
+ float alpha;
+ if (isForward) {
+ alpha = progress;
+ } else {
+ alpha = 1 - progress;
+ }
+ g.setComposite(AlphaComposite.SrcOver.derive(alpha));
+ skin.paintSkinRaw(g, dx, dy, dw, dh, state);
}
- g.setComposite(AlphaComposite.SrcOver.derive(alpha));
- skin.paintSkinRaw(g, dx, dy, dw, dh, state);
g.dispose();
} else {
skin.paintSkinRaw(_g, dx, dy, dw, dh, state);
+ skin.switchStates(false);
}
}
boolean isDone() {
diff -r 7f92506142d1 src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/TMSchema.java
--- a/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/TMSchema.java Thu Feb 16 23:56:51 2017 +0300
+++ b/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/TMSchema.java Fri Feb 17 12:36:28 2017 +0100
@@ -121,6 +121,12 @@
LBP_LISTBOX(Control.LISTBOX, 0),
+ LBCP_BORDER_HSCROLL (Control.LISTBOX, 1),
+ LBCP_BORDER_HVSCROLL (Control.LISTBOX, 2),
+ LBCP_BORDER_NOSCROLL (Control.LISTBOX, 3),
+ LBCP_BORDER_VSCROLL (Control.LISTBOX, 4),
+ LBCP_ITEM (Control.LISTBOX, 5),
+
LVP_LISTVIEW(Control.LISTVIEW, 0),
PP_PROGRESS (Control.PROGRESS, 0),
@@ -343,6 +349,12 @@
stateMap.put(Part.HP_HEADERSORTARROW,
new State[] {SORTEDDOWN, SORTEDUP});
+ State[] listBoxStates = new State[] { NORMAL, PRESSED, HOT, DISABLED};
+ stateMap.put(Part.LBCP_BORDER_HSCROLL, listBoxStates);
+ stateMap.put(Part.LBCP_BORDER_HVSCROLL, listBoxStates);
+ stateMap.put(Part.LBCP_BORDER_NOSCROLL, listBoxStates);
+ stateMap.put(Part.LBCP_BORDER_VSCROLL, listBoxStates);
+
State[] scrollBarStates = new State[] { NORMAL, HOT, PRESSED, DISABLED, HOVER };
stateMap.put(Part.SBP_SCROLLBAR, scrollBarStates);
stateMap.put(Part.SBP_THUMBBTNVERT, scrollBarStates);
diff -r 7f92506142d1 src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsComboBoxUI.java
--- a/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsComboBoxUI.java Thu Feb 16 23:56:51 2017 +0300
+++ b/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsComboBoxUI.java Fri Feb 17 12:36:28 2017 +0100
@@ -41,6 +41,7 @@
import sun.swing.DefaultLookup;
import sun.swing.StringUIClientPropertyKey;
+import com.sun.java.swing.plaf.windows.WindowsBorders.DashedBorder;
/**
* Windows combo box.
@@ -97,6 +98,9 @@
} else if (source instanceof XPComboBoxButton) {
rv = ((XPComboBoxButton) source)
.getWindowsComboBoxUI().comboBox;
+ } else if (source instanceof JTextField &&
+ ((JTextField) source).getParent() instanceof JComboBox) {
+ rv = (JComboBox) ((JTextField) source).getParent();
}
return rv;
}
@@ -149,6 +153,8 @@
//is initialized after installListeners is invoked
comboBox.addMouseListener(rolloverListener);
arrowButton.addMouseListener(rolloverListener);
+ // set empty border as default to see vista animated border
+ comboBox.setBorder(new EmptyBorder(0,0,0,0));
}
}
@@ -224,6 +230,9 @@
state = State.DISABLED;
} else if (isPopupVisible(comboBox)) {
state = State.PRESSED;
+ } else if (comboBox.isEditable()
+ && comboBox.getEditor().getEditorComponent().isFocusOwner()) {
+ state = State.PRESSED;
} else if (isRollover) {
state = State.HOT;
}
@@ -242,7 +251,7 @@
skin = xp.getSkin(c, Part.CP_READONLY);
}
if (skin == null) {
- skin = xp.getSkin(c, Part.CP_COMBOBOX);
+ skin = xp.getSkin(c, Part.CP_BORDER);
}
skin.paintSkin(g, 0, 0, c.getWidth(), c.getHeight(), state);
}
@@ -368,7 +377,8 @@
}
protected ComboPopup createPopup() {
- return super.createPopup();
+ return new WinComboPopUp(comboBox);
+ //super.createPopup();
}
/**
@@ -414,8 +424,10 @@
@SuppressWarnings("serial") // Superclass is not serializable across versions
private class XPComboBoxButton extends XPStyle.GlyphButton {
+ private State prevState = null;
+
public XPComboBoxButton(XPStyle xp) {
- super(null,
+ super(comboBox,
(! xp.isSkinDefined(comboBox, Part.CP_DROPDOWNBUTTONRIGHT))
? Part.CP_DROPDOWNBUTTON
: (comboBox.getComponentOrientation() == ComponentOrientation.RIGHT_TO_LEFT)
@@ -428,18 +440,33 @@
@Override
protected State getState() {
State rv;
+
+ getModel().setPressed(comboBox.isPopupVisible());
+
rv = super.getState();
XPStyle xp = XPStyle.getXP();
if (rv != State.DISABLED
- && comboBox != null && ! comboBox.isEditable()
- && xp != null && xp.isSkinDefined(comboBox,
- Part.CP_DROPDOWNBUTTONRIGHT)) {
+ && comboBox != null && ! comboBox.isEditable()
+ && xp != null && xp.isSkinDefined(comboBox,
+ Part.CP_DROPDOWNBUTTONRIGHT)) {
/*
* for non editable ComboBoxes Vista seems to have the
* same glyph for all non DISABLED states
*/
rv = State.NORMAL;
}
+ if (rv == State.NORMAL && (prevState == State.HOT || prevState == State.PRESSED)) {
+ /*
+ * State NORMAL of combobox button cannot overpaint states HOT or PRESSED
+ * Therefore HOT state must be painted from alpha 1 to 0 and not as usual that
+ * NORMAL state is painted from alpha 0 to alpha 1.
+ */
+ skin.switchStates(true);
+ }
+ if (rv != prevState) {
+ prevState = rv;
+ }
+
return rv;
}
@@ -484,6 +511,38 @@
}
}
+ protected class WinComboPopUp extends BasicComboPopup {
+ private Skin listBoxBorder = null;
+ private XPStyle xp;
+
+ public WinComboPopUp(JComboBox combo) {
+ super(combo);
+ xp = XPStyle.getXP();
+ if (xp != null && xp.isSkinDefined(combo, Part.LBCP_BORDER_NOSCROLL)) {
+ this.listBoxBorder = new Skin(combo, Part.LBCP_BORDER_NOSCROLL);
+ this.setBorder(new EmptyBorder(1,1,1,1));
+ }
+ }
+
+ protected KeyListener createKeyListener() {
+ return new InvocationKeyHandler();
+ }
+
+ protected class InvocationKeyHandler extends BasicComboPopup.InvocationKeyHandler {
+ protected InvocationKeyHandler() {
+ WinComboPopUp.this.super();
+ }
+ }
+
+ protected void paintComponent(Graphics g) {
+ super.paintComponent(g);
+ if (this.listBoxBorder != null) {
+ this.listBoxBorder.paintSkinRaw(g, this.getX(), this.getY(),
+ this.getWidth(), this.getHeight(), State.HOT);
+ }
+ }
+ }
+
/**
* Subclassed to highlight selected item in an editable combo box.
@@ -498,6 +557,7 @@
protected JTextField createEditorComponent() {
JTextField editor = super.createEditorComponent();
Border border = (Border)UIManager.get("ComboBox.editorBorder");
+
if (border != null) {
editor.setBorder(border);
}
@@ -524,6 +584,31 @@
private static final Object BORDER_KEY
= new StringUIClientPropertyKey("BORDER_KEY");
private static final Border NULL_BORDER = new EmptyBorder(0, 0, 0, 0);
+
+ // Create own version of DashedBorder with more space on left side
+ private class WindowsComboBoxDashedBorder extends DashedBorder {
+
+ public WindowsComboBoxDashedBorder(Color color, int thickness) {
+ super(color, thickness);
+ }
+
+ public WindowsComboBoxDashedBorder(Color color) {
+ super(color);
+ }
+
+ @Override
+ public Insets getBorderInsets(Component c, Insets i) {
+ return new Insets(0,2,0,0);
+ }
+ }
+
+ public WindowsComboBoxRenderer() {
+ super();
+
+ // correct space on the left side of text items in the combo popup list
+ Insets i = getBorder().getBorderInsets(this);
+ setBorder(new EmptyBorder(0, 2, 0, i.right));
+ }
/**
* {@inheritDoc}
*/
@@ -542,7 +627,7 @@
if (index == -1 && isSelected) {
Border border = component.getBorder();
Border dashedBorder =
- new WindowsBorders.DashedBorder(list.getForeground());
+ new WindowsComboBoxDashedBorder(list.getForeground());
component.setBorder(dashedBorder);
//store current border in client property if needed
if (component.getClientProperty(BORDER_KEY) == null) {
diff -r 7f92506142d1 src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java
--- a/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java Thu Feb 16 23:56:51 2017 +0300
+++ b/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java Fri Feb 17 12:36:28 2017 +0100
@@ -672,7 +672,7 @@
"ComboBox.buttonHighlight", ControlHighlightColor,
"ComboBox.selectionBackground", SelectionBackgroundColor,
"ComboBox.selectionForeground", SelectionTextColor,
- "ComboBox.editorBorder", new XPValue(new EmptyBorder(1,2,1,1),
+ "ComboBox.editorBorder", new XPValue(new EmptyBorder(1,4,1,1),
new EmptyBorder(1,4,1,4)),
"ComboBox.disabledBackground",
new XPColorValue(Part.CP_COMBOBOX, State.DISABLED,
diff -r 7f92506142d1 src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/XPStyle.java
--- a/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/XPStyle.java Thu Feb 16 23:56:51 2017 +0300
+++ b/src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/XPStyle.java Fri Feb 17 12:36:28 2017 +0100
@@ -479,6 +479,7 @@
private final String string;
private Dimension size = null;
+ private boolean switchStates = false;
Skin(Component component, Part part) {
this(component, part, null);
@@ -513,6 +514,14 @@
return (insets != null) ? insets : new Insets(0, 0, 0, 0);
}
+ boolean haveToSwitchStates() {
+ return switchStates;
+ }
+
+ void switchStates(boolean b) {
+ switchStates = b;
+ }
+
private int getWidth(State state) {
if (size == null) {
size = getPartSize(part, state);
@@ -689,7 +698,7 @@
@SuppressWarnings("serial") // Superclass is not serializable across versions
static class GlyphButton extends JButton {
- private Skin skin;
+ protected Skin skin;
public GlyphButton(Component parent, Part part) {
XPStyle xp = getXP();
More information about the swing-dev
mailing list