wip(ui5-side-navigation): fix overlapping issue

packages/fiori/cypress/specs/SideNavigation.cy.tsx

@@ -76,6 +76,64 @@ describe("Side Navigation Rendering", () => {
 			.should("have.attr", "design", "Action");
 	});
 
+	it.only("Tests overflow item visibility and items in overflow", () => {
+		cy.mount(
+			<SideNavigation id="sideNav" collapsed={true} style={{ height: "430px" }}>
+				<SideNavigationItem text="Item 1" icon={home}></SideNavigationItem>
+				<SideNavigationItem text="Item 2" icon={home}></SideNavigationItem>
+
+				<SideNavigationGroup>
+					<SideNavigationItem text="Item 3" icon={home}></SideNavigationItem>
+					<SideNavigationItem text="Item 4" icon={home}></SideNavigationItem>
+				</SideNavigationGroup>
+
+				<SideNavigationGroup>
+					<SideNavigationItem text="Item 5" icon={home}></SideNavigationItem>
+					<SideNavigationItem text="Item 6" icon={home}></SideNavigationItem>
+				</SideNavigationGroup>
+
+				<SideNavigationItem text="Outer" selected={true} icon={home}></SideNavigationItem>
+
+				<SideNavigationItem slot="fixedItems" text="Legal" icon={home}></SideNavigationItem>
+			</SideNavigation>
+		);
+
+		cy.get("#sideNav")
+			.should("be.visible");
+
+		// the overflow item should be visible
+		cy.get("#sideNav")
+			.shadow()
+			.find(".ui5-sn-item-overflow:not(.ui5-sn-item-hidden)")
+			.should("be.visible")
+			.realClick();
+
+		// exactly 2 items should be in the overflow menu
+		cy.get("#sideNav")
+			.shadow()
+			.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item]")
+			.should("have.length", 2);
+
+
+		// check the last separator calculations
+		// when the height is 440px, also 2 items should go to the overflow
+		cy.get("#sideNav")
+			.invoke("attr", "style", "height:440px");
+
+		// the overflow item should be visible
+		cy.get("#sideNav")
+			.shadow()
+			.find(".ui5-sn-item-overflow:not(.ui5-sn-item-hidden)")
+			.should("be.visible")
+			.realClick();
+
+		// exactly 2 items should be in the overflow menu
+		cy.get("#sideNav")
+			.shadow()
+			.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item]")
+			.should("have.length", 2);
+	});
+
 	it("Tests accessibility", () => {
 		cy.mount(
 			<SideNavigation id="sideNav" accessibleName="Main">
@@ -1369,7 +1427,7 @@ describe("Side Navigation Accessibility", () => {
 			.shadow()
 			.find(".ui5-sn-item-overflow")
 			.realClick();
-	
+
 		// Assert
 		cy.get("#sideNav")
 			.shadow()

packages/fiori/src/SideNavigation.ts

@@ -313,8 +313,6 @@ class SideNavigation extends UI5Element {
 		(popover?.opener as HTMLElement)?.classList.remove("ui5-sn-item-active");
 	}
 
-	_bn?: SideNavigationSelectableItemBase;
-
 	_onMenuClose() {
 		const menu = this.getOverflowPopover();
 		if (!menu._popover.preventFocusRestore) {
@@ -543,8 +541,7 @@ class SideNavigation extends UI5Element {
 			return null;
 		}
 
-		const overflowItem = this._overflowItem!;
-		const flexibleContentDomRef: HTMLElement = domRef.querySelector(".ui5-sn-flexible")!;
+		const overflowItem = this._overflowItem;
 		if (!overflowItem) {
 			return null;
 		}
@@ -553,60 +550,129 @@ class SideNavigation extends UI5Element {
 
 		const overflowItems = this.overflowItems;
 
-		let itemsHeight = overflowItems.reduce<number>((sum, itemRef) => {
-			if (!itemRef) {
-				return sum;
-			}
-			itemRef.classList.remove("ui5-sn-item-hidden");
-			return sum + itemRef.offsetHeight;
-		}, 0);
+		let itemsHeight = this._calculateItemsHeight(overflowItems);
 
+		const flexibleContentDomRef: HTMLElement = domRef.querySelector(".ui5-sn-flexible")!;
 		const { paddingTop, paddingBottom } = window.getComputedStyle(flexibleContentDomRef);
 		const listHeight = flexibleContentDomRef?.offsetHeight - parseInt(paddingTop) - parseInt(paddingBottom);
 
-		if (itemsHeight <= listHeight) {
+		if (itemsHeight < listHeight) {
 			return;
 		}
 
 		overflowItem.classList.remove("ui5-sn-item-hidden");
 
 		itemsHeight = overflowItem.offsetHeight;
 
-		const selectedItem = overflowItems.filter(isInstanceOfSideNavigationSelectableItemBase).find(item => item._selected);
+		const navItems = overflowItems.filter(isInstanceOfSideNavigationSelectableItemBase);
+		const selectedItem = navItems.find(item => item._selected);
+
+		itemsHeight += this._getSelectedItemHeight(overflowItems, selectedItem) + 1; // +1 for sub-pixel rounding
+		itemsHeight += this._getLastSeparatorHeight(navItems, overflowItems);
+
+		this._updateItemsVisibility(overflowItems, selectedItem, itemsHeight, listHeight);
+
+		this._flexibleItemNavigation._init();
+	}
+
+	_calculateItemsHeight(overflowItems: Array<HTMLElement>) {
+		return overflowItems.reduce<number>((sum, itemRef) => {
+			if (!itemRef) {
+				return sum;
+			}
+			itemRef.classList.remove("ui5-sn-item-hidden");
+
+			let itemDomRef = itemRef;
+
+			if (isInstanceOfSideNavigationItemBase(itemRef) && itemRef.getDomRef()) {
+				itemDomRef = itemRef.getDomRef()!;
+			}
+
+			const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef);
+
+			return sum + itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom);
+		}, 0);
+	}
+
+	_getSelectedItemHeight(overflowItems: Array<HTMLElement>, selectedItem: SideNavigationSelectableItemBase | undefined) {
+		if (!selectedItem) {
+			return 0;
+		}
+
+		let height = 0;
 
 		if (selectedItem) {
 			const selectedItemDomRef = selectedItem.getDomRef();
 
 			if (selectedItemDomRef) {
 				const { marginTop, marginBottom } = window.getComputedStyle(selectedItemDomRef);
-				itemsHeight += selectedItemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom);
+				height += selectedItemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom);
+			}
+
+			const indexOf = overflowItems.indexOf(selectedItem);
+			const itemAfterSelected = overflowItems[indexOf + 1];
+			if (itemAfterSelected && !isInstanceOfSideNavigationItemBase(itemAfterSelected)) {
+				height += itemAfterSelected.offsetHeight;
 			}
 		}
 
-		overflowItems.forEach(item => {
+		return height;
+	}
+
+	_getLastSeparatorHeight(navItems: Array<SideNavigationSelectableItemBase>, overflowItems: Array<HTMLElement>) {
+		const lastNonSelectedItem = navItems.findLast(item => !item._selected);
+		if (!lastNonSelectedItem) {
+			return 0;
+		}
+
+		const indexOf = overflowItems.indexOf(lastNonSelectedItem);
+		const nextSeparator = overflowItems[indexOf + 1];
+
+		if (nextSeparator && !isInstanceOfSideNavigationItemBase(nextSeparator)) {
+			return nextSeparator.offsetHeight;
+		}
+
+		return 0;
+	}
+
+	_updateItemsVisibility(overflowItems: Array<HTMLElement>, selectedItem: SideNavigationSelectableItemBase | undefined, itemsHeight: number, listHeight: number) {
+		for (let i = 0; i < overflowItems.length; i++) {
+			const item = overflowItems[i];
+
 			if (!item || item === selectedItem) {
-				return;
+				// eslint-disable-next-line no-continue
+				continue;
 			}
 
 			let itemDomRef;
 
-			if (isInstanceOfSideNavigationItemBase(item) && item.getDomRef()) {
+			if (isInstanceOfSideNavigationItemBase(item)) {
 				itemDomRef = item.getDomRef();
-			} else {
-				itemDomRef = item;
 			}
 
-			if (itemDomRef) {
-				const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef);
-				itemsHeight += itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom);
+			if (!itemDomRef) {
+				// eslint-disable-next-line no-continue
+				continue;
+			}
 
-				if (itemsHeight > listHeight) {
-					item.classList.add("ui5-sn-item-hidden");
-				}
+			const { marginTop, marginBottom } = window.getComputedStyle(itemDomRef);
+			itemsHeight += itemDomRef.offsetHeight + parseFloat(marginTop) + parseFloat(marginBottom);
+
+			// if the next item is a separator, the item and the separator
+			// should be hidden together, so we need to add the separator height to the itemsHeight
+			const nextItem = overflowItems[i + 1];
+			let nextItemDomRef;
+			if (nextItem && !isInstanceOfSideNavigationItemBase(nextItem)) {
+				nextItemDomRef = nextItem;
+				itemsHeight += nextItemDomRef.offsetHeight;
+				i++;
 			}
-		});
 
-		this._flexibleItemNavigation._init();
+			if (itemsHeight > listHeight) {
+				item.classList.add("ui5-sn-item-hidden");
+				nextItemDomRef?.classList.add("ui5-sn-item-hidden");
+			}
+		}
 	}
 
 	_findFocusedItem(items: Array<SideNavigationItemBase>): SideNavigationItemBase | undefined {

packages/fiori/src/themes/SideNavigation.css

@@ -85,4 +85,12 @@
 
 .ui5-sn-item-overflow {
 	margin-top: auto;
+}
+
+.ui5-sn-spacer {
+	margin: var(--_ui5_side_navigation_navigation_separator_margin);
+	height: var(--_ui5_side_navigation_navigation_separator_height);
+	min-height: var(--_ui5_side_navigation_navigation_separator_height);
+	background-color: var(--_ui5_side_navigation_navigation_separator_background_color);
+	border-radius: var(--_ui5_side_navigation_navigation_separator_radius);
 }

packages/fiori/src/themes/SideNavigationGroup.css

@@ -22,12 +22,4 @@
 .ui5-sn-item-group-below-group.ui5-sn-item-separator,
 .ui5-sn-item-group-below-group .ui5-sn-item-separator:first-child {
 	display: none;
-}
-
-.ui5-sn-spacer {
-	margin: var(--_ui5_side_navigation_navigation_separator_margin);
-	height: var(--_ui5_side_navigation_navigation_separator_height);
-	min-height: var(--_ui5_side_navigation_navigation_separator_height);
-	background-color: var(--_ui5_side_navigation_navigation_separator_background_color);
-	border-radius: var(--_ui5_side_navigation_navigation_separator_radius);
-}
+}

packages/fiori/test/pages/SideNavigationOverflowOverlap.html

@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta charset="utf-8">
+	<title>Side Navigation Only</title>
+	<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
+	<script src="%VITE_BUNDLE_PATH%" type="module"></script>
+</head>
+
+<body style="background-color: var(--sapBackgroundColor); height: 100vh;">
+<ui5-side-navigation style="height: 430px" id="sn1" collapsed>
+	<!-- Top items: enough items to overflow into fixed bottom area on short screens -->
+	<ui5-side-navigation-item
+		text="Item 1"
+		icon="home"
+	></ui5-side-navigation-item>
+	<ui5-side-navigation-item
+		text="Item 2"
+		icon="home"
+	></ui5-side-navigation-item>
+
+	<ui5-side-navigation-group>
+		<ui5-side-navigation-item
+			text="Item 3"
+			icon="home"
+		></ui5-side-navigation-item>
+		<ui5-side-navigation-item
+			text="Item 4"
+			icon="home"
+		></ui5-side-navigation-item>
+	</ui5-side-navigation-group>
+
+	<ui5-side-navigation-group>
+		<ui5-side-navigation-item
+			text="Item 5"
+			icon="home"
+		></ui5-side-navigation-item>
+		<ui5-side-navigation-item
+			text="Item 6"
+			icon="home"
+		></ui5-side-navigation-item>
+	</ui5-side-navigation-group>
+
+	<ui5-side-navigation-item
+		text="Outer"
+		selected
+		icon="home"
+	></ui5-side-navigation-item>
+
+	<!-- Fixed bottom items: these should never be overlapped by top items -->
+	<ui5-side-navigation-item
+		slot="fixedItems"
+		text="Legal"
+		icon="home"
+	></ui5-side-navigation-item>
+</ui5-side-navigation>
+</body>
+</html>