Skip to content

Commit 7390bd2

Browse files
aborjinikCopilot
aborjinik
and
Copilot
authored
refactor(ui5-table): implement empty area rendering with custom focus outline (#13462)
* refactor(table): implement dummy cell rendering Co-authored-by: Copilot <copilot@github.com> * refactor(table): implements empty area rendering --------- Co-authored-by: Copilot <copilot@github.com>
1 parent 2286f10 commit 7390bd2

14 files changed

Lines changed: 592 additions & 104 deletions

File tree

‎packages/main/cypress/specs/Table.cy.tsx‎

Lines changed: 191 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import TableHeaderCell from "../../src/TableHeaderCell.js";
88
import TableHeaderCellActionAI from "../../src/TableHeaderCellActionAI.js";
99
import Label from "../../src/Label.js";
1010
import Input from "../../src/Input.js";
11+
import TableRowAction from "../../src/TableRowAction.js";
1112
import Bar from "../../src/Bar.js";
1213
import Title from "../../src/Title.js";
1314
import Slider from "../../src/Slider.js";
@@ -429,8 +430,6 @@ describe("Table - Popin Mode", () => {
429430
cy.get("ui5-table").then($table => {
430431
$table.css("width", "150px");
431432
});
432-
433-
// eslint-disable-next-line cypress/no-unnecessary-waiting
434433
cy.wait(50);
435434

436435
cy.get("ui5-table").then($table => {
@@ -443,7 +442,7 @@ describe("Table - Popin Mode", () => {
443442
for (const cell of row.cells) {
444443
if (cell._popin) {
445444
popinCellCount++;
446-
const popinText = cell._headerCell.popinText || cell._headerCell.textContent;
445+
const popinText = cell._headerCell!.popinText || cell._headerCell!.textContent;
447446
if (cell.shadowRoot!.textContent === `${popinText}:`) {
448447
validPopinTextCount++;
449448
}
@@ -883,26 +882,24 @@ describe("Table - Navigated Rows", () => {
883882
</Table>
884883
);
885884

886-
cy.get("#row1")
887-
.shadow()
888-
.find("#navigated-cell")
885+
// Navigated cell rendered on all rows and header row
886+
cy.get("[ui5-table-header-row]").shadow().find("#navigated-cell")
889887
.should("exist")
888+
.should("have.attr", "aria-hidden", "true");
889+
cy.get("[ui5-table-header-row]").shadow().find("#navigated-cell")
890890
.should("have.attr", "data-excluded-from-navigation");
891891

892-
cy.get("#row2")
893-
.shadow()
894-
.find("#navigated-cell")
892+
cy.get("#row1").shadow().find("#navigated-cell")
895893
.should("exist")
896894
.should("have.attr", "data-excluded-from-navigation");
897895

898-
cy.get("#row1")
899-
.shadow()
900-
.find("#navigated")
901-
.as("navigated1");
896+
cy.get("#row2").shadow().find("#navigated-cell")
897+
.should("exist")
898+
.should("have.attr", "data-excluded-from-navigation");
902899

903-
cy.get("#row2")
904-
.shadow()
905-
.find("#navigated")
900+
// Navigated indicator differs between navigated and non-navigated rows
901+
cy.get("#row1").shadow().find("#navigated").as("navigated1");
902+
cy.get("#row2").shadow().find("#navigated")
906903
.then($navigated2 => {
907904
cy.get("@navigated1")
908905
.should($navigated1 => {
@@ -1146,7 +1143,7 @@ describe("Table - Cell Merging", () => {
11461143
cy.wait(50);
11471144

11481145
// Merged cell border should fall back to normal border color (not transparent)
1149-
cy.get("#row2").should("have.attr", "_haspopin");
1146+
cy.get("#row2").should("have.attr", "_has-popin");
11501147
cy.get("#r2cA").should("not.have.css", "border-top-color", TRANSPARENT);
11511148
cy.get("#row2").shadow().find("#selection-cell").should("not.have.css", "border-top-color", TRANSPARENT);
11521149

@@ -1193,3 +1190,180 @@ describe("Table - Cell Merging", () => {
11931190
cy.get("#row2").shadow().find("#selection-cell").should("have.css", "border-top-color", TRANSPARENT);
11941191
});
11951192
});
1193+
1194+
describe("Table - Dummy Cell", () => {
1195+
it("should render dummy cell with border and nofocus when all columns have fixed widths", () => {
1196+
cy.mount(
1197+
<Table id="table">
1198+
<TableHeaderRow slot="headerRow">
1199+
<TableHeaderCell width="200px">Product</TableHeaderCell>
1200+
<TableHeaderCell width="40%">Supplier</TableHeaderCell>
1201+
</TableHeaderRow>
1202+
<TableRow id="row1">
1203+
<TableCell><Label>Product 1</Label></TableCell>
1204+
<TableCell><Label>Supplier 1</Label></TableCell>
1205+
</TableRow>
1206+
<TableRow id="row2" interactive>
1207+
<TableCell><Label>Product 2</Label></TableCell>
1208+
<TableCell><Label>Supplier 2</Label></TableCell>
1209+
</TableRow>
1210+
</Table>
1211+
);
1212+
1213+
cy.get("[ui5-table-header-row]").shadow().find("#dummy-cell").should("exist");
1214+
cy.get("#row1").shadow().find("#dummy-cell").should("exist");
1215+
1216+
// Should have left border
1217+
cy.get("[ui5-table-header-row]").shadow().find("#dummy-cell")
1218+
.should("not.have.css", "border-inline-start-width", "0px");
1219+
cy.get("#row1").shadow().find("#dummy-cell")
1220+
.should("not.have.css", "border-inline-start-width", "0px");
1221+
1222+
// Should have data-excluded-from-navigation="nofocus" and data-border-merged when no popin
1223+
cy.get("#row1").shadow().find("#dummy-cell")
1224+
.should("have.attr", "data-excluded-from-navigation", "nofocus")
1225+
.should("have.attr", "data-border-merged");
1226+
1227+
// Should not focus row or fire row-click when clicking dummy cell
1228+
cy.get("#table").invoke("on", "row-click", cy.stub().as("rowClickHandler"));
1229+
cy.get("#row2").shadow().find("#dummy-cell").realClick();
1230+
cy.get("#row2").should("not.be.focused");
1231+
cy.get("#row2").should("not.have.attr", "_active");
1232+
cy.get("@rowClickHandler").should("not.have.been.called");
1233+
});
1234+
1235+
it("should not render dummy cell when columns are flexible", () => {
1236+
const cases = [
1237+
{ widths: ["200px", "auto"] },
1238+
{ widths: ["Auto"] },
1239+
{ widths: ["200px", undefined] },
1240+
];
1241+
1242+
cases.forEach(({ widths }) => {
1243+
cy.mount(
1244+
<Table id="table">
1245+
<TableHeaderRow slot="headerRow">
1246+
{widths.map((w, i) => (
1247+
<TableHeaderCell width={w}>{`Col ${i}`}</TableHeaderCell>
1248+
))}
1249+
</TableHeaderRow>
1250+
<TableRow id="row1">
1251+
{widths.map((_, i) => (
1252+
<TableCell><Label>{`Cell ${i}`}</Label></TableCell>
1253+
))}
1254+
</TableRow>
1255+
</Table>
1256+
);
1257+
1258+
cy.get("[ui5-table-header-row]").shadow().find("#dummy-cell").should("not.exist");
1259+
cy.get("#row1").shadow().find("#dummy-cell").should("not.exist");
1260+
});
1261+
});
1262+
1263+
it("should render dummy cell after actions when no popin and before actions when popin", () => {
1264+
cy.mount(
1265+
<Table id="table" rowActionCount={2} overflowMode="Popin">
1266+
<TableHeaderRow slot="headerRow">
1267+
<TableHeaderCell width="200px" minWidth="200px">Product</TableHeaderCell>
1268+
<TableHeaderCell width="200px" minWidth="200px">Supplier</TableHeaderCell>
1269+
</TableHeaderRow>
1270+
<TableRow id="row1">
1271+
<TableCell><Label>Product 1</Label></TableCell>
1272+
<TableCell><Label>Supplier 1</Label></TableCell>
1273+
<TableRowAction slot="actions" icon="edit" text="Edit"></TableRowAction>
1274+
<TableRowAction slot="actions" icon="delete" text="Delete"></TableRowAction>
1275+
</TableRow>
1276+
</Table>
1277+
);
1278+
1279+
// No popin: dummy cell is after actions cell (rightmost) in both row and header row
1280+
cy.get("#row1").shadow().then($shadow => {
1281+
const dummyCell = $shadow.find("#dummy-cell")[0];
1282+
const actionsCell = $shadow.find("#actions-cell")[0];
1283+
const position = dummyCell.compareDocumentPosition(actionsCell);
1284+
expect(position & Node.DOCUMENT_POSITION_PRECEDING).to.be.greaterThan(0);
1285+
});
1286+
cy.get("[ui5-table-header-row]").shadow().then($shadow => {
1287+
const dummyCell = $shadow.find("#dummy-cell")[0];
1288+
const actionsCell = $shadow.find("#actions-cell")[0];
1289+
const position = dummyCell.compareDocumentPosition(actionsCell);
1290+
expect(position & Node.DOCUMENT_POSITION_PRECEDING).to.be.greaterThan(0);
1291+
});
1292+
1293+
// Shrink to trigger popin: dummy cell moves before actions cell
1294+
cy.get("ui5-table").invoke("css", "width", "250px");
1295+
cy.wait(50);
1296+
1297+
cy.get("#row1").should("have.attr", "_has-popin");
1298+
cy.get("#row1").shadow().then($shadow => {
1299+
const dummyCell = $shadow.find("#dummy-cell")[0];
1300+
const actionsCell = $shadow.find("#actions-cell")[0];
1301+
const position = dummyCell.compareDocumentPosition(actionsCell);
1302+
expect(position & Node.DOCUMENT_POSITION_FOLLOWING).to.be.greaterThan(0);
1303+
});
1304+
cy.get("[ui5-table-header-row]").shadow().then($shadow => {
1305+
const dummyCell = $shadow.find("#dummy-cell")[0];
1306+
const actionsCell = $shadow.find("#actions-cell")[0];
1307+
const position = dummyCell.compareDocumentPosition(actionsCell);
1308+
expect(position & Node.DOCUMENT_POSITION_FOLLOWING).to.be.greaterThan(0);
1309+
});
1310+
});
1311+
1312+
it("should adapt dummy cell border, navigation attribute, and custom focus outline with popin", () => {
1313+
cy.mount(
1314+
<Table id="table" overflowMode="Popin">
1315+
<TableSelectionMulti slot="features" />
1316+
<TableHeaderRow slot="headerRow">
1317+
<TableHeaderCell width="200px" minWidth="200px">Product</TableHeaderCell>
1318+
<TableHeaderCell width="200px" minWidth="200px">Supplier</TableHeaderCell>
1319+
</TableHeaderRow>
1320+
<TableRow id="row1">
1321+
<TableCell><Label>Product 1</Label></TableCell>
1322+
<TableCell><Label>Supplier 1</Label></TableCell>
1323+
</TableRow>
1324+
</Table>
1325+
);
1326+
1327+
// At full width: left border visible, nofocus attribute on both row and header row
1328+
cy.get("#row1").shadow().find("#dummy-cell")
1329+
.should("not.have.css", "border-inline-start-width", "0px")
1330+
.should("have.attr", "data-excluded-from-navigation", "nofocus");
1331+
cy.get("[ui5-table-header-row]").shadow().find("#dummy-cell")
1332+
.should("have.attr", "data-excluded-from-navigation", "nofocus");
1333+
1334+
// Focus row - custom outline should be applied
1335+
cy.get("#row1").should("have.attr", "_render-dummy-cell");
1336+
cy.get("#row1").should("not.have.attr", "_has-popin");
1337+
cy.realPress("Tab");
1338+
cy.get("#row1").shadow().find("[data-ui5-custom-outline='start']").should("exist");
1339+
cy.get("#row1").find("[data-ui5-custom-outline='end']").should("exist");
1340+
cy.get("#row1").shadow().find("#dummy-cell")
1341+
.should("not.have.attr", "data-ui5-custom-outline");
1342+
1343+
// Shrink to trigger popin to test custom outline is removed
1344+
cy.get("ui5-table").invoke("css", "width", "250px");
1345+
cy.wait(50);
1346+
1347+
cy.get("#row1").should("have.attr", "_has-popin");
1348+
1349+
// Left border removed, data-excluded-from-navigation without nofocus
1350+
cy.get("#row1").shadow().find("#dummy-cell")
1351+
.should("have.css", "border-inline-start-width", "0px")
1352+
.should("have.attr", "data-excluded-from-navigation", "");
1353+
cy.get("[ui5-table-header-row]").shadow().find("#dummy-cell")
1354+
.should("have.attr", "data-excluded-from-navigation", "");
1355+
1356+
// Expand back - border and nofocus should return, custom outline reapplied via onAfterRendering
1357+
cy.get("ui5-table").invoke("css", "width", "800px");
1358+
cy.wait(50);
1359+
1360+
cy.get("#row1").should("not.have.attr", "_has-popin");
1361+
cy.get("#row1").shadow().find("#dummy-cell")
1362+
.should("not.have.css", "border-inline-start-width", "0px")
1363+
.should("have.attr", "data-excluded-from-navigation", "nofocus");
1364+
cy.get("[ui5-table-header-row]").shadow().find("#dummy-cell")
1365+
.should("have.attr", "data-excluded-from-navigation", "nofocus");
1366+
cy.get("#row1").shadow().find("[data-ui5-custom-outline='start']").should("exist");
1367+
cy.get("#row1").find("[data-ui5-custom-outline='end']").should("exist");
1368+
});
1369+
});

‎packages/main/src/Table.ts‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,9 @@ class Table extends UI5Element {
457457
onBeforeRendering(): void {
458458
this._renderNavigated = this.rows.some(row => row.navigated);
459459
[...this.headerRow, ...this.rows].forEach((row, index) => {
460+
row._rowActionCount = this.rows.length > 0 ? this.rowActionCount : 0;
460461
row._renderNavigated = this._renderNavigated;
461-
row._rowActionCount = this.rowActionCount;
462+
row._renderDummyCell = !this._hasFlexibleColumns;
462463
row._alternate = this.alternateRowColors && index % 2 === 0;
463464
});
464465

@@ -649,8 +650,16 @@ class Table extends UI5Element {
649650
return width;
650651
}));
651652

653+
// Dummy Cell Width (before actions when popin, after navigated otherwise)
654+
const dummyColumnWidth = !this._hasFlexibleColumns ? "minmax(0, 1fr)" : "";
655+
const hasPopinCells = this.headerRow[0]._popinCells.length > 0;
656+
657+
if (dummyColumnWidth && hasPopinCells) {
658+
widths.push(dummyColumnWidth);
659+
}
660+
652661
// Row Action Cell Width
653-
if (this.rowActionCount > 0) {
662+
if (this.rowActionCount > 0 && this.rows.length > 0) {
654663
widths.push(`calc(var(--_ui5_button_base_min_width) * ${this.rowActionCount} + var(--_ui5_table_row_actions_gap) * ${this.rowActionCount - 1} + var(--_ui5_table_cell_horizontal_padding) * 2)`);
655664
}
656665

@@ -659,9 +668,17 @@ class Table extends UI5Element {
659668
widths.push(`var(--_ui5_table_navigated_cell_width)`);
660669
}
661670

671+
if (dummyColumnWidth && !hasPopinCells) {
672+
widths.push(dummyColumnWidth);
673+
}
674+
662675
return widths.join(" ");
663676
}
664677

678+
get _hasFlexibleColumns(): boolean {
679+
return this.headerRow?.[0]?._visibleCells.some(cell => !isValidColumnWidth(cell.width));
680+
}
681+
665682
get _isRowSelectorRequired() {
666683
return this.rows.length > 0 && this._getSelection()?.isRowSelectorRequired();
667684
}
@@ -701,7 +718,7 @@ class Table extends UI5Element {
701718
if (this._isRowSelectorRequired) {
702719
ariaColCount++;
703720
}
704-
if (this.rowActionCount > 0) {
721+
if (this.rowActionCount > 0 && this.rows.length > 0) {
705722
ariaColCount++;
706723
}
707724
if (this.headerRow[0]._popinCells.length > 0) {

‎packages/main/src/TableCell.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ class TableCell extends TableCellBase {
3434
/**
3535
* Defines whether the cell is visually merged with the cell directly above it.
3636
*
37-
* This is useful when consecutive cells in a column have the same value and should visually appear as a single merged cell.
37+
* This is useful if consecutive cells in a column have the same value and should visually appear as a single merged cell.
3838
* Although the cell is visually merged with the previous one, its content must still be provided for accessibility purposes.
39-
* **Note:** This feature is disabled when cells are rendered as popin, and should remain `false` for interactive cell content.
39+
* **Note:** This feature is disabled when cells are rendered as a popin, and should remain `false` for interactive cell content.
4040
*
4141
* @default false
4242
* @since 2.21.0

‎packages/main/src/TableHeaderRowTemplate.tsx‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,35 @@ export default function TableHeaderRowTemplate(this: TableHeaderRow, ariaColInde
5454
return [<slot name={cell._individualSlot}></slot>];
5555
})}
5656

57+
{ this._renderDummyCell && this._hasPopin &&
58+
<TableHeaderCell id="dummy-cell" role="none" aria-hidden={true}
59+
data-excluded-from-navigation="">
60+
</TableHeaderCell>
61+
}
62+
5763
{ this._rowActionCount > 0 &&
5864
<TableHeaderCell id="actions-cell" aria-colindex={ariaColIndex++}>
5965
<div id="actions-cell-content">{this._i18nRowActions}</div>
6066
</TableHeaderCell>
6167
}
6268

63-
{ this._popinCells.length > 0 &&
69+
{ this._renderNavigated &&
70+
<TableHeaderCell id="navigated-cell"
71+
data-excluded-from-navigation
72+
aria-hidden={true}
73+
role="none"
74+
>
75+
<div id="navigated"></div>
76+
</TableHeaderCell>
77+
}
78+
79+
{ this._renderDummyCell && !this._hasPopin &&
80+
<TableHeaderCell id="dummy-cell" role="none" aria-hidden={true}
81+
data-excluded-from-navigation="nofocus">
82+
</TableHeaderCell>
83+
}
84+
85+
{ this._hasPopin &&
6486
<TableHeaderCell id="popin-cell" aria-colindex={ariaColIndex++} data-excluded-from-navigation>
6587
<div id="popin-cell-content">{this._i18nRowPopin}</div>
6688
</TableHeaderCell>

‎packages/main/src/TableNavigation.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,9 @@ class TableNavigation extends TableExtension {
246246
for (const target of e.composedPath() as any[]) {
247247
if (target.nodeType === Node.ELEMENT_NODE) {
248248
const element = target as HTMLElement;
249+
if (element.getAttribute("data-excluded-from-navigation") === "nofocus") {
250+
break;
251+
}
249252
if (element.matches(":focus-within")) {
250253
focusableElement = element;
251254
break;

‎packages/main/src/TableRow.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,6 @@ class TableRow extends TableRowBase<TableCell> {
130130
toggleAttribute(this, "draggable", this.movable, "true");
131131
toggleAttribute(this, "_interactive", this._isInteractive);
132132
toggleAttribute(this, "_alternate", this._alternate);
133-
toggleAttribute(this, "_haspopin", this._hasPopin);
134-
}
135-
136-
async focus(focusOptions?: FocusOptions | undefined): Promise<void> {
137-
this.setAttribute("tabindex", "-1");
138-
HTMLElement.prototype.focus.call(this, focusOptions);
139-
return Promise.resolve();
140133
}
141134

142135
async _onpointerdown(e: PointerEvent) {
@@ -198,10 +191,6 @@ class TableRow extends TableRowBase<TableCell> {
198191
}) !== undefined;
199192
}
200193

201-
get _hasPopin() {
202-
return this.cells.some(c => c._popin && !c._popinHidden);
203-
}
204-
205194
get _rowIndex() {
206195
if (this.position !== undefined) {
207196
return this.position;

0 commit comments

Comments
 (0)