From 148619ead6d8944cb6120b1e07b4bdc21ffc69d9 Mon Sep 17 00:00:00 2001 From: Duc Vo Ngoc Date: Thu, 10 Jul 2025 14:25:07 +0200 Subject: [PATCH 1/3] fix(ui5-table): register capture individually for selection --- packages/main/src/Table.ts | 2 +- packages/main/src/TableSelection.ts | 34 ++++++++++++++++++---- packages/main/src/TableSelectionMulti.ts | 37 ++++++++++++++++++++---- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/packages/main/src/Table.ts b/packages/main/src/Table.ts index fad14b99514f..547189a037fb 100644 --- a/packages/main/src/Table.ts +++ b/packages/main/src/Table.ts @@ -419,7 +419,7 @@ class Table extends UI5Element { } onEnterDOM() { - this._events.forEach(eventType => this.addEventListener(eventType, this._onEventBound, { capture: true })); + this._events.forEach(eventType => this.addEventListener(eventType, this._onEventBound)); this.features.forEach(feature => feature.onTableActivate?.(this)); this._tableNavigation = new TableNavigation(this); this._tableDragAndDrop = new TableDragAndDrop(this); diff --git a/packages/main/src/TableSelection.ts b/packages/main/src/TableSelection.ts index 756bbf9c3425..5c94772cc5d3 100644 --- a/packages/main/src/TableSelection.ts +++ b/packages/main/src/TableSelection.ts @@ -88,6 +88,17 @@ class TableSelection extends UI5Element implements ITableFeature { _rowsLength = 0; _rangeSelection?: {selected: boolean, isUp: boolean | null, rows: TableRow[], isMouse: boolean, shiftPressed: boolean} | null; + onKeyUpCaptureBound: (e: KeyboardEvent) => void; + onKeyDownCaptureBound: (e: KeyboardEvent) => void; + onClickCaptureBound: (e: MouseEvent) => void; + + constructor() { + super(); + this.onKeyUpCaptureBound = this._onKeyupCapture.bind(this); + this.onKeyDownCaptureBound = this._onKeydownCapture.bind(this); + this.onClickCaptureBound = this._onClickCapture.bind(this); + } + onTableActivate(table: Table) { this._table = table; this._invalidateTableAndRows(); @@ -101,6 +112,10 @@ class TableSelection extends UI5Element implements ITableFeature { onBeforeRendering() { this._invalidateTableAndRows(); + + this._table?.removeEventListener("keydown", this.onKeyDownCaptureBound); + this._table?.removeEventListener("keyup", this.onKeyUpCaptureBound); + this._table?.removeEventListener("click", this.onClickCaptureBound); } onTableBeforeRendering() { @@ -108,6 +123,10 @@ class TableSelection extends UI5Element implements ITableFeature { this._rowsLength = this._table.rows.length; this._table.headerRow[0]._invalidate++; } + + this._table?.addEventListener("keydown", this.onKeyDownCaptureBound, { capture: true }); + this._table?.addEventListener("keyup", this.onKeyUpCaptureBound, { capture: true }); + this._table?.addEventListener("click", this.onClickCaptureBound, { capture: true }); } isSelectable(): boolean { @@ -231,7 +250,7 @@ class TableSelection extends UI5Element implements ITableFeature { this._table.rows.forEach(row => row._invalidate++); } - _onkeydown(e: KeyboardEvent) { + _onKeydownCapture(e: KeyboardEvent) { if (!this.isMultiSelectable() || !this._table || !e.shiftKey) { return; } @@ -248,7 +267,7 @@ class TableSelection extends UI5Element implements ITableFeature { const row = focusedElement as TableRow; this._startRangeSelection(row, this.isSelected(row)); } else if (e.key === "ArrowUp" || e.key === "ArrowDown") { - const change = isUpShift(e) ? -1 : 1; + const change = isUpShift(e) ? 1 : -1; this._handleRangeSelection(focusedElement as TableRow, change); } @@ -257,11 +276,12 @@ class TableSelection extends UI5Element implements ITableFeature { } } - _onkeyup(e: KeyboardEvent, eventOrigin: HTMLElement) { + _onKeyupCapture(e: KeyboardEvent) { if (!this._table) { return; } + const eventOrigin = e.composedPath()[0] as HTMLElement; if (!eventOrigin.hasAttribute("ui5-table-row") || !this._rangeSelection || !e.shiftKey) { // Stop range selection if a) Shift is relased or b) the event target is not a row this._stopRangeSelection(); @@ -272,7 +292,7 @@ class TableSelection extends UI5Element implements ITableFeature { } } - _onclick(e: MouseEvent) { + _onClickCapture(e: MouseEvent) { if (!this._table || this.mode !== TableSelectionMode.Multiple) { return; } @@ -294,11 +314,13 @@ class TableSelection extends UI5Element implements ITableFeature { const startIndex = this._table.rows.indexOf(startRow); const endIndex = this._table.rows.indexOf(row); + const selectionState = this.isSelected(startRow); + // When doing a range selection and clicking on an already selected row, the checked status should not change // Therefore, we need to manually set the checked attribute again, as clicking it would deselect it and leads to // a visual inconsistency. - row.shadowRoot?.querySelector("#selection-component")?.toggleAttribute("checked", true); - e.stopImmediatePropagation(); + row.shadowRoot?.querySelector("#selection-component")?.toggleAttribute("checked", selectionState); + e.stopPropagation(); if (startIndex === -1 || endIndex === -1 || row.rowKey === startRow.rowKey || row.rowKey === this._rangeSelection.rows[this._rangeSelection.rows.length - 1].rowKey) { return; diff --git a/packages/main/src/TableSelectionMulti.ts b/packages/main/src/TableSelectionMulti.ts index 58c6c2f6baeb..06ed0f619e06 100644 --- a/packages/main/src/TableSelectionMulti.ts +++ b/packages/main/src/TableSelectionMulti.ts @@ -67,11 +67,32 @@ class TableSelectionMulti extends TableSelectionBase { shiftPressed: boolean } | null; + _onKeyDownCaptureBound: (e: KeyboardEvent) => void; + _onClickCaptureBound: (e: MouseEvent) => void; + _onKeyUpCaptureBound: (e: KeyboardEvent) => void; + + constructor() { + super(); + this._onKeyDownCaptureBound = this._onkeydownCapture.bind(this); + this._onClickCaptureBound = this._onclickCapture.bind(this); + this._onKeyUpCaptureBound = this._onkeyupCapture.bind(this); + } + onTableBeforeRendering() { if (this._table && this._table.headerRow[0] && this._rowsLength !== this._table.rows.length) { this._rowsLength = this._table.rows.length; this._table.headerRow[0]._invalidate++; } + + this._table?.removeEventListener("keydown", this._onKeyDownCaptureBound); + this._table?.removeEventListener("keyup", this._onKeyUpCaptureBound); + this._table?.removeEventListener("click", this._onClickCaptureBound); + } + + onTableAfterRendering() { + this._table?.addEventListener("keydown", this._onKeyDownCaptureBound, { capture: true }); + this._table?.addEventListener("keyup", this._onKeyUpCaptureBound, { capture: true }); + this._table?.addEventListener("click", this._onClickCaptureBound, { capture: true }); } isMultiSelectable(): boolean { @@ -156,7 +177,7 @@ class TableSelectionMulti extends TableSelectionBase { this.selected = [...selectedSet].join(" "); } - _onkeydown(e: KeyboardEvent) { + _onkeydownCapture(e: KeyboardEvent) { if (!this._table || !e.shiftKey) { return; } @@ -173,7 +194,7 @@ class TableSelectionMulti extends TableSelectionBase { const row = focusedElement as TableRow; this._startRangeSelection(row, this.isSelected(row)); } else if (e.key === "ArrowUp" || e.key === "ArrowDown") { - const change = isUpShift(e) ? -1 : 1; + const change = isUpShift(e) ? 1 : -1; this._handleRangeSelection(focusedElement as TableRow, change); } @@ -182,11 +203,12 @@ class TableSelectionMulti extends TableSelectionBase { } } - _onkeyup(e: KeyboardEvent, eventOrigin: HTMLElement) { + _onkeyupCapture(e: KeyboardEvent) { if (!this._table) { return; } + const eventOrigin = e.composedPath()[0] as HTMLElement; if (!eventOrigin.hasAttribute("ui5-table-row") || !this._rangeSelection || !e.shiftKey) { // Stop range selection if a) Shift is relased or b) the event target is not a row this._stopRangeSelection(); @@ -197,7 +219,7 @@ class TableSelectionMulti extends TableSelectionBase { } } - _onclick(e: MouseEvent) { + _onclickCapture(e: MouseEvent) { if (!this._table) { return; } @@ -219,11 +241,14 @@ class TableSelectionMulti extends TableSelectionBase { const startIndex = this._table.rows.indexOf(startRow); const endIndex = this._table.rows.indexOf(row); + // Set checkbox to the selection state of the start row (if it is selected) + const selectionState = this.isSelected(startRow); + // When doing a range selection and clicking on an already selected row, the checked status should not change // Therefore, we need to manually set the checked attribute again, as clicking it would deselect it and leads to // a visual inconsistency. - row.shadowRoot?.querySelector("#selection-component")?.toggleAttribute("checked", true); - e.stopImmediatePropagation(); + row.shadowRoot?.querySelector("#selection-component")?.toggleAttribute("checked", selectionState); + e.stopPropagation(); if (startIndex === -1 || endIndex === -1 || row.rowKey === startRow.rowKey || row.rowKey === this._rangeSelection.rows[this._rangeSelection.rows.length - 1].rowKey) { return; From f5a4ac6adc78ec9173952c24ddce3063a6a52ef5 Mon Sep 17 00:00:00 2001 From: Duc Vo Ngoc Date: Thu, 10 Jul 2025 14:46:29 +0200 Subject: [PATCH 2/3] fix: remove captures for keyup/down --- packages/main/src/TableSelection.ts | 15 +++------------ packages/main/src/TableSelectionMulti.ts | 15 +++------------ 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/packages/main/src/TableSelection.ts b/packages/main/src/TableSelection.ts index 5c94772cc5d3..25c47ffa1158 100644 --- a/packages/main/src/TableSelection.ts +++ b/packages/main/src/TableSelection.ts @@ -88,14 +88,10 @@ class TableSelection extends UI5Element implements ITableFeature { _rowsLength = 0; _rangeSelection?: {selected: boolean, isUp: boolean | null, rows: TableRow[], isMouse: boolean, shiftPressed: boolean} | null; - onKeyUpCaptureBound: (e: KeyboardEvent) => void; - onKeyDownCaptureBound: (e: KeyboardEvent) => void; onClickCaptureBound: (e: MouseEvent) => void; constructor() { super(); - this.onKeyUpCaptureBound = this._onKeyupCapture.bind(this); - this.onKeyDownCaptureBound = this._onKeydownCapture.bind(this); this.onClickCaptureBound = this._onClickCapture.bind(this); } @@ -113,8 +109,6 @@ class TableSelection extends UI5Element implements ITableFeature { onBeforeRendering() { this._invalidateTableAndRows(); - this._table?.removeEventListener("keydown", this.onKeyDownCaptureBound); - this._table?.removeEventListener("keyup", this.onKeyUpCaptureBound); this._table?.removeEventListener("click", this.onClickCaptureBound); } @@ -124,8 +118,6 @@ class TableSelection extends UI5Element implements ITableFeature { this._table.headerRow[0]._invalidate++; } - this._table?.addEventListener("keydown", this.onKeyDownCaptureBound, { capture: true }); - this._table?.addEventListener("keyup", this.onKeyUpCaptureBound, { capture: true }); this._table?.addEventListener("click", this.onClickCaptureBound, { capture: true }); } @@ -250,7 +242,7 @@ class TableSelection extends UI5Element implements ITableFeature { this._table.rows.forEach(row => row._invalidate++); } - _onKeydownCapture(e: KeyboardEvent) { + _onkeydown(e: KeyboardEvent) { if (!this.isMultiSelectable() || !this._table || !e.shiftKey) { return; } @@ -267,7 +259,7 @@ class TableSelection extends UI5Element implements ITableFeature { const row = focusedElement as TableRow; this._startRangeSelection(row, this.isSelected(row)); } else if (e.key === "ArrowUp" || e.key === "ArrowDown") { - const change = isUpShift(e) ? 1 : -1; + const change = isUpShift(e) ? -1 : 1; this._handleRangeSelection(focusedElement as TableRow, change); } @@ -276,12 +268,11 @@ class TableSelection extends UI5Element implements ITableFeature { } } - _onKeyupCapture(e: KeyboardEvent) { + _onkeyup(e: KeyboardEvent, eventOrigin: HTMLElement) { if (!this._table) { return; } - const eventOrigin = e.composedPath()[0] as HTMLElement; if (!eventOrigin.hasAttribute("ui5-table-row") || !this._rangeSelection || !e.shiftKey) { // Stop range selection if a) Shift is relased or b) the event target is not a row this._stopRangeSelection(); diff --git a/packages/main/src/TableSelectionMulti.ts b/packages/main/src/TableSelectionMulti.ts index 06ed0f619e06..8bb86b0d2582 100644 --- a/packages/main/src/TableSelectionMulti.ts +++ b/packages/main/src/TableSelectionMulti.ts @@ -67,15 +67,11 @@ class TableSelectionMulti extends TableSelectionBase { shiftPressed: boolean } | null; - _onKeyDownCaptureBound: (e: KeyboardEvent) => void; _onClickCaptureBound: (e: MouseEvent) => void; - _onKeyUpCaptureBound: (e: KeyboardEvent) => void; constructor() { super(); - this._onKeyDownCaptureBound = this._onkeydownCapture.bind(this); this._onClickCaptureBound = this._onclickCapture.bind(this); - this._onKeyUpCaptureBound = this._onkeyupCapture.bind(this); } onTableBeforeRendering() { @@ -84,14 +80,10 @@ class TableSelectionMulti extends TableSelectionBase { this._table.headerRow[0]._invalidate++; } - this._table?.removeEventListener("keydown", this._onKeyDownCaptureBound); - this._table?.removeEventListener("keyup", this._onKeyUpCaptureBound); this._table?.removeEventListener("click", this._onClickCaptureBound); } onTableAfterRendering() { - this._table?.addEventListener("keydown", this._onKeyDownCaptureBound, { capture: true }); - this._table?.addEventListener("keyup", this._onKeyUpCaptureBound, { capture: true }); this._table?.addEventListener("click", this._onClickCaptureBound, { capture: true }); } @@ -177,7 +169,7 @@ class TableSelectionMulti extends TableSelectionBase { this.selected = [...selectedSet].join(" "); } - _onkeydownCapture(e: KeyboardEvent) { + _onkeydown(e: KeyboardEvent) { if (!this._table || !e.shiftKey) { return; } @@ -194,7 +186,7 @@ class TableSelectionMulti extends TableSelectionBase { const row = focusedElement as TableRow; this._startRangeSelection(row, this.isSelected(row)); } else if (e.key === "ArrowUp" || e.key === "ArrowDown") { - const change = isUpShift(e) ? 1 : -1; + const change = isUpShift(e) ? -1 : 1; this._handleRangeSelection(focusedElement as TableRow, change); } @@ -203,12 +195,11 @@ class TableSelectionMulti extends TableSelectionBase { } } - _onkeyupCapture(e: KeyboardEvent) { + _onkeyup(e: KeyboardEvent, eventOrigin: HTMLElement) { if (!this._table) { return; } - const eventOrigin = e.composedPath()[0] as HTMLElement; if (!eventOrigin.hasAttribute("ui5-table-row") || !this._rangeSelection || !e.shiftKey) { // Stop range selection if a) Shift is relased or b) the event target is not a row this._stopRangeSelection(); From 8bd11ea993fb26eaa4f21878ec24df7c5bf9670e Mon Sep 17 00:00:00 2001 From: Duc Vo Ngoc Date: Thu, 10 Jul 2025 14:56:26 +0200 Subject: [PATCH 3/3] chore: adjust event registration --- packages/main/src/TableSelection.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/main/src/TableSelection.ts b/packages/main/src/TableSelection.ts index 25c47ffa1158..f3edd1104364 100644 --- a/packages/main/src/TableSelection.ts +++ b/packages/main/src/TableSelection.ts @@ -108,8 +108,6 @@ class TableSelection extends UI5Element implements ITableFeature { onBeforeRendering() { this._invalidateTableAndRows(); - - this._table?.removeEventListener("click", this.onClickCaptureBound); } onTableBeforeRendering() { @@ -118,6 +116,10 @@ class TableSelection extends UI5Element implements ITableFeature { this._table.headerRow[0]._invalidate++; } + this._table?.removeEventListener("click", this.onClickCaptureBound); + } + + onTableAfterRendering(): void { this._table?.addEventListener("click", this.onClickCaptureBound, { capture: true }); }