United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-4199382 : Swing Menus - text/icon/checkmark alignment schemes severely broken

Details
Type:
Bug
Submit Date:
1998-12-22
Status:
Closed
Updated Date:
2004-09-29
Project Name:
JDK
Resolved Date:
2004-09-29
Component:
client-libs
OS:
solaris_2.6,generic,windows_xp
Sub-Component:
javax.swing
CPU:
x86,sparc,generic
Priority:
P3
Resolution:
Duplicate
Affected Versions:
1.2.0,1.4.2,5.0
Fixed Versions:

Related Reports
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Relates:
Relates:

Sub Tasks

Description

Name: krT82822			Date: 12/22/98


When a menu pops up - from a right-click, JMenuBar, or whatever, the individual items in the list should follow some basic alignment rules so that the
whole menu appears consistent.
  - WinLF - Extra space is allocated along the left edge of the menu for the check/radio mark for any potentially present JCheckBoxMenuItem or
JRadioButtonMenuItem components even if there are none. By convention on Win32, the checkmark in a menu is actually a depressed button
representation of the menuitem's icon (if any), and it appears in the same 'column' as the icons for other non-check items.
  If no icon is supplied by the JCheckBoxMenuItem, a default checkmark icon should be used.  The text should be left aligned for the whole menu.
The accelerators (called 'shortcuts' in Win32) should also be left-aligned along the right side of the menu (bug #4113639) for the entire menu.  This
should look and feel like a native Win32 application.
  - JLF - Similar problems with WinLF: Wasted space, and text should line up.
  - MotifLF - Slightly better than JLF/WinLF - less wasted space, but text is still not lined up between check (bug #4133790), radio, and regular items.
(Review ID: 48242)
======================================================================

                                    

Comments
EVALUATION

This is partially fixed (by weddie) for Kestrel RC.  The part that is fixed
is the Accelerator text allighnment into a second column.  Still left is any
fix to the checkbox/Icon alignment scheme.  Code used to test included:


import javax.swing.*;
import java.awt.event.*;


public class AccTest extends JFrame implements ActionListener{

  private String title;


  public AccTest(String title) {
    this.title = title;
    
    buildGUI();
  }


  private void buildGUI() {
    this.setSize(400, 400);
    this.setTitle(this.title);
    this.getContentPane().setLayout(new java.awt.FlowLayout() );

    //Create menus
    JMenuBar menubar = new JMenuBar();

    JMenu menu1 = new JMenu("File");
    JMenu menu2 = new JMenu("Options and other Stuff");
    JMenu submenu = new JMenu("SubWay!");
    submenu.setIcon(new ImageIcon("paste.gif"));

    //Menu 1
    JMenuItem openItem = new JMenuItem("Open");
    openItem.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_DELETE, ActionEvent.ALT_MASK+ ActionEvent.CTRL_MASK ));

    JMenuItem newItem = new JMenuItem("New Stuff and other stuff too", new ImageIcon("new.gif") );
    //newItem.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_DELETE, ActionEvent.ALT_MASK + ActionEvent.CTRL_MASK));

    JMenuItem quitItem = new JMenuItem("Quit");
    quitItem.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_Q, ActionEvent.ALT_MASK));
    quitItem.setActionCommand("exit");
    quitItem.addActionListener(this);

    menu1.add(openItem);
    menu1.add(newItem);
    menu1.add(new JSeparator());
    menu1.add(quitItem);


    //Menu2
    JCheckBoxMenuItem checkItem = new JCheckBoxMenuItem("Check this out!");
    checkItem.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_C, ActionEvent.ALT_MASK + ActionEvent.SHIFT_MASK + ActionEvent.CTRL_MASK));
    checkItem.setEnabled(false);
    JRadioButtonMenuItem radioItem = new JRadioButtonMenuItem("Video killed the radio star!");

    JMenuItem normItem = new JMenuItem("Normal");

    JMenuItem iconItem = new JMenuItem("Iconize it!", new ImageIcon("cut.gif"));
    iconItem.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_DELETE, ActionEvent.CTRL_MASK));

    menu2.add(checkItem);
    menu2.add(radioItem);
    menu2.add(normItem);
    menu2.add(new JSeparator());
    menu2.add(iconItem); 

    
    
    //SubMenu
    JMenuItem item1 = new JMenuItem("First SubMenuItem");
    
    submenu.add(item1);
    submenu.add(new JSeparator());

    menu2.add(submenu);

    menubar.add(menu1);
    menubar.add(menu2);
    
    this.setJMenuBar(menubar);


    //Create buttons for change of LNF
    JButton winButton = new JButton("Windows");
    JButton motifButton = new JButton("Motif");
    JButton metalButton = new JButton("Metal");
    JButton macButton = new JButton("Mac"); 

    winButton.addActionListener(this);
    motifButton.addActionListener(this);
    metalButton.addActionListener(this);
    macButton.addActionListener(this);

    winButton.setActionCommand("windows");
    motifButton.setActionCommand("motif");
    metalButton.setActionCommand("metal");
    macButton.setActionCommand("mac");

    this.getContentPane().add(winButton);
    this.getContentPane().add(motifButton);
    this.getContentPane().add(metalButton);
    this.getContentPane().add(macButton);
  }
    
  
  public void actionPerformed(ActionEvent e) {
    String lnfName = null;
    
    if(e.getActionCommand().equals("exit")) {
      this.dispose();
      System.exit(0);
    }
    else { 
      if(e.getActionCommand().equals("windows")) {
	lnfName = 
	  "com.sun.java.swing.plaf.windows.WindowsLookAndFeel";
      }
      else if(e.getActionCommand().equals("motif")) {
	lnfName = 
	  "com.sun.java.swing.plaf.motif.MotifLookAndFeel";	
      }
      else if(e.getActionCommand().equals("metal")) {
	lnfName = "javax.swing.plaf.metal.MetalLookAndFeel";
	  }
      else if(e.getActionCommand().equals("mac")) {
	lnfName = "com.sun.java.swing.plaf.mac.MacLookAndFeel";
	  }
      try {
	UIManager.setLookAndFeel(lnfName);
	SwingUtilities.updateComponentTreeUI(this);
      } 
      catch (Exception exc) {
	System.err.println("Could not load LookAndFeel: " + lnfName);
      }
    }
  }       



  public static void main(String[] args) {

    (new AccTest("TestFrame") ).show();

  }

}

georges.saab@Eng 1999-10-18

Name: pzR10082			Date: 02/20/2002


We now left-align accelerator keys, but text is still not aligned.
To align text, we need to know if there is an item in the menu which
has check icon. If such item exists, we have to indent text in all
the other menu items.

We can calculate maximum check icon width across all menu items and
then indent text accordingly.

###@###.### 2002-02-20

======================================================================

Currently all the development for this issue is concentrated in the bug 4729669. I'm closing this one as a duplicate of 4729669.
###@###.### 2004-09-29
                                     
2004-09-29
SUGGESTED FIX



Name: pzR10082			Date: 02/20/2002



------- BasicMenuItemUI.java -------
*** /tmp/sccs.XHfV6G	Wed Feb 20 18:39:41 2002
--- BasicMenuItemUI.java	Wed Feb 20 13:57:01 2002
***************
*** 63,68 ****
--- 63,69 ----
      /* Client Property keys for text and accelerator text widths */
      static final String MAX_TEXT_WIDTH =  "maxTextWidth";
      static final String MAX_ACC_WIDTH  =  "maxAccWidth";
+     static final String MAX_CHECK_ICON_WIDTH =  "maxCheckIconWidth";
  
      public static ComponentUI createUI(JComponent c) {
          return new BasicMenuItemUI();
***************
*** 182,191 ****
  	//Remove the textWidth and accWidth values from the parent's Client Properties.
  	Container parent = menuItem.getParent();
  	if ( (parent != null && parent instanceof JComponent)  && 
! 	     !(menuItem instanceof JMenu && ((JMenu) menuItem).isTopLevelMenu())) {
  	    JComponent p = (JComponent) parent;
  	    p.putClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH, null );
  	    p.putClientProperty(BasicMenuItemUI.MAX_TEXT_WIDTH, null ); 
  	}
  
  	menuItem = null;
--- 183,193 ----
  	//Remove the textWidth and accWidth values from the parent's Client Properties.
  	Container parent = menuItem.getParent();
  	if ( (parent != null && parent instanceof JComponent)  && 
! 	     !isTopLevelMenu()) {
  	    JComponent p = (JComponent) parent;
  	    p.putClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH, null );
  	    p.putClientProperty(BasicMenuItemUI.MAX_TEXT_WIDTH, null ); 
+ 	    p.putClientProperty(BasicMenuItemUI.MAX_CHECK_ICON_WIDTH, null);
  	}
  
  	menuItem = null;
***************
*** 372,378 ****
          FontMetrics fmAccel = b.getToolkit().getFontMetrics( acceleratorFont );
  
          resetRects();
!         
          layoutMenuItem(
                    fm, text, fmAccel, acceleratorText, icon, checkIcon, arrowIcon,
                    b.getVerticalAlignment(), b.getHorizontalAlignment(),
--- 374,380 ----
          FontMetrics fmAccel = b.getToolkit().getFontMetrics( acceleratorFont );
  
          resetRects();
! 
          layoutMenuItem(
                    fm, text, fmAccel, acceleratorText, icon, checkIcon, arrowIcon,
                    b.getVerticalAlignment(), b.getHorizontalAlignment(),
***************
*** 381,386 ****
--- 383,389 ----
                    text == null ? 0 : defaultTextIconGap,
                    defaultTextIconGap
                    );
+ 
          // find the union of the icon and text rects
          r.setBounds(textRect);
          r = SwingUtilities.computeUnion(iconRect.x,
***************
*** 399,413 ****
  	
  	//Check the parent, and see that it is not a top-level menu.
          if (parent != null && parent instanceof JComponent && 
! 	    !(menuItem instanceof JMenu && ((JMenu) menuItem).isTopLevelMenu())) {
  	    JComponent p = (JComponent) parent;
  	    
  	    //Get widest text so far from parent, if no one exists null is returned.
! 	    Integer maxTextWidth = (Integer) p.getClientProperty(BasicMenuItemUI.MAX_TEXT_WIDTH);
! 	    Integer maxAccWidth = (Integer) p.getClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH);
! 	    
  	    int maxTextValue = maxTextWidth!=null ? maxTextWidth.intValue() : 0;
  	    int maxAccValue = maxAccWidth!=null ? maxAccWidth.intValue() : 0;
  	    
  	    //Compare the text widths, and adjust the r.width to the widest.
  	    if (r.width < maxTextValue) {
--- 402,422 ----
  	
  	//Check the parent, and see that it is not a top-level menu.
          if (parent != null && parent instanceof JComponent && 
! 	    !isTopLevelMenu()) {
  	    JComponent p = (JComponent) parent;
  	    
  	    //Get widest text so far from parent, if no one exists null is returned.
! 	    Integer maxTextWidth = (Integer)p.getClientProperty(
!                 BasicMenuItemUI.MAX_TEXT_WIDTH);
! 	    Integer maxAccWidth = (Integer)p.getClientProperty(
!                 BasicMenuItemUI.MAX_ACC_WIDTH);
! 	    Integer maxCheckIconWidth = (Integer)p.getClientProperty(
!                 BasicMenuItemUI.MAX_CHECK_ICON_WIDTH);
! 
  	    int maxTextValue = maxTextWidth!=null ? maxTextWidth.intValue() : 0;
  	    int maxAccValue = maxAccWidth!=null ? maxAccWidth.intValue() : 0;
+ 	    int maxCheckValue = maxCheckIconWidth != null ?
+                 maxCheckIconWidth.intValue() : 0;
  	    
  	    //Compare the text widths, and adjust the r.width to the widest.
  	    if (r.width < maxTextValue) {
***************
*** 421,427 ****
  		maxAccValue = acceleratorRect.width;
  		p.putClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH, new Integer(acceleratorRect.width) );
  	    }
! 	    
  	    //Add on the widest accelerator 
  	    r.width += maxAccValue;
  	    r.width += defaultTextIconGap;
--- 430,443 ----
  		maxAccValue = acceleratorRect.width;
  		p.putClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH, new Integer(acceleratorRect.width) );
  	    }
! 
!             if (checkIconRect.width > maxCheckValue) {
! 		p.putClientProperty(BasicMenuItemUI.MAX_CHECK_ICON_WIDTH,
!                                     new Integer(checkIconRect.width));
! 	    } else {
!                 checkIconRect.width = maxCheckValue;
!             }
! 
  	    //Add on the widest accelerator 
  	    r.width += maxAccValue;
  	    r.width += defaultTextIconGap;
***************
*** 547,553 ****
              b.getText() == null ? 0 : defaultTextIconGap,
              defaultTextIconGap
          );
!           
          // Paint background
  	paintBackground(g, b, background);
  
--- 563,590 ----
              b.getText() == null ? 0 : defaultTextIconGap,
              defaultTextIconGap
          );
! 
!         int maxAccWidth = 0;
!         int checkDelta = 0;
!         Container parent = menuItem.getParent();
!         if (parent != null && parent instanceof JComponent) {
! 	    JComponent p = (JComponent) parent;
! 	    Integer n = (Integer)p.getClientProperty(
!                 BasicMenuItemUI.MAX_ACC_WIDTH);
!             maxAccWidth = n == null? 0 : n.intValue();
! 	    n = (Integer)p.getClientProperty(
!                 BasicMenuItemUI.MAX_CHECK_ICON_WIDTH);
!             checkDelta = n == null? 0 : n.intValue();
!         }
! 
!         // Shift icon and text rects by the maximum check icon width.
!         // This is needed to align menu items
!         if (!isTopLevelMenu() && checkIcon != null) {
!             checkDelta -= checkIcon.getIconWidth();
!         }
!         iconRect.x += checkDelta;
!         textRect.x += checkDelta;
! 
          // Paint background
  	paintBackground(g, b, background);
  
***************
*** 598,615 ****
          if(acceleratorText != null && !acceleratorText.equals("")) {
  
  	  //Get the maxAccWidth from the parent to calculate the offset.
! 	  int accOffset = 0;
! 	  Container parent = menuItem.getParent();
! 	  if (parent != null && parent instanceof JComponent) {
! 	    JComponent p = (JComponent) parent;
! 	    Integer maxValueInt = (Integer) p.getClientProperty(BasicMenuItemUI.MAX_ACC_WIDTH);
! 	    int maxValue = maxValueInt != null ?
!                 maxValueInt.intValue() : acceleratorRect.width;
! 
! 	    //Calculate the offset, with which the accelerator texts will be drawn with.
! 	    accOffset = maxValue - acceleratorRect.width;
! 	  }
! 	  
  	  g.setFont( acceleratorFont );
              if(!model.isEnabled()) {
                  // *** paint the acceleratorText disabled
--- 635,642 ----
          if(acceleratorText != null && !acceleratorText.equals("")) {
  
  	  //Get the maxAccWidth from the parent to calculate the offset.
!           int accOffset =
!               maxAccWidth == 0 ? 0 : maxAccWidth - acceleratorRect.width;
  	  g.setFont( acceleratorFont );
              if(!model.isEnabled()) {
                  // *** paint the acceleratorText disabled
***************
*** 848,861 ****
          return text;
      }
  
      /*
       * Returns false if the component is a JMenu and it is a top
       * level menu (on the menubar).
       */
      private boolean useCheckAndArrow(){
  	boolean b = true;
! 	if((menuItem instanceof JMenu) &&
! 	   (((JMenu)menuItem).isTopLevelMenu())) {
  	    b = false;
  	}
  	return b;
--- 875,892 ----
          return text;
      }
  
+     private boolean isTopLevelMenu() {
+         return menuItem instanceof JMenu &&
+             ((JMenu)menuItem).isTopLevelMenu();
+     }
+ 
      /*
       * Returns false if the component is a JMenu and it is a top
       * level menu (on the menubar).
       */
      private boolean useCheckAndArrow(){
  	boolean b = true;
! 	if(isTopLevelMenu()) {
  	    b = false;
  	}
  	return b;

------- MotifGraphicsUtils.java -------
*** /tmp/sccs.zlu2LA	Wed Feb 20 18:39:41 2002
--- MotifGraphicsUtils.java	Wed Feb 20 18:39:56 2002
***************
*** 31,37 ****
  {       
      /* Client Property keys for text and accelerator text widths */
      private static final String MAX_ACC_WIDTH  =  "maxAccWidth";
!     
      /**
       * Draws the point (<b>x</b>, <b>y</b>) in the current color.
       */
--- 31,38 ----
  {       
      /* Client Property keys for text and accelerator text widths */
      private static final String MAX_ACC_WIDTH  =  "maxAccWidth";
!     private static final String MAX_CHECK_ICON_WIDTH =  "maxCheckIconWidth";
! 
      /**
       * Draws the point (<b>x</b>, <b>y</b>) in the current color.
       */
***************
*** 181,187 ****
  				     ? 0 : defaultTextIconGap,
  				     defaultTextIconGap
  				     );
! 	
  	// Paint the Check
  	Color holdc = g.getColor();
  	if (checkIcon != null) {
--- 182,209 ----
  				     ? 0 : defaultTextIconGap,
  				     defaultTextIconGap
  				     );
! 
!         int maxAccWidth = 0;
!         int checkDelta = 0;
!         Container parent = b.getParent();
!         if (parent != null && parent instanceof JComponent) {
! 	    JComponent p = (JComponent) parent;
! 	    Integer n = (Integer)p.getClientProperty(MAX_ACC_WIDTH);
!             maxAccWidth = n == null? 0 : n.intValue();
! 	    n = (Integer)p.getClientProperty(MAX_CHECK_ICON_WIDTH);
!             checkDelta = n == null? 0 : n.intValue();
!         }
! 
!         // Shift icon and text rects by the maximum check icon width.
!         // This is needed to align menu items
!         if (! (b instanceof JMenu && ((JMenu)b).isTopLevelMenu()) &&
!             checkIcon != null) {
! 
!             checkDelta -= checkIcon.getIconWidth();
!         }
!         iconRect.x += checkDelta;
!         textRect.x += checkDelta;
! 
  	// Paint the Check
  	Color holdc = g.getColor();
  	if (checkIcon != null) {
***************
*** 252,267 ****
  
  	    //Get the maxAccWidth from the parent to calculate the offset.
  	    int accOffset = 0;
! 	    Container parent = b.getParent();
! 	    if (parent != null && parent instanceof JComponent) {
! 		JComponent p = (JComponent) parent;
! 		Integer maxValueInt = (Integer) p.getClientProperty(MotifGraphicsUtils.MAX_ACC_WIDTH);
! 		int maxValue = maxValueInt != null ?
!                     maxValueInt.intValue() : acceleratorRect.width;
! 		
! 		//Calculate the offset, with which the accelerator texts will be drawn with.
! 		accOffset = maxValue - acceleratorRect.width;
! 	    }
  
  	    g.setFont( UIManager.getFont("MenuItem.acceleratorFont") );
  	    if(!model.isEnabled()) {
--- 274,282 ----
  
  	    //Get the maxAccWidth from the parent to calculate the offset.
  	    int accOffset = 0;
!             if (maxAccWidth > 0) {
!                 accOffset = maxAccWidth - acceleratorRect.width;
!             }
  
  	    g.setFont( UIManager.getFont("MenuItem.acceleratorFont") );
  	    if(!model.isEnabled()) {

###@###.### 2002-02-20

======================================================================
                                     
2002-02-20



Hardware and Software, Engineered to Work Together