MenuBar fixes (again)

 * Fix submenu indicators for IE
 * Fixes #3678: enhancement: MenuBar needs a separator item

svn changeset:9722/svn branch:6.2

2 files changed, 131 insertions, 64 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java b/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java
index c756b32453..0e09c7a287 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java
@@ -22,6 +22,7 @@ import com.vaadin.terminal.gwt.client.ApplicationConnection;
 import com.vaadin.terminal.gwt.client.ContainerResizedListener;
 import com.vaadin.terminal.gwt.client.Paintable;
 import com.vaadin.terminal.gwt.client.UIDL;
+import com.vaadin.terminal.gwt.client.Util;
 
 public class VMenuBar extends Widget implements Paintable,
         CloseHandler<PopupPanel>, ContainerResizedListener {
@@ -156,47 +157,53 @@ public class VMenuBar extends Widget implements Paintable,
             String itemText = item.getStringAttribute("text");
             final int itemId = item.getIntAttribute("id");
 
-            boolean itemHasCommand = item.getBooleanAttribute("command");
+            boolean itemHasCommand = item.hasAttribute("command");
 
             // Construct html from the text and the optional icon
             StringBuffer itemHTML = new StringBuffer();
+            Command cmd = null;
 
-            // Add submenu indicator
-            if (item.getChildCount() > 0) {
-                // FIXME For compatibility reasons: remove in version 7
-                String bgStyle = "";
-                if (submenuIcon != null) {
-                    bgStyle = " style=\"background-image: url(" + submenuIcon
-                            + "); text-indent: -999px; width: 1em;\"";
+            if (item.hasAttribute("separator")) {
+                itemHTML.append("<span>---</span>");
+            } else {
+                // Add submenu indicator
+                if (item.getChildCount() > 0) {
+                    // FIXME For compatibility reasons: remove in version 7
+                    String bgStyle = "";
+                    if (submenuIcon != null) {
+                        bgStyle = " style=\"background-image: url("
+                                + submenuIcon
+                                + "); text-indent: -999px; width: 1em;\"";
+                    }
+                    itemHTML.append("<span class=\"" + CLASSNAME
+                            + "-submenu-indicator\"" + bgStyle
+                            + ">&#x25BA;</span>");
                 }
-                itemHTML
-                        .append("<span class=\"" + CLASSNAME
-                                + "-submenu-indicator\"" + bgStyle
-                                + ">&#x25BA;</span>");
-            }
 
-            if (item.hasAttribute("icon")) {
-                itemHTML.append("<img src=\""
-                        + client.translateVaadinUri(item
-                                .getStringAttribute("icon")) + "\" class=\""
-                        + Icon.CLASSNAME + "\" alt=\"\" />");
-            }
-
-            itemHTML.append(itemText);
+                if (item.hasAttribute("icon")) {
+                    itemHTML
+                            .append("<img src=\""
+                                    + client.translateVaadinUri(item
+                                            .getStringAttribute("icon"))
+                                    + "\" class=\"" + Icon.CLASSNAME
+                                    + "\" alt=\"\" />");
+                }
 
-            Command cmd = null;
+                itemHTML.append(Util.escapeHTML(itemText));
 
-            if (itemHasCommand) {
-                // Construct a command that fires onMenuClick(int) with the
-                // item's id-number
-                cmd = new Command() {
-                    public void execute() {
-                        hostReference.onMenuClick(itemId);
-                    }
-                };
+                if (itemHasCommand) {
+                    // Construct a command that fires onMenuClick(int) with the
+                    // item's id-number
+                    cmd = new Command() {
+                        public void execute() {
+                            hostReference.onMenuClick(itemId);
+                        }
+                    };
+                }
             }
 
             currentItem = currentMenu.addItem(itemHTML.toString(), cmd);
+            currentItem.setSeparator(item.hasAttribute("separator"));
             currentItem.setEnabled(!item.hasAttribute("disabled"));
 
             if (item.getChildCount() > 0) {
@@ -448,7 +455,7 @@ public class VMenuBar extends Widget implements Paintable,
      * @param item
      */
     public void itemOver(CustomMenuItem item) {
-        if (subMenu || menuVisible) {
+        if ((subMenu || menuVisible) && !item.isSeparator()) {
             setSelected(item);
         }
 
@@ -484,6 +491,8 @@ public class VMenuBar extends Widget implements Paintable,
      * @param item
      */
     public void showChildMenu(CustomMenuItem item) {
+        final int shadowSpace = 10;
+
         popup = new VOverlay(true, false, true);
         popup.setWidget(item.getSubMenu());
         popup.addCloseHandler(this);
@@ -509,14 +518,18 @@ public class VMenuBar extends Widget implements Paintable,
         popup.show();
 
         if (left + popup.getOffsetWidth() >= RootPanel.getBodyElement()
-                .getOffsetWidth()) {
+                .getOffsetWidth()
+                - shadowSpace) {
             if (subMenu) {
                 left = item.getParentMenu().getAbsoluteLeft()
-                        - popup.getOffsetWidth();
+                        - popup.getOffsetWidth() - shadowSpace;
             } else {
                 left = RootPanel.getBodyElement().getOffsetWidth()
-                        - popup.getOffsetWidth();
-                ApplicationConnection.getConsole().log("" + left);
+                        - popup.getOffsetWidth() - shadowSpace;
+            }
+            // Accommodate space for shadow
+            if (left < shadowSpace) {
+                left = shadowSpace;
             }
             popup.setPopupPosition(left, top);
         }
@@ -643,6 +656,7 @@ public class VMenuBar extends Widget implements Paintable,
         protected VMenuBar subMenu = null;
         protected VMenuBar parentMenu = null;
         protected boolean enabled = true;
+        protected boolean isSeparator = false;
 
         public CustomMenuItem(String html, Command cmd) {
             setElement(DOM.createTD());
@@ -655,7 +669,7 @@ public class VMenuBar extends Widget implements Paintable,
         }
 
         public void setSelected(boolean selected) {
-            if (selected) {
+            if (selected && !isSeparator) {
                 addStyleDependentName("selected");
             } else {
                 removeStyleDependentName("selected");
@@ -719,6 +733,20 @@ public class VMenuBar extends Widget implements Paintable,
         public boolean isEnabled() {
             return enabled;
         }
+
+        private void setSeparator(boolean separator) {
+            isSeparator = separator;
+            if (separator) {
+                setStyleName(CLASSNAME + "-separator");
+            } else {
+                setStyleName(CLASSNAME + "-menuitem");
+                setEnabled(enabled);
+            }
+        }
+
+        public boolean isSeparator() {
+            return isSeparator;
+        }
     }
 
     /**
diff --git a/src/com/vaadin/ui/MenuBar.java b/src/com/vaadin/ui/MenuBar.java
index 77523f9680..286da7a4ff 100644
--- a/src/com/vaadin/ui/MenuBar.java
+++ b/src/com/vaadin/ui/MenuBar.java
@@ -79,35 +79,39 @@ public class MenuBar extends AbstractComponent {
         while (itr.hasNext()) {
 
             MenuItem item = itr.next();
-
             target.startTag("item");
-
-            target.addAttribute("text", item.getText());
             target.addAttribute("id", item.getId());
 
-            Command command = item.getCommand();
-            if (command != null) {
-                target.addAttribute("command", true);
+            if (item.isSeparator()) {
+                target.addAttribute("separator", true);
+                target.endTag("item");
             } else {
-                target.addAttribute("command", false);
-            }
+                target.addAttribute("text", item.getText());
 
-            Resource icon = item.getIcon();
-            if (icon != null) {
-                target.addAttribute("icon", icon);
-            }
+                Command command = item.getCommand();
+                if (command != null) {
+                    target.addAttribute("command", true);
+                }
 
-            if (!item.isEnabled()) {
-                target.addAttribute("disabled", true);
-            }
+                Resource icon = item.getIcon();
+                if (icon != null) {
+                    target.addAttribute("icon", icon);
+                }
 
-            if (item.hasChildren()) {
-                iteratorStack.push(itr); // For later use
+                if (!item.isEnabled()) {
+                    target.addAttribute("disabled", true);
+                }
+
+                if (item.hasChildren()) {
+                    iteratorStack.push(itr); // For later use
+
+                    // Go through the children
+                    itr = item.getChildren().iterator();
+                } else {
+                    target.endTag("item"); // Item had no children, end
+                    // description
+                }
 
-                // Go through the children
-                itr = item.getChildren().iterator();
-            } else {
-                target.endTag("item"); // Item had no children, end description
             }
 
             // The end submenu. More than one submenu may end at once.
@@ -115,7 +119,6 @@ public class MenuBar extends AbstractComponent {
                 itr = iteratorStack.pop();
                 target.endTag("item");
             }
-
         }
 
         target.endTag("items");
@@ -382,6 +385,7 @@ public class MenuBar extends AbstractComponent {
         private Resource itsIcon;
         private MenuItem itsParent;
         private boolean enabled = true;
+        private boolean isSeparator = false;
 
         /**
          * Constructs a new menu item that can optionally have an icon and a
@@ -410,7 +414,27 @@ public class MenuBar extends AbstractComponent {
          * @return True if this item has children
          */
         public boolean hasChildren() {
-            return itsChildren != null;
+            return !isSeparator() && itsChildren != null;
+        }
+
+        /**
+         * Adds a separator to this menu. A separator is a way to visually group
+         * items in a menu, to make it easier for users to find what they are
+         * looking for in the menu.
+         * 
+         * @author Jouni Koivuviita / IT Mill Ltd.
+         * @since 6.2.0
+         */
+        public MenuBar.MenuItem addSeparator() {
+            MenuItem item = addItem("", null, null);
+            item.setSeparator(true);
+            return item;
+        }
+
+        public MenuBar.MenuItem addSeparatorBefore(MenuItem itemToAddBefore) {
+            MenuItem item = addItemBefore("", null, null, itemToAddBefore);
+            item.setSeparator(true);
+            return item;
         }
 
         /**
@@ -439,8 +463,12 @@ public class MenuBar extends AbstractComponent {
          */
         public MenuBar.MenuItem addItem(String caption, Resource icon,
                 MenuBar.Command command) {
+            if (isSeparator()) {
+                throw new UnsupportedOperationException(
+                        "Cannot add items to a separator");
+            }
             if (caption == null) {
-                throw new IllegalArgumentException("caption cannot be null");
+                throw new IllegalArgumentException("Caption cannot be null");
             }
 
             if (itsChildren == null) {
@@ -483,7 +511,6 @@ public class MenuBar extends AbstractComponent {
                 newItem = new MenuItem(caption, icon, command);
                 newItem.setParent(this);
                 itsChildren.add(index, newItem);
-
             } else {
                 newItem = addItem(caption, icon, command);
             }
@@ -546,7 +573,10 @@ public class MenuBar extends AbstractComponent {
          * @return The number of child items
          */
         public int getSize() {
-            return itsChildren.size();
+            if (itsChildren != null) {
+                return itsChildren.size();
+            }
+            return -1;
         }
 
         /**
@@ -604,8 +634,8 @@ public class MenuBar extends AbstractComponent {
                 if (itsChildren.isEmpty()) {
                     itsChildren = null;
                 }
+                requestRepaint();
             }
-            requestRepaint();
         }
 
         /**
@@ -615,8 +645,8 @@ public class MenuBar extends AbstractComponent {
             if (itsChildren != null) {
                 itsChildren.clear();
                 itsChildren = null;
+                requestRepaint();
             }
-            requestRepaint();
         }
 
         /**
@@ -638,6 +668,15 @@ public class MenuBar extends AbstractComponent {
             return enabled;
         }
 
+        private void setSeparator(boolean isSeparator) {
+            this.isSeparator = isSeparator;
+            requestRepaint();
+        }
+
+        public boolean isSeparator() {
+            return isSeparator;
+        }
+
     }// class MenuItem
 
 }// class MenuBar