Re: [cp-patches] FYI: ComboBox fixes

[Lillian Angel]

The arrow is not being painted anymore. It was this morning, so I
suspect this patch has something to do with it.

Thanks,
Lillian


On Wed, 2005-10-19 at 15:56 +0100, David Gilbert wrote:
> This patch (committed) fixes most of the failing Mauve tests for the combo 
> box classes:
> 
> 2005-10-19  David Gilbert  <address@hidden>
> 
>       * javax/swing/plaf/basic/BasicArrowButton.java
>       (buttonBorder): removed,
>       (BasicArrowButton): don't set border, use default border,
>       * javax/swing/plaf/basic/BasicComboBoxUI.java
>       (configureArrowButton): don't set margin here,
>       (getPreferredSize): added comment,
>       (getMinimumSize): icon width is the same as the display area height,
>       (getDefaultSize): return size of space char plus 1 pixel margin,
>       (getDisplaySize): take into account the prototype display value, if
>       there is one,
>       * javax/swing/plaf/basic/BasicLookAndFeel.java
>       (initComponentDefaults): updated 'Button.margin' default,
>       * javax/swing/plaf/metal/MetalComboBoxButton.java
>       (MetalComboBoxButton(JComboBox, Icon, boolean, CellRendererPane,
>       JList)): check for null JComboBox, don't set margin here,
>       * javax/swing/plaf/metal/MetalComboBoxUI.java
>       (createArrowButton): set margin,
>       (getMinimumSize): reimplemented,
>       * javax/swing/plaf/metal/MetalLookAndFeel.java
>       (initComponentDefaults): changed 'Button.margin' default to from Insets
>       to InsetsUIResource.
> 
> Regards,
> 
> Dave
> plain text document attachment (diff.txt)
> Index: javax/swing/plaf/basic/BasicArrowButton.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicArrowButton.java,v
> retrieving revision 1.14
> diff -u -r1.14 BasicArrowButton.java
> --- javax/swing/plaf/basic/BasicArrowButton.java      19 Sep 2005 16:53:46 
> -0000      1.14
> +++ javax/swing/plaf/basic/BasicArrowButton.java      19 Oct 2005 14:28:10 
> -0000
> @@ -98,48 +98,6 @@
>     */
>    transient Color highlight = Color.WHITE;
>  
> -  /** The border around the ArrowButton. */
> -  private transient Border buttonBorder = new Border()
> -    {
> -      public Insets getBorderInsets(Component c)
> -      {
> -     return new Insets(2, 2, 2, 2);
> -      }
> -
> -      public boolean isBorderOpaque()
> -      {
> -     return true;
> -      }
> -
> -      public void paintBorder(Component c, Graphics g, int x, int y, int w,
> -                              int h)
> -      {
> -        Color saved = g.getColor();
> -        AbstractButton b = (AbstractButton) c;
> -        if (b.getModel().isPressed())
> -          {
> -            g.setColor(darkShadow);
> -            g.drawRect(x, y, x + w - 1, y + h - 1);
> -          }
> -        else
> -          {
> -            g.setColor(highlight);
> -            g.drawLine(x + 1, y + 1, x + w - 3, y + 1);
> -            g.drawLine(x + 1, y + 1, x + 1, y + h - 2);
> -
> -            g.setColor(shadow);
> -            g.drawLine(x + 1, y + h - 2, x + w - 1, y + h - 2);
> -            g.drawLine(x + w - 2, y + 1, x + w - 2, y + h - 2);
> -
> -            g.setColor(darkShadow);
> -            g.drawLine(x, y + h - 1, x + w - 1, y + h - 1);
> -            g.drawLine(x + w - 1, y, x + w - 1, y + h - 1);
> -
> -            g.setColor(saved);
> -          }
> -      }
> -    };
> -
>    /**
>     * Creates a new BasicArrowButton object.
>     *
> @@ -148,7 +106,6 @@
>    public BasicArrowButton(int direction)
>    {
>      super();
> -    setBorder(buttonBorder);
>      setDirection(direction);
>    }
>  
> Index: javax/swing/plaf/basic/BasicComboBoxUI.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicComboBoxUI.java,v
> retrieving revision 1.25
> diff -u -r1.25 BasicComboBoxUI.java
> --- javax/swing/plaf/basic/BasicComboBoxUI.java       18 Oct 2005 15:27:55 
> -0000      1.25
> +++ javax/swing/plaf/basic/BasicComboBoxUI.java       19 Oct 2005 14:28:11 
> -0000
> @@ -43,6 +43,7 @@
>  import java.awt.Container;
>  import java.awt.Dimension;
>  import java.awt.Font;
> +import java.awt.FontMetrics;
>  import java.awt.Graphics;
>  import java.awt.Insets;
>  import java.awt.LayoutManager;
> @@ -556,7 +557,6 @@
>    {
>      arrowButton.setEnabled(comboBox.isEnabled());
>      arrowButton.setFont(comboBox.getFont());
> -    arrowButton.setMargin(new Insets(0, 0, 0, 0));
>    }
>  
>    /**
> @@ -651,6 +651,9 @@
>     */
>    public Dimension getPreferredSize(JComponent c)
>    {
> +    // note:  overriding getMinimumSize() (for example in the 
> MetalComboBoxUI 
> +    // class) affects the getPreferredSize() result, so it seems logical that
> +    // this method is implemented by delegating to the getMinimumSize() 
> method
>      return getMinimumSize(c);
>    }
>  
> @@ -665,9 +668,8 @@
>    public Dimension getMinimumSize(JComponent c)
>    {
>      Dimension d = getDisplaySize();
> -    Dimension arrowDim = arrowButton.getPreferredSize();
> -    Dimension result = new Dimension(d.width + arrowDim.width, 
> -            Math.max(d.height, arrowDim.height));
> +    int arrowButtonWidth = d.height;
> +    Dimension result = new Dimension(d.width + arrowButtonWidth, d.height);
>      return result;
>    }
>  
> @@ -828,15 +830,24 @@
>    }
>  
>    /**
> -   * Returns default size for the combo box that doesn't contain any elements
> -   * in it
> +   * Returns the default size for the display area of a combo box that does 
> +   * not contain any elements.  This method returns the width and height of
> +   * a single space in the current font, plus a margin of 1 pixel. 
>     *
> -   * @return Default size of the combo box with no elements in it.
> +   * @return The default display size.
> +   * 
> +   * @see #getDisplaySize()
>     */
>    protected Dimension getDefaultSize()
>    {
> -    // FIXME: Not implemented properly.
> -    return new Dimension(100, 5);
> +    // There is nothing in the spec to say how this method should be
> +    // implemented...so I've done some guessing, written some Mauve tests,
> +    // and written something that gives dimensions that are close to the 
> +    // reference implementation.
> +    FontMetrics fm = comboBox.getFontMetrics(comboBox.getFont());
> +    int w = fm.charWidth(' ') + 2;
> +    int h = fm.getHeight() + 2;
> +    return new Dimension(w, h);
>    }
>  
>    /**
> @@ -847,40 +858,52 @@
>     */
>    protected Dimension getDisplaySize()
>    {
> -    ComboBoxModel model = comboBox.getModel();
> -    int numItems = model.getSize();
> -
> -    // if combo box doesn't have any items then simply
> -    // return its default size
> -    if (numItems == 0)
> +    Object prototype = comboBox.getPrototypeDisplayValue();
> +    if (prototype != null)
>        {
> -     displaySize = getDefaultSize();
> -     return displaySize;
> +        // calculate result based on prototype
> +        ListCellRenderer renderer = comboBox.getRenderer();
> +        Component comp = renderer.getListCellRendererComponent(listBox, 
> +                prototype, -1, false, false);
> +        Dimension compSize = comp.getPreferredSize();
> +        compSize.width += 2;  // add 1 pixel margin around area
> +        compSize.height += 2;
> +        return compSize;
>        }
> +    else
> +      {
> +        ComboBoxModel model = comboBox.getModel();
> +        int numItems = model.getSize();
> +
> +        // if combo box doesn't have any items then simply
> +        // return its default size
> +        if (numItems == 0)
> +          {
> +            displaySize = getDefaultSize();
> +            return displaySize;
> +          }
>  
> -    Dimension size = new Dimension(0, 0);
> +        Dimension size = new Dimension(0, 0);
>  
> -    // ComboBox's display size should be equal to the 
> -    // size of the largest item in the combo box. 
> -    ListCellRenderer renderer = comboBox.getRenderer();
> -
> -    // FIXME: use the JComboBox.getPrototypeDisplayValue() if there is
> -    // one
> -    for (int i = 0; i < numItems; i++)
> -      {
> -        Object item = model.getElementAt(i);
> -        String s = item.toString();
> -        Component comp = renderer.getListCellRendererComponent(listBox, item,
> -            -1, false, false);
> +        // ComboBox's display size should be equal to the 
> +        // size of the largest item in the combo box. 
> +        ListCellRenderer renderer = comboBox.getRenderer();
>  
> -        Dimension compSize = comp.getPreferredSize();
> -        if (compSize.width > size.width)
> -          size.width = compSize.width;
> -        if (compSize.height > size.height)
> -          size.height = compSize.height;
> +        for (int i = 0; i < numItems; i++)
> +          {
> +            Object item = model.getElementAt(i);
> +            Component comp = renderer.getListCellRendererComponent(listBox, 
> +                    item, -1, false, false);
> +
> +            Dimension compSize = comp.getPreferredSize();
> +            if (compSize.width + 2 > size.width)
> +              size.width = compSize.width + 2;
> +            if (compSize.height + 2 > size.height)
> +              size.height = compSize.height + 2;
> +          }
> +        displaySize = size;
> +        return displaySize;
>        }
> -    displaySize = size;
> -    return displaySize;
>    }
>  
>    /**
> Index: javax/swing/plaf/basic/BasicLookAndFeel.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicLookAndFeel.java,v
> retrieving revision 1.63
> diff -u -r1.63 BasicLookAndFeel.java
> --- javax/swing/plaf/basic/BasicLookAndFeel.java      18 Oct 2005 17:48:03 
> -0000      1.63
> +++ javax/swing/plaf/basic/BasicLookAndFeel.java      19 Oct 2005 14:28:13 
> -0000
> @@ -273,7 +273,7 @@
>        "Button.foreground", new ColorUIResource(Color.BLACK),
>        "Button.highlight", new ColorUIResource(Color.WHITE),
>        "Button.light", new ColorUIResource(Color.LIGHT_GRAY),
> -      "Button.margin", new InsetsUIResource(2, 2, 2, 2),
> +      "Button.margin", new InsetsUIResource(2, 14, 2, 14),
>        "Button.shadow", new ColorUIResource(Color.GRAY),
>        "Button.textIconGap", new Integer(4),
>        "Button.textShiftOffset", new Integer(0),
> Index: javax/swing/plaf/metal/MetalComboBoxButton.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/javax/swing/plaf/metal/MetalComboBoxButton.java,v
> retrieving revision 1.5
> diff -u -r1.5 MetalComboBoxButton.java
> --- javax/swing/plaf/metal/MetalComboBoxButton.java   17 Oct 2005 10:02:52 
> -0000      1.5
> +++ javax/swing/plaf/metal/MetalComboBoxButton.java   19 Oct 2005 14:28:14 
> -0000
> @@ -100,12 +100,13 @@
>        CellRendererPane pane, JList list)
>    {
>      super();
> +    if (cb == null)
> +      throw new NullPointerException("Null 'cb' argument");
>      comboBox = cb;
>      comboIcon = i;
>      iconOnly = onlyIcon;
>      listBox = list;
>      rendererPane = pane;
> -    setMargin(new Insets(0, 0, 0, 0));
>    }
>    
>    /**
> Index: javax/swing/plaf/metal/MetalComboBoxUI.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/javax/swing/plaf/metal/MetalComboBoxUI.java,v
> retrieving revision 1.6
> diff -u -r1.6 MetalComboBoxUI.java
> --- javax/swing/plaf/metal/MetalComboBoxUI.java       19 Oct 2005 13:46:02 
> -0000      1.6
> +++ javax/swing/plaf/metal/MetalComboBoxUI.java       19 Oct 2005 14:28:14 
> -0000
> @@ -41,6 +41,7 @@
>  import java.awt.Container;
>  import java.awt.Dimension;
>  import java.awt.Graphics;
> +import java.awt.Insets;
>  import java.awt.LayoutManager;
>  import java.awt.Rectangle;
>  import java.awt.event.MouseEvent;
> @@ -49,6 +50,7 @@
>  
>  import javax.swing.CellRendererPane;
>  import javax.swing.ComboBoxEditor;
> +import javax.swing.Icon;
>  import javax.swing.JButton;
>  import javax.swing.JComboBox;
>  import javax.swing.JComponent;
> @@ -210,8 +212,10 @@
>     */
>    protected JButton createArrowButton()
>    {
> -    return new MetalComboBoxButton(comboBox, new MetalComboBoxIcon(), 
> +    JButton button = new MetalComboBoxButton(comboBox, new 
> MetalComboBoxIcon(), 
>              new CellRendererPane(), listBox);  
> +    button.setMargin(new Insets(0, 1, 1, 3));
> +    return button;
>    }
>    
>    /**
> @@ -291,10 +295,15 @@
>     */
>    public Dimension getMinimumSize(JComponent c)
>    {
> -    // FIXME: this needs work
> -    Dimension result = super.getMinimumSize(c);
> -    result.height = result.height + 9;
> -    return result;   
> +    MetalComboBoxButton b = (MetalComboBoxButton) arrowButton;
> +    Icon icon = b.getComboIcon();
> +    Insets insets = b.getInsets();
> +    Dimension d = getDisplaySize();
> +    int insetsH = insets.top + insets.bottom;
> +    int insetsW = insets.left + insets.right;
> +    int iconWidth = icon.getIconWidth() + 6;
> +    return new Dimension(d.width + insetsW + iconWidth, 
> +            d.height + insetsH);
>    }
>    
>  }
> Index: javax/swing/plaf/metal/MetalLookAndFeel.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/javax/swing/plaf/metal/MetalLookAndFeel.java,v
> retrieving revision 1.66
> diff -u -r1.66 MetalLookAndFeel.java
> --- javax/swing/plaf/metal/MetalLookAndFeel.java      17 Oct 2005 10:02:52 
> -0000      1.66
> +++ javax/swing/plaf/metal/MetalLookAndFeel.java      19 Oct 2005 14:28:16 
> -0000
> @@ -781,7 +781,7 @@
>        "Button.foreground", getControlTextColor(),
>        "Button.highlight", getControlHighlight(),
>        "Button.light", getControlHighlight(),
> -      "Button.margin", new Insets(2, 14, 2, 14),
> +      "Button.margin", new InsetsUIResource(2, 14, 2, 14),
>        "Button.select", getControlShadow(),
>        "Button.shadow", getControlShadow(),
>  
> _______________________________________________
> Classpath-patches mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/classpath-patches