fix(ui5-table): keep focused cell visible next to sticky columns

packages/main/cypress/specs/TableNavigation.cy.tsx

@@ -4,6 +4,9 @@ import TableHeaderCell from "../../src/TableHeaderCell.js";
 import TableRow from "../../src/TableRow.js";
 import TableCell from "../../src/TableCell.js";
 import TableGrowing from "../../src/TableGrowing.js";
+import TableSelectionMulti from "../../src/TableSelectionMulti.js";
+import TableRowAction from "../../src/TableRowAction.js";
+import TableRowActionNavigation from "../../src/TableRowActionNavigation.js";
 import Bar from "../../src/Bar.js";
 import Title from "../../src/Title.js";
 
@@ -432,4 +435,183 @@ describe("Table - Keyboard Navigation with Fixed Headers", () => {
 			});
 		}
 	});
+});
+
+describe("Table - Keyboard Navigation with overflowMode=Scroll", () => {
+	const sixColumnHeader = (
+		<TableHeaderRow slot="headerRow">
+			<TableHeaderCell minWidth="180px">Column A</TableHeaderCell>
+			<TableHeaderCell minWidth="180px">Column B</TableHeaderCell>
+			<TableHeaderCell minWidth="180px">Column C</TableHeaderCell>
+			<TableHeaderCell minWidth="180px">Column D</TableHeaderCell>
+			<TableHeaderCell minWidth="180px">Column E</TableHeaderCell>
+			<TableHeaderCell minWidth="180px">Column F</TableHeaderCell>
+		</TableHeaderRow>
+	);
+
+	const wideRow = (rowId: string, keyLabel: string) => (
+		<TableRow id={rowId} rowKey={rowId}>
+			<TableCell>{keyLabel}-A</TableCell>
+			<TableCell>{keyLabel}-B</TableCell>
+			<TableCell>{keyLabel}-C</TableCell>
+			<TableCell>{keyLabel}-D</TableCell>
+			<TableCell>{keyLabel}-E</TableCell>
+			<TableCell>{keyLabel}-F</TableCell>
+		</TableRow>
+	);
+
+	const focusFirstCell = (key: "ArrowRight" | "ArrowLeft", pressesToFirstCell: number) => {
+		// Cypress#focus() rejects the custom element, and .click()
+		// fail because the sticky column partially covers the row
+		cy.get("#row-1").then($row => $row[0].focus());
+		for (let p = 0; p < pressesToFirstCell; p++) {
+			cy.realPress(key);
+		}
+		cy.get("#row-1").children("ui5-table-cell").eq(0).should("be.focused");
+	};
+
+	const walkAndAssertClearOfSticky = (
+		key: "ArrowRight" | "ArrowLeft",
+		stickyCellId: string,
+		assertClear: (stickyRect: DOMRect, focusedRect: DOMRect) => void,
+	) => {
+		for (let i = 1; i < 6; i++) {
+			cy.realPress(key);
+			cy.get("#row-1").children("ui5-table-cell").eq(i).as("focused").should("be.focused");
+			cy.get("#row-1").shadow().find(stickyCellId).then($sticky => {
+				const stickyRect = $sticky[0].getBoundingClientRect();
+				cy.get("@focused").then($f => assertClear(stickyRect, $f[0].getBoundingClientRect()));
+			});
+		}
+	};
+
+	it("horizontal arrow keys keep focus past the sticky selection column (LTR)", () => {
+		cy.mount(
+			<div style="width:400px;">
+				<Table id="table" overflowMode="Scroll">
+					<TableSelectionMulti slot="features" />
+					{sixColumnHeader}
+					{wideRow("row-1", "R1")}
+				</Table>
+			</div>
+		);
+
+		focusFirstCell("ArrowRight", 2);
+		walkAndAssertClearOfSticky("ArrowRight", "#selection-cell", (sel, focused) => {
+			expect(focused.left).to.be.at.least(sel.right - 0.5);
+		});
+	});
+
+	it("horizontal arrow keys keep focus past the sticky actions/navigated column (LTR)", () => {
+		cy.mount(
+			<div style="width:400px;">
+				<Table id="table" overflowMode="Scroll" rowActionCount={1}>
+					{sixColumnHeader}
+					<TableRow id="row-1" rowKey="row-1" navigated={true}>
+						<TableCell>R1-A</TableCell>
+						<TableCell>R1-B</TableCell>
+						<TableCell>R1-C</TableCell>
+						<TableCell>R1-D</TableCell>
+						<TableCell>R1-E</TableCell>
+						<TableCell>R1-F</TableCell>
+						<TableRowAction slot="actions" icon="edit" text="Edit" />
+						<TableRowActionNavigation slot="actions" />
+					</TableRow>
+				</Table>
+			</div>
+		);
+
+		focusFirstCell("ArrowRight", 1);
+		walkAndAssertClearOfSticky("ArrowRight", "#actions-cell", (act, focused) => {
+			expect(focused.right).to.be.at.most(act.left + 0.5);
+		});
+	});
+
+	it("horizontal arrow keys keep focus past sticky columns (RTL)", () => {
+		cy.mount(
+			<div dir="rtl" style="width:400px;">
+				<Table id="table" overflowMode="Scroll">
+					<TableSelectionMulti slot="features" />
+					{sixColumnHeader}
+					{wideRow("row-1", "R1")}
+				</Table>
+			</div>
+		);
+
+		// In RTL, ArrowLeft moves to the next logical column.
+		focusFirstCell("ArrowLeft", 2);
+		walkAndAssertClearOfSticky("ArrowLeft", "#selection-cell", (sel, focused) => {
+			expect(focused.right).to.be.at.most(sel.left + 0.5);
+		});
+	});
+
+	it("cell wider than viewport is aligned to leading edge", () => {
+		cy.mount(
+			<div style="width:300px;">
+				<Table id="table" overflowMode="Scroll">
+					<TableSelectionMulti slot="features" />
+					<TableHeaderRow slot="headerRow">
+						<TableHeaderCell minWidth="600px">Wide Column</TableHeaderCell>
+						<TableHeaderCell minWidth="180px">Column B</TableHeaderCell>
+					</TableHeaderRow>
+					<TableRow id="row-1" rowKey="row-1">
+						<TableCell>Wide content that is far wider than the visible viewport</TableCell>
+						<TableCell>B</TableCell>
+					</TableRow>
+				</Table>
+			</div>
+		);
+
+		focusFirstCell("ArrowRight", 2);
+		cy.get("#row-1").shadow().find("#selection-cell").then($sel => {
+			const selRight = $sel[0].getBoundingClientRect().right;
+			cy.get("#row-1").children("ui5-table-cell").eq(0).then($w => {
+				expect(Math.abs($w[0].getBoundingClientRect().left - selRight)).to.be.at.most(1.5);
+			});
+		});
+	});
+
+	const verticalStickyTable = (
+		<Table id="table" overflowMode="Scroll" style="height: 150px">
+			<TableHeaderRow slot="headerRow" sticky>
+				<TableHeaderCell>Column A</TableHeaderCell>
+			</TableHeaderRow>
+			{Array.from({ length: 30 }).map((_, i) => (
+				<TableRow id={`row-${i}`} rowKey={`${i}`}>
+					<TableCell>Cell {i}</TableCell>
+				</TableRow>
+			))}
+		</Table>
+	);
+
+	// Scrolls down, navigates back up with ArrowUp and asserts the focused row is not hidden under the sticky header.
+	const assertFocusedRowNotHiddenUnderHeader = () => {
+		cy.get("#row-0").then($row => $row[0].focus());
+		for (let i = 0; i < 20; i++) {
+			cy.realPress("ArrowDown");
+		}
+		cy.get("#row-20").should("be.focused");
+
+		for (let i = 0; i < 5; i++) {
+			cy.realPress("ArrowUp");
+		}
+		cy.get("#row-15").should("be.focused");
+
+		cy.get("#table").find("[ui5-table-header-row]").then($header => {
+			const headerRect = $header[0].getBoundingClientRect();
+			cy.get("#row-15").then($row => {
+				expect($row[0].getBoundingClientRect().top).to.be.at.least(headerRect.bottom - 0.5);
+			});
+		});
+	};
+
+	it("focused row is not hidden under the sticky header (table scrolls)", () => {
+		cy.mount(verticalStickyTable);
+		assertFocusedRowNotHiddenUnderHeader();
+	});
+
+	it("focused row is not hidden under the sticky header (wrapper scrolls)", () => {
+		cy.mount(<div style="height: 150px; overflow: auto;">{verticalStickyTable}</div>);
+		assertFocusedRowNotHiddenUnderHeader();
+	});
 });

packages/main/src/Table.ts

@@ -14,7 +14,7 @@ import TableDragAndDrop from "./TableDragAndDrop.js";
 import TableCustomAnnouncement from "./TableCustomAnnouncement.js";
 import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
 import {
-	findVerticalScrollContainer, scrollElementIntoView, isFeature, isValidColumnWidth,
+	findVerticalScrollContainer, computeAxisScrollDelta, isFeature, isValidColumnWidth,
 } from "./TableUtils.js";
 import { getEffectiveAriaLabelText } from "@ui5/webcomponents-base/dist/util/AccessibilityTextsHelper.js";
 import type DropIndicator from "./DropIndicator.js";
@@ -562,8 +562,23 @@ class Table extends UI5Element {
 			return;
 		}
 
-		// Handles focus in the table, when the focus is below a sticky element
-		scrollElementIntoView(this._scrollContainer, e.target as HTMLElement, this._stickyElements, this.effectiveDir === "rtl");
+		this._scrollElementIntoView(e.target as HTMLElement);
+	}
+
+	_scrollElementIntoView(element: HTMLElement) {
+		const verticalStickyElements = this.headerRow.filter(row => row.sticky);
+		if (verticalStickyElements.length) {
+			const verticalScrollContainer = findVerticalScrollContainer(this._tableElement, true);
+			const deltaY = computeAxisScrollDelta(element, verticalScrollContainer, verticalStickyElements, "y");
+			verticalScrollContainer.scrollBy({ top: deltaY });
+		}
+
+		const horizontalStickyElements = this.overflowMode === "Scroll" ? this.headerRow[0]._stickyCells : [];
+		if (horizontalStickyElements.length) {
+			const horizontalScrollContainer = this._tableElement;
+			const deltaX = computeAxisScrollDelta(element, horizontalScrollContainer, horizontalStickyElements, "x");
+			horizontalScrollContainer.scrollBy({ left: deltaX });
+		}
 	}
 
 	_onGrow() {
@@ -704,12 +719,6 @@ class Table extends UI5Element {
 		return this._getVirtualizer() ? this._tableElement : findVerticalScrollContainer(this);
 	}
 
-	get _stickyElements() {
-		const stickyRows = this.headerRow.filter(row => row.sticky);
-		const stickyColumns = this.headerRow[0]._stickyCells as TableHeaderCell[];
-		return [...stickyRows, ...stickyColumns];
-	}
-
 	get _effectiveNoDataText() {
 		return this.noDataText || Table.i18nBundle.getText(TABLE_NO_DATA);
 	}

packages/main/src/TableRow.ts

@@ -121,9 +121,6 @@ class TableRow extends TableRowBase<TableCell> {
 	@query("#popin-cell")
 	_popinCell?: TableCell;
 
-	@query("#actions-cell")
-	_actionsCell?: TableCell;
-
 	onBeforeRendering() {
 		super.onBeforeRendering();
 		this.ariaRowIndex = (this.role === "row") ? `${this._rowIndex + 2}` : null;

packages/main/src/TableRowBase.ts

@@ -46,6 +46,9 @@ abstract class TableRowBase<TCell extends TableCellBase = TableCellBase> extends
 	@query("#selection-cell")
 	_selectionCell?: HTMLElement;
 
+	@query("#actions-cell")
+	_actionsCell?: HTMLElement;
+
 	@query("#navigated-cell")
 	_navigatedCell?: HTMLElement;
 
@@ -162,7 +165,7 @@ abstract class TableRowBase<TCell extends TableCellBase = TableCellBase> extends
 	}
 
 	get _stickyCells() {
-		return [this._selectionCell, ...this.cells, this._navigatedCell].filter(cell => cell?.hasAttribute("fixed"));
+		return [this._selectionCell, this._actionsCell, this._navigatedCell].filter(Boolean) as HTMLElement[];
 	}
 
 	get _i18nRowSelector(): string {

packages/main/src/TableUtils.ts

@@ -16,10 +16,10 @@ const findRowInPath = (composedPath: Array<EventTarget>) => {
 	return composedPath.find((el: EventTarget) => el instanceof HTMLElement && el.hasAttribute("ui5-table-row")) as TableRow;
 };
 
-const findVerticalScrollContainer = (element: HTMLElement): HTMLElement => {
+const findVerticalScrollContainer = (element: HTMLElement, requireOverflow = false): HTMLElement => {
 	while (element) {
 		const { overflowY } = window.getComputedStyle(element);
-		if (overflowY === "auto" || overflowY === "scroll") {
+		if ((overflowY === "auto" || overflowY === "scroll") && (!requireOverflow || element.scrollHeight > element.clientHeight)) {
 			return element;
 		}
 
@@ -33,43 +33,59 @@ const findVerticalScrollContainer = (element: HTMLElement): HTMLElement => {
 	return document.scrollingElement as HTMLElement || document.documentElement;
 };
 
-const scrollElementIntoView = (scrollContainer: HTMLElement, element: HTMLElement, stickyElements: HTMLElement[], isRtl: boolean) => {
-	if (stickyElements.length === 0) {
-		return;
-	}
-
+type Axis = "x" | "y";
+
+const AXIS_PROPS = {
+	x: { start: "left", end: "right", size: "width" },
+	y: { start: "top", end: "bottom", size: "height" },
+} as const;
+
+// Computes the scroll delta needed to bring an element into view within a scroll container, considering sticky elements that may be in the way
+const computeAxisScrollDelta = (
+	element: HTMLElement,
+	scrollContainer: HTMLElement,
+	stickyElements: HTMLElement[],
+	axis: Axis,
+): number => {
+	const { start, end, size } = AXIS_PROPS[axis];
 	const elementRect = element.getBoundingClientRect();
-	const inline = isRtl ? "right" : "left";
+	const scrollContainerRect = scrollContainer.getBoundingClientRect();
 
-	const { x: stickyX, y: stickyY } = stickyElements.reduce(({ x, y }, stickyElement) => {
-		const { top, [inline]: inlineStart } = getComputedStyle(stickyElement);
-		const stickyElementRect = stickyElement.getBoundingClientRect();
-		if (top !== "auto" && stickyElementRect.bottom > elementRect.top) {
-			y = Math.max(y, stickyElementRect.bottom);
-		}
-		if (inlineStart !== "auto") {
-			if (!isRtl && stickyElementRect.right > elementRect.left) {
-				x = Math.max(x, stickyElementRect.right);
-			} else if (isRtl && stickyElementRect.left < elementRect.right) {
-				x = Math.min(x, stickyElementRect.left);
-			}
+	let pinStart = 0;
+	let pinEnd = 0;
+	stickyElements.forEach(sticky => {
+		if (sticky === element || sticky.contains(element)) {
+			return;
 		}
 
-		return { x, y };
-	}, { x: elementRect[inline], y: elementRect.top });
+		const stickyStyle = getComputedStyle(sticky);
+		const stickyRect = sticky.getBoundingClientRect();
+		if (stickyStyle[start] !== "auto") {
+			pinStart = Math.max(pinStart, stickyRect[end] - scrollContainerRect[start]);
+		}
+		if (stickyStyle[end] !== "auto") {
+			pinEnd = Math.max(pinEnd, scrollContainerRect[end] - stickyRect[start]);
+		}
+	});
 
-	const scrollX = elementRect[inline] - stickyX;
-	const scrollY = elementRect.top - stickyY;
+	const viewportStart = scrollContainerRect[start] + pinStart;
+	const viewportEnd = scrollContainerRect[end] - pinEnd;
 
-	if (scrollX === 0 && scrollY === 0) {
-		// avoid unnecessary scroll call, when nothing changes
-		return;
+	// Element already spans the whole container
+	if (elementRect[start] <= scrollContainerRect[start] && elementRect[end] >= scrollContainerRect[end]) {
+		return 0;
 	}
-
-	scrollContainer.scrollBy({
-		top: scrollY,
-		left: scrollX,
-	});
+	// Element larger than the visible viewport
+	if (elementRect[size] > viewportEnd - viewportStart) {
+		return (axis === "x" && element.matches(":dir(rtl)")) ? elementRect[end] - viewportEnd : elementRect[start] - viewportStart;
+	}
+	if (elementRect[start] < viewportStart) {
+		return elementRect[start] - viewportStart;
+	}
+	if (elementRect[end] > viewportEnd) {
+		return elementRect[end] - viewportEnd;
+	}
+	return 0;
 };
 
 const isFeature = <T>(element: any, identifier: string): element is T => {
@@ -116,7 +132,7 @@ export {
 	isHeaderSelectionCell,
 	findRowInPath,
 	findVerticalScrollContainer,
-	scrollElementIntoView,
+	computeAxisScrollDelta,
 	isFeature,
 	throttle,
 	toggleAttribute,

packages/main/test/pages/Table.html

@@ -31,7 +31,7 @@
 		</ui5-segmented-button>
 	</ui5-bar>
 
-	<ui5-table id="table" overflow-mode="Popin" accessible-name-ref="title" style="height: 400px" row-action-count="3" alternate-row-colors>
+	<ui5-table id="table" overflow-mode="Scroll" accessible-name-ref="title" style="height: 400px" row-action-count="3" alternate-row-colors>
 		<ui5-table-growing id="growing" mode="Scroll" slot="features"></ui5-table-growing>
 		<ui5-table-selection-multi id="selection" selected="1 3" slot="features"></ui5-table-selection-multi>
 		<!-- <ui5-table-selection-single id="selection" selected="1" slot="features"></ui5-table-selection-single> -->