Skip to content

Commit 825e285

Browse files
aborjinikaborjinik
committed
fix(ui5-table): improve horizontal alignment handling
- use headerCell id instead of individualSlot - popin cells must always be left aligned Fixes: #11858 Fixes: #12155 Fixes: #12854
1 parent 5c3103d commit 825e285

File tree

8 files changed

+36
-40
lines changed

8 files changed

+36
-40
lines changed

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -493,12 +493,7 @@ describe("Table - Popin Mode", () => {
493493
describe("Table - Horizontal alignment of cells", () => {
494494
function check(id: string, index: number, alignment: string) {
495495
cy.get(id)
496-
.should("have.css", "justify-content", alignment)
497-
.and($el => {
498-
const style = $el.attr("style");
499-
const variable = style?.match(/justify-content: ([^;]+)/)?.[1] ?? "";
500-
expect(variable).to.equal(`var(--horizontal-align-default-${index})`);
501-
});
496+
.should("have.css", "justify-content", alignment);
502497

503498
cy.get("ui5-table-row")
504499
.get(`ui5-table-cell:nth-of-type(${index})`)
@@ -530,11 +525,11 @@ describe("Table - Horizontal alignment of cells", () => {
530525
<TableCell><Label><b>1249</b> EUR</Label></TableCell>
531526
</TableRow>
532527
<TableRow>
533-
<TableCell><Label><b>Notebook Basic 18</b><br></br>HT-1002</Label></TableCell>
534-
<TableCell><Label>Technocom</Label></TableCell>
535-
<TableCell><Label>32 x 21 x 4 cm</Label></TableCell>
536-
<TableCell><Label style={{ color: "#2b7c2b" }}><b>3.7</b> KG</Label></TableCell>
537-
<TableCell><Label><b>29</b> EUR</Label></TableCell>
528+
<TableCell class="productCol"><Label><b>Notebook Basic 18</b><br></br>HT-1002</Label></TableCell>
529+
<TableCell class="supplierCol"><Label>Technocom</Label></TableCell>
530+
<TableCell class="dimensionsCol"><Label>32 x 21 x 4 cm</Label></TableCell>
531+
<TableCell class="weightCol"><Label style={{ color: "#2b7c2b" }}><b>3.7</b> KG</Label></TableCell>
532+
<TableCell class="priceCol"><Label><b>29</b> EUR</Label></TableCell>
538533
</TableRow>
539534
</Table>
540535
);
@@ -647,8 +642,10 @@ describe("Table - Horizontal alignment of cells", () => {
647642

648643
if (shouldBePoppedIn) {
649644
check(`#${id}`, index + 1, "normal");
645+
cy.get(`.${id}`).should("have.css", "justify-content", "normal");
650646
} else {
651647
check(`#${id}`, index + 1, alignments[id]);
648+
cy.get(`.${id}`).should("have.css", "justify-content", alignments[id]);
652649
}
653650
});
654651
});

‎packages/main/src/Table.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -609,12 +609,10 @@ class Table extends UI5Element {
609609

610610
get styles() {
611611
const virtualizer = this._getVirtualizer();
612-
const headerStyleMap = this.headerRow?.[0]?.cells?.reduce((headerStyles, headerCell) => {
613-
if (headerCell.horizontalAlign !== undefined && !headerCell._popin) {
614-
headerStyles[`--horizontal-align-${headerCell._individualSlot}`] = headerCell.horizontalAlign;
615-
}
616-
return headerStyles;
617-
}, {} as { [key: string]: string });
612+
const headerStyleMap: Record<string, string> = {};
613+
this.headerRow[0]?.cells.forEach(headerCell => {
614+
headerStyleMap[`--halign-${headerCell._id}`] = headerCell.horizontalAlign || "normal";
615+
});
618616
return {
619617
table: {
620618
"grid-template-columns": this._gridTemplateColumns,

‎packages/main/src/TableCell.ts‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ class TableCell extends TableCellBase {
4040
super.onBeforeRendering();
4141
if (this.horizontalAlign) {
4242
this.style.justifyContent = this.horizontalAlign;
43-
} else if (this._individualSlot) {
44-
this.style.justifyContent = `var(--horizontal-align-${this._individualSlot})`;
43+
} else if (this._headerCell) {
44+
this.style.justifyContent = `var(--halign-${this._headerCell._id})`;
4545
}
4646
}
4747

@@ -52,22 +52,25 @@ class TableCell extends TableCellBase {
5252
}
5353

5454
get _headerCell() {
55-
const row = this.parentElement as TableRow;
56-
const table = row.parentElement as Table;
57-
const index = row.cells.indexOf(this);
58-
return table.headerRow[0].cells[index];
55+
const row = this.parentElement as TableRow | null;
56+
const table = row?.parentElement as Table | null;
57+
const index = row?.cells?.indexOf(this) ?? -1;
58+
59+
return (index !== -1) ? table?.headerRow?.[0]?.cells?.[index] : null;
5960
}
6061

6162
get _popinHeaderNodes() {
6263
const nodes: Node[] = [];
6364
const headerCell = this._headerCell;
64-
if (headerCell.popinText) {
65-
nodes.push(document.createTextNode(headerCell.popinText));
66-
} else {
67-
nodes.push(...this._headerCell.content.map(node => node.cloneNode(true)));
68-
}
69-
if (headerCell.action[0]) {
70-
nodes.push(headerCell.action[0].cloneNode(true));
65+
if (headerCell) {
66+
if (headerCell.popinText) {
67+
nodes.push(document.createTextNode(headerCell.popinText));
68+
} else {
69+
nodes.push(...headerCell.content.map(node => node.cloneNode(true)));
70+
}
71+
if (headerCell.action[0]) {
72+
nodes.push(headerCell.action[0].cloneNode(true));
73+
}
7174
}
7275
return nodes;
7376
}

‎packages/main/src/TableCustomAnnouncement.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ class TableCustomAnnouncement extends TableExtension {
204204

205205
const cells = [...row._visibleCells, ...row._popinCells];
206206
cells.flatMap(cell => {
207-
return cell._popin ? [cell._popinHeader!, cell._popinContent!] : [cell._headerCell, cell];
207+
return cell._popin ? [cell._popinHeader!, cell._popinContent!] : [cell._headerCell!, cell];
208208
}).forEach(node => {
209209
const nodeDescription = getAccessibilityDescription(node, true);
210210
descriptions.push(nodeDescription);

‎packages/main/src/TableHeaderCell.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,6 @@ class TableHeaderCell extends TableCellBase {
122122
@slot()
123123
action!: Array<TableHeaderCellActionBase>;
124124

125-
@property({ type: Boolean, noAttribute: true })
126-
_popin = false;
127-
128125
@query("slot:not([name])")
129126
_defaultSlot!: HTMLSlotElement;
130127

@@ -136,10 +133,7 @@ class TableHeaderCell extends TableCellBase {
136133

137134
onBeforeRendering() {
138135
super.onBeforeRendering();
139-
if (this._individualSlot) {
140-
// overwrite setting of TableCellBase so that the TableHeaderCell always uses the slot variable
141-
this.style.justifyContent = `var(--horizontal-align-${this._individualSlot})`;
142-
}
136+
this.style.justifyContent = this.horizontalAlign || "";
143137
toggleAttribute(this, "aria-sort", this.sortIndicator !== SortOrder.None, this.sortIndicator.toLowerCase());
144138
}
145139

‎packages/main/src/TableHeaderCellActionBase.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ abstract class TableHeaderCellActionBase extends UI5Element {
6060

6161
_onClick(e: UI5CustomEvent<Button, "click">) {
6262
// Retrieve the real action (if parent is header cell this instance is fine, otherwise retrieve it from the header cell)
63-
const action = this.parentElement?.hasAttribute("ui5-table-header-cell") ? this : ((this.getRootNode() as ShadowRoot).host as TableCell)._headerCell.action[0] as this;
63+
const action = this.parentElement?.hasAttribute("ui5-table-header-cell") ? this : ((this.getRootNode() as ShadowRoot).host as TableCell)._headerCell!.action[0] as this;
6464
action.fireDecoratorEvent("click", { targetRef: e.target as HTMLElement });
6565
e.stopPropagation();
6666
}

‎packages/main/src/themes/TableCellBase.css‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
box-sizing: border-box;
1111
}
1212

13+
:host([_popin]) {
14+
justify-content: normal !important;
15+
}
16+
1317
:host([tabindex]:focus) {
1418
outline: var(--sapContent_FocusWidth) var(--sapContent_FocusStyle) var(--sapContent_FocusColor);
1519
outline-offset: calc(-1 * var(--sapContent_FocusWidth));

‎packages/main/test/pages/Table.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
<ui5-table-cell><ui5-label>Very Best Screens</ui5-label></ui5-table-cell>
5454
<ui5-table-cell><div style="display: flex; flex-direction: row;"><ui5-label>30 x 18 x 3 cm</ui5-label><ui5-button>Calculate</ui5-button></div></ui5-table-cell>
5555
<ui5-table-cell><ui5-label style="color: #2b7c2b"><b>4.2</b> KG</ui5-label></ui5-table-cell>
56-
<ui5-table-cell><ui5-label><b>956</b> EUR</ui5-label><ui5-checkbox></ui5-checkbox></ui5-table-cell>
56+
<ui5-table-cell horizontal-align="End"><ui5-label><b>956</b> EUR</ui5-label><ui5-checkbox></ui5-checkbox></ui5-table-cell>
5757
</ui5-table-row>
5858
<ui5-table-row row-key="1" navigated interactive>
5959
<ui5-table-cell><ui5-label><b>Notebook Basic 16</b><br><a href="#">HT-1001</a></ui5-label></ui5-table-cell>

0 commit comments

Comments
 (0)