<Swing Dev> Review request for JDK-6490753 JComboBox doesn't look as native combobox in different states of component

[Alexandr Scherbatiy]

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

Thanks,
Alexandr.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170203/2b5d1ed1/attachment.html>