MenuBar fixes (again)

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

svn changeset:9722/svn branch:6.2

WebContent/VAADIN/themes/base/menubar/menubar.css

@@ -56,10 +56,22 @@
 	margin-right: 0;
 	margin-top: -2px;
 }
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	position: relative;
+	margin-left: 0;
+}
 .v-menubar-menuitem-disabled {
 	color: #999;
 }
 .v-menubar-more-menuitem {
 	/* Arial has the most coverage for geometric entity characters */
 	font-family: arial, helvetica, sans-serif;
+}
+.v-menubar-separator span {
+	display: block;
+	text-indent: -9999px;
+	height: 1px;
+	margin: 3px 0;
+	overflow: hidden;
+	background: #ddd;
 }

WebContent/VAADIN/themes/base/styles.css

@@ -679,6 +679,10 @@ div.v-app-loading {
 	margin-right: 0;
 	margin-top: -2px;
 }
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	position: relative;
+	margin-left: 0;
+}
 .v-menubar-menuitem-disabled {
 	color: #999;
 }
@@ -686,6 +690,14 @@ div.v-app-loading {
 	/* Arial has the most coverage for geometric entity characters */
 	font-family: arial, helvetica, sans-serif;
 }
+.v-menubar-separator span {
+	display: block;
+	text-indent: -9999px;
+	height: 1px;
+	margin: 3px 0;
+	overflow: hidden;
+	background: #ddd;
+}
 
 .v-Notification {
 	background: #999;

WebContent/VAADIN/themes/reindeer/menubar/menubar.css

@@ -41,12 +41,17 @@
 	background-image: url(img/menu-sel-bg.png); /** sprite-ref: verticals; sprite-alignment: repeat */
 }
 .v-menubar-submenu .v-menubar-submenu-indicator {
-	background: transparent url(img/submenu-icon.png) no-repeat right bottom;
+	background: transparent url(img/submenu-icon.png) no-repeat right 70%;
 	width: 16px;
 	margin: 0 -20px 0 5px;
 	text-indent: -999px;
 	vertical-align: middle;
 }
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	margin: 0 -20px 0 0;
+	position: relative;
+	right: -4px;
+}
 .v-menubar-submenu .v-menubar-menuitem-selected .v-menubar-submenu-indicator {
 	background-image: url(img/submenu-icon-hover.png);
 }

WebContent/VAADIN/themes/reindeer/styles.css

@@ -679,6 +679,10 @@ div.v-app-loading {
 	margin-right: 0;
 	margin-top: -2px;
 }
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	position: relative;
+	margin-left: 0;
+}
 .v-menubar-menuitem-disabled {
 	color: #999;
 }
@@ -686,6 +690,14 @@ div.v-app-loading {
 	/* Arial has the most coverage for geometric entity characters */
 	font-family: arial, helvetica, sans-serif;
 }
+.v-menubar-separator span {
+	display: block;
+	text-indent: -9999px;
+	height: 1px;
+	margin: 3px 0;
+	overflow: hidden;
+	background: #ddd;
+}
 
 .v-Notification {
 	background: #999;
@@ -3058,12 +3070,17 @@ td.v-datefield-calendarpanel-nextyear {
   background-position: left -423px;
 }
 .v-menubar-submenu .v-menubar-submenu-indicator {
-	background: transparent url(menubar/img/submenu-icon.png) no-repeat right bottom;
+	background: transparent url(menubar/img/submenu-icon.png) no-repeat right 70%;
 	width: 16px;
 	margin: 0 -20px 0 5px;
 	text-indent: -999px;
 	vertical-align: middle;
 }
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	margin: 0 -20px 0 0;
+	position: relative;
+	right: -4px;
+}
 .v-menubar-submenu .v-menubar-menuitem-selected .v-menubar-submenu-indicator {
 	background-image: url(menubar/img/submenu-icon-hover.png);
 }

WebContent/VAADIN/themes/runo/menubar/menubar.css

@@ -24,12 +24,17 @@
 	border-left: 1px solid #d0d4d5;
 }
 .v-menubar-submenu .v-menubar-menuitem {
-	padding: 2px 10px;
+	padding: 2px 16px 2px 10px;
 }
 .v-menubar-submenu .v-menubar-menuitem-selected {
 	color: #fff;
 	background: #5daee8;
 }
 .v-menubar-submenu .v-menubar-submenu-indicator {
-	margin-right: -3px;
+	margin-right: -12px;
+	height: 14px;
+}
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	margin-right: -12px;
+	right: -3px;
 }

WebContent/VAADIN/themes/runo/styles.css

@@ -679,6 +679,10 @@ div.v-app-loading {
 	margin-right: 0;
 	margin-top: -2px;
 }
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	position: relative;
+	margin-left: 0;
+}
 .v-menubar-menuitem-disabled {
 	color: #999;
 }
@@ -686,6 +690,14 @@ div.v-app-loading {
 	/* Arial has the most coverage for geometric entity characters */
 	font-family: arial, helvetica, sans-serif;
 }
+.v-menubar-separator span {
+	display: block;
+	text-indent: -9999px;
+	height: 1px;
+	margin: 3px 0;
+	overflow: hidden;
+	background: #ddd;
+}
 
 .v-Notification {
 	background: #999;
@@ -2177,14 +2189,19 @@ div.v-tree-node-leaf {
 	border-left: 1px solid #d0d4d5;
 }
 .v-menubar-submenu .v-menubar-menuitem {
-	padding: 2px 10px;
+	padding: 2px 16px 2px 10px;
 }
 .v-menubar-submenu .v-menubar-menuitem-selected {
 	color: #fff;
 	background: #5daee8;
 }
 .v-menubar-submenu .v-menubar-submenu-indicator {
-	margin-right: -3px;
+	margin-right: -12px;
+	height: 14px;
+}
+.v-ie7 .v-menubar-submenu .v-menubar-submenu-indicator {
+	margin-right: -12px;
+	right: -3px;
 }
 
 .v-Notification {

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;
+        }
     }
 
     /**

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