Skip to content

Commit

Permalink
fix(ui5-table): wrong horiz. alignm. behavior and wrong texts (#10040)
Browse files Browse the repository at this point in the history
Fixed a problem where the horizontalAlign property would be applied to cells inside the popin area.
In addition I've changed some of the texts reviewed in the last PR (#9639).

fixes: #10017
  • Loading branch information
nowakdaniel authored Oct 18, 2024
1 parent efd7e6a commit f3da992
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/main/src/Table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ class Table extends UI5Element {

get styles() {
const headerStyleMap = this.headerRow?.[0]?.cells?.reduce((headerStyles, headerCell) => {
if (headerCell.horizontalAlign !== undefined) {
if (headerCell.horizontalAlign !== undefined && !headerCell._popin) {
headerStyles[`--horizontal-align-${headerCell._individualSlot}`] = headerCell.horizontalAlign;
}
return headerStyles;
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/TableCellBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ abstract class TableCellBase extends UI5Element {

/**
* Determines the horizontal alignment of table cells.
* Note: All values valid for justify-content can be used not just the ones inside the enum.
* **Note:** All values valid for justify-content can be used, not just the ones inside the enumeration.
* @default undefined
* @public
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/main/test/pages/HAlignTable.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

<body style="background-color: var(--sapBackgroundColor)">
<div class="section">
<ui5-table id="table" overflow-mode="Popin">
<ui5-table id="table" overflow-mode="Popin" style="width: 1120px;">
<ui5-table-header-row slot="headerRow">
<ui5-table-header-cell id="productCol" width="300px"><span>Product</span></ui5-table-header-cell>
<ui5-table-header-cell id="supplierCol" horizontal-align="Center" width="200px">Supplier</ui5-table-header-cell>
Expand Down
66 changes: 66 additions & 0 deletions packages/main/test/specs/Table.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,72 @@ describe("Table - Horizontal alignment of cells", async () => {
assert.equal(justifyContent.value, index === rowWithChangedCell ? customAlignmentCell : alignment, "justify-content correctly set.");
}
});

it("test horizontalAlign behavior with one by one popping in", async() => {
let table = await browser.$("#table");
assert.ok(table.isExisting(), "Table exists");

const headerRow = await table.$("ui5-table-header-row");
const headerCells = await headerRow.$$("ui5-table-header-cell");

const testWidths = [
{width: 1120, poppedIn: []},
{width: 900, poppedIn: ["priceCol"]},
{width: 800, poppedIn: ["priceCol", "weightCol"]},
{width: 500, poppedIn: ["priceCol", "weightCol", "dimensionsCol"]},
{width: 300, poppedIn: ["priceCol", "weightCol", "dimensionsCol", "supplierCol"]}
];

for (const testWidth of testWidths) {
await table.setProperty("style", `width: ${testWidth.width}px`);
assert.strictEqual(await table.getSize("width"), testWidth.width, `Table is ${testWidth.width} wide`);

for (const [index, headerCell] of headerCells.entries()) {
const headerRole = await headerCell.getAttribute("role");
const headerId = await headerCell.getAttribute("id");
const slotName = await headerCell.getAttribute("slot");

let expectRole = true;
if (testWidth.poppedIn.includes(headerId)) {
expectRole = false;
}

assert.strictEqual(headerRole === ROLE_COLUMN_HEADER, expectRole, `Cell has role (width: ${testWidth.width}): (${expectRole})`)
assert.strictEqual(await headerRow.shadow$(`slot[name=${slotName}]`).isExisting(), expectRole, `Header cell ${slotName} has been rendered (width: ${testWidth.width}): ${expectRole}`);

const style = await headerCell.getAttribute("style");
const justifyContentHeaderCellUncomputed = style.match(/justify-content: ([^;]+)/)[1];
const cssVariable = `var(--horizontal-align-default-${index+1})`;
assert.equal(justifyContentHeaderCellUncomputed, cssVariable);

// justify-content should be the default value in case the cell is inside the popin area
if (!expectRole) {
const tableRows = await table.$$("ui5-table-row");
for (const row of tableRows) {
const rowCells = await row.$$("ui5-table-cell");
const justifyContent = await rowCells[index].getCSSProperty("justify-content");

assert.equal(justifyContent.value, "normal", "justify-content correctly set.");
}
}
}
}

/* const justifyContentHeaderCell = await headerCell.getCSSProperty("justify-content");
const style = await headerCell.getAttribute("style");
const justifyContentHeaderCellUncomputed = style.match(/justify-content: ([^;]+)/)[1];
const cssVariable = "var(--horizontal-align-default-1)";
assert.equal(justifyContentHeaderCell.value, alignment, "justify-content correctly set.");
assert.equal(justifyContentHeaderCellUncomputed, cssVariable, "horizontalAlign not set");
const tableRows = await table.$$("ui5-table-row");
for (const row of tableRows) {
const rowCells = await row.$$("ui5-table-cell");
const justifyContent = await rowCells[3].getCSSProperty("justify-content");
assert.equal(justifyContent.value, alignment, "justify-content correctly set.");
} */
});
});

// Tests for the fixed header, whether it is shown correctly and behaves as expected
Expand Down
14 changes: 7 additions & 7 deletions packages/website/docs/_components_pages/main/Table/TableCell.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import HAlign from "../../../_samples/main/Table/HAlign/HAlign.md";

## Horizontal cell alignment

Control the horizontal alignment of cells by utilizing the `horizontalAlign` property.
Controls the horizontal alignment of cells by using the `horizontalAlign` property.

Please note that the behaviour of `horizontalAlign` depends on where you set it:
1. `horizontalAlign` is set on `TableHeaderCell` level<br />
Changes the horizontal alignment of the `TableHeaderCell` and their corresponding `TableCells`.
2. `horizontalAlign` is set on `TableCell` level only<br />
Please note that the behavior of `horizontalAlign` depends on where you set it:
- `horizontalAlign` is set on `TableHeaderCell` level<br />
Changes the horizontal alignment of the `TableHeaderCell` and its corresponding `TableCell`.
- `horizontalAlign` is set on `TableCell` level only<br />
The horizontal alignment is only changed for this one `TableCell`
3. `horizontalAlign` is set on `TableHeaderCell` and `TableCell` level<br />
Changing the `horizontalAlign` property on `TableHeaderCell` level changes only the `TableHeaderCell` itself and those `TableCells` without a `horizontalAlign` property.
- `horizontalAlign` is set on `TableHeaderCell` and `TableCell` level<br />
Changing the `horizontalAlign` property on `TableHeaderCell` level changes only the `TableHeaderCell` itself and those `TableCell` without a `horizontalAlign` property.

<HAlign />
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import HAlign from "../../../_samples/main/Table/HAlign/HAlign.md";

## Horizontal cell alignment

Control the horizontal alignment of cells by utilizing the `horizontalAlign` property.
Controls the horizontal alignment of cells by using the `horizontalAlign` property.

Please note that the behaviour of `horizontalAlign` depends on where you set it:
1. `horizontalAlign` is set on `TableHeaderCell` level<br />
Changes the horizontal alignment of the `TableHeaderCell` and their corresponding `TableCells`.
2. `horizontalAlign` is set on `TableCell` level only<br />
Please note that the behavior of `horizontalAlign` depends on where you set it:
- `horizontalAlign` is set on `TableHeaderCell` level<br />
Changes the horizontal alignment of the `TableHeaderCell` and its corresponding `TableCell`.
- `horizontalAlign` is set on `TableCell` level only<br />
The horizontal alignment is only changed for this one `TableCell`
3. `horizontalAlign` is set on `TableHeaderCell` and `TableCell` level<br />
Changing the `horizontalAlign` property on `TableHeaderCell` level changes only the `TableHeaderCell` itself and those `TableCells` without a `horizontalAlign` property.
- `horizontalAlign` is set on `TableHeaderCell` and `TableCell` level<br />
Changing the `horizontalAlign` property on `TableHeaderCell` level changes only the `TableHeaderCell` itself and those `TableCell` without a `horizontalAlign` property.

<HAlign />

0 comments on commit f3da992

Please sign in to comment.