JDK-4199382 : Swing Menus - text/icon/checkmark alignment schemes severely broken
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.swing
  • Affected Version: 1.2.0,1.4.2,5.0
  • Priority: P3
  • Status: Closed
  • Resolution: Duplicate
  • OS: generic,solaris_2.6,windows_xp
  • CPU: generic,x86,sparc
  • Submitted: 1998-12-22
  • Updated: 2004-09-29
  • Resolved: 2004-09-29
Related Reports
Duplicate :  
JDK-4729669 - 1.4 REGRESSION: Text edge of different types of JMenuItems are not aligned
Duplicate :  
JDK-4113639 - Hotkey labels for menus should be left-aligned
Duplicate :  
JDK-4907679 - XP L&F: Menu appearance does not conform to native XP look with icons
Duplicate :  
JDK-4992232 - JMenuItem indenting rendered under a JMenu in odd
Relates :  
JDK-4991598 - Win L&F: Icon glyph for radio button menu item is wrong in winxp L&F
Relates :  
JDK-6385366 - REGRESSION: Descriptive text and accelerator text overlap on JMenuItem.
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
29-09-2004
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 ======================================================================
20-02-2002