ZK-5025: redundant selection highlight on a menupopup

zkoss · Sep 7, 2023 · 35c844e · 35c844e
1 parent 00bdb92
commit 35c844e
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 6 deletions.
diff --git a/zkdoc/release-note b/zkdoc/release-note
@@ -62,6 +62,7 @@ ZK 10.0.0
   ZK-5539: $init() will call twice on Keikai component
   ZK-5540: zk.wpd with browser condition won't work with ZK 10
   ZK-5530: users cannot focus on the icons on the Calendar header
+  ZK-5025: redundant selection highlight on a menupopup
 
 * Upgrade Notes
   + The Java binary-compatible level is Java 11 since ZK 10.

diff --git a/zktest/src/main/webapp/test2/B100-ZK-5025.zul b/zktest/src/main/webapp/test2/B100-ZK-5025.zul
@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+B100-ZK-5025.zul
+
+        Purpose:
+
+        Description:
+
+        History:
+                Tue Sep 05 11:12:30 CST 2023, Created by jamson
+
+Copyright (C) 2023 Potix Corporation. All Rights Reserved.
+-->
+<zk xmlns:h="xhtml">
+	<h:h1 style="color:blue">menuitem-hover</h:h1>
+	<h:h1 style="color:red">menu-hover</h:h1>
+	<h:h1 style="color:green">menuitem:hover</h:h1>
+	<h:h1 style="color:orange">menu:hover</h:h1>
+	<menubar>
+		<menu label="Nested">
+			<menupopup>
+				<menuitem label="New"/>
+				<menuitem label="Open"/>
+				<menuitem label="Save"/>
+				<menuitem label="Help"/>
+				<menu label="other">
+					<menupopup>
+						<menuitem label="New"/>
+						<menuitem label="Open"/>
+						<menuitem label="Save"/>
+						<menuitem label="Help"/>
+						<menu label="other">
+							<menupopup>
+								<menuitem label="New"/>
+								<menuitem label="Open"/>
+								<menuitem label="Save"/>
+								<menuitem label="Help"/>
+							</menupopup>
+						</menu>
+					</menupopup>
+				</menu>
+			</menupopup>
+		</menu>
+	</menubar>
+	<menubar>
+		<menu label="No Nest">
+			<menupopup>
+				<menuitem label="Help"/>
+				<menuitem label="Help"/>
+				<menuitem label="Help"/>
+			</menupopup>
+		</menu>
+	</menubar>
+</zk>
diff --git a/zktest/src/main/webapp/test2/config.properties b/zktest/src/main/webapp/test2/config.properties
@@ -3158,6 +3158,7 @@ B90-ZK-4431.zul=A,E,Multislider
 ##zats##B100-ZK-5468-Tabbox.zul=A,E,Model,Tabbox
 ##zats##B100-ZK-5468-Tree.zul=A,E,Model,Tree
 ##zats##B100-ZK-5235.zul=A,E,Menu,Menupopup,focus
+##zays##B100-ZK-5025.zul=A,E,Menu,Menupopup,Menuitem,focus,hover
 
 ##
 # Features - 3.0.x version

diff --git a/zul/src/main/resources/web/js/zul/menu/Menuitem.ts b/zul/src/main/resources/web/js/zul/menu/Menuitem.ts
@@ -535,7 +535,23 @@ export class Menuitem extends zul.LabelImageWidget implements zul.LabelImageWidg
 		if (!isTopmost && !this._disabled) {
 			if (this.parent)
 				this.parent.removeActive_();
+			// remove all active
+			for (var mi = this.parent?.firstChild; mi != undefined; mi = mi.nextSibling) {
+				if (mi instanceof Menuitem) {
+					(mi.$class as typeof Menuitem)._rmActive(mi);
+				}
+				if (mi instanceof zul.menu.Menu) {
+					(mi.$class as typeof zul.menu.Menu)._rmActive(mi);
+					var pp = mi.menupopup;
+					if (pp?.isOpen()) {
+						jq(mi.$n_()).removeClass(mi.$s('hover')).removeClass(mi.$s('selected'));
+						pp.close();
+					}
+				}
+			}
+			// add new active
 			(this.$class as typeof Menuitem)._addActive(this);
+			this.focus();
 		}
 	}
 

diff --git a/zul/src/main/resources/web/js/zul/menu/Menupopup.ts b/zul/src/main/resources/web/js/zul/menu/Menupopup.ts
@@ -367,8 +367,26 @@ export class Menupopup extends zul.wgt.Popup {
 			// UP: 1. jump to the previousSibling item
 			// DOWN: 1. jump to the nextSibling item
 			if (w) (w.$class as typeof zul.menu.Menuitem)._rmActive(w);
+			// remove all active
+			for (var mi = this?.firstChild; mi != undefined; mi = mi.nextSibling) {
+				if (mi instanceof zul.menu.Menuitem) {
+					(mi.$class as typeof zul.menu.Menuitem)._rmActive(mi);
+				}
+				if (mi instanceof zul.menu.Menu) {
+					(mi.$class as typeof zul.menu.Menu)._rmActive(mi);
+					var pp = mi.menupopup;
+					if (pp?.isOpen()) {
+						jq(mi.$n_()).removeClass(mi.$s('hover')).removeClass(mi.$s('selected'));
+						pp.close();
+					}
+				}
+			}
+			// add new active
 			w = keyCode == 38 ? _prevChild(this, w) : _nextChild(this, w);
-			if (w) (w.$class as typeof zul.menu.Menuitem)._addActive(w as zul.menu.Menuitem); // FIXME: type of w is inconsistent
+			if (w) {
+				(w.$class as typeof zul.menu.Menuitem)._addActive(w as zul.menu.Menuitem); // FIXME: type of w is inconsistent
+				w.focus();
+			}
 			break;
 		case 37: //LEFT
 			// 1. close the contenthandler (like colorbox), if any

diff --git a/zul/src/main/resources/web/js/zul/menu/less/menu.less b/zul/src/main/resources/web/js/zul/menu/less/menu.less
@@ -115,12 +115,11 @@
 		white-space: nowrap;
 		min-height: @menuContentMinHeight;
 		z-index: 20; // the 20 is greater than menupopup-separator's z-index
-
 		&:hover {
-			.contentStyle(@menuItemHoverColor, @menuItemHoverBackground);
+			.contentStyle(@menuItemHoverColor, @menuItemBackground);
 		}
 		&:focus {
-			.contentStyle(@menuItemHoverColor, @menuItemHoverBackground);
+			.contentStyle(@menuItemHoverColor, @menuItemBackground);
 		}
 		&:active {
 			.contentStyle(@menuItemActiveColor, @menuItemActiveBackground);
@@ -217,10 +216,10 @@
 		color: @menuPopupItemColor;
 		background: @menuPopupItemBackground;
 		&:hover {
-			.contentStyle(@menuPopupItemHoverColor, @menuPopupItemHoverBackground);
+			.contentStyle(@menuPopupItemHoverColor, @menuPopupItemBackground);
 		}
 		&:focus {
-			.contentStyle(@menuPopupItemHoverColor, @menuPopupItemHoverBackground);
+			.contentStyle(@menuPopupItemHoverColor, @menuPopupItemBackground);
 		}
 		&:active {
 			.contentStyle(@menuPopupItemActiveColor, @menuPopupItemActiveBackground);