Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui5-table): improve horizontal scrolling #9293

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 23 additions & 44 deletions packages/main/src/Table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from "./generated/i18n/i18n-defaults.js";
import BusyIndicator from "./BusyIndicator.js";
import TableCell from "./TableCell.js";
import { findVerticalScrollContainer, scrollElementIntoView } from "./TableUtils.js";

/**
* Interface for components that can be slotted inside the <code>features</code> slot of the <code>ui5-table</code>.
Expand Down Expand Up @@ -279,6 +280,17 @@ class Table extends UI5Element {
@property()
stickyTop = "0";

/**
* Defines the height of the component.
*
* **Note:** When using sticky headers, make sure that the height of the table is defined.
*
* @default "auto"
* @public
*/
@property()
height = "auto";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if i liked this property, i guess you know why.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it for now. Leads to issues if you set the table to Scroll mode (because of overflow-x: auto)


@property({ type: Number, noAttribute: true })
_invalidate = 0;

Expand Down Expand Up @@ -397,36 +409,8 @@ class Table extends UI5Element {
}

_onfocusin(e: FocusEvent) {
// Handles focus that is below sticky element
const stickyElements = this._stickyElements;

if (stickyElements.length === 0) {
return;
}

// Find the sticky element that is closest to the focused element
const target = e.target as HTMLElement;
const element = target.closest("ui5-table-cell, ui5-table-row") as HTMLElement ?? target;
const elementRect = element.getBoundingClientRect();
const stickyBottom = stickyElements.reduce((min, stickyElement) => {
const stickyRect = stickyElement.getBoundingClientRect();

if (stickyRect.bottom > elementRect.top) {
return Math.max(min, stickyRect.bottom);
}
return min;
}, -Infinity);

// If the focused element is not behind any sticky element, do nothing
if (stickyBottom === -Infinity) {
return;
}

// Scroll the focused element into view
const scrollContainer = this._scrollContainer;
scrollContainer.scrollBy({
top: elementRect.top - stickyBottom,
});
// 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");
}

/**
Expand Down Expand Up @@ -488,6 +472,8 @@ class Table extends UI5Element {
return {
table: {
"grid-template-columns": this._gridTemplateColumns,
"height": this.height,
DonkeyCo marked this conversation as resolved.
Show resolved Hide resolved
"overflow-x": this._tableOverflowX,
},
};
}
Expand All @@ -512,7 +498,7 @@ class Table extends UI5Element {
}

get _tableOverflowX() {
return (this.overflowMode === TableOverflowMode.Popin) ? "hidden" : "auto";
return (this.overflowMode === TableOverflowMode.Popin) ? "" : "auto";
DonkeyCo marked this conversation as resolved.
Show resolved Hide resolved
}

get _tableOverflowY() {
Expand Down Expand Up @@ -560,22 +546,15 @@ class Table extends UI5Element {
return this.features.find(feature => this._isGrowingFeature(feature)) as ITableGrowing;
}

// TODO: Could be moved to UI5Element. TBD
get _scrollContainer() {
let element: HTMLElement = this as HTMLElement;
while (element) {
const { overflowY } = window.getComputedStyle(element);
if (overflowY === "auto" || overflowY === "scroll") {
return element;
}
element = element.parentElement as HTMLElement;
}
get _stickyElements() {
const stickyRows = [this.headerRow[0]].filter(row => row.sticky);
DonkeyCo marked this conversation as resolved.
Show resolved Hide resolved
const stickyColumns = this.headerRow[0]._stickyCells as TableHeaderCell[];

return document.scrollingElement as HTMLElement || document.documentElement;
return [...stickyRows, ...stickyColumns];
}

get _stickyElements() {
return [this.headerRow[0]].filter(row => row.sticky);
get _scrollContainer() {
return findVerticalScrollContainer(this._tableElement);
}

get isTable() {
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/TableHeaderRow.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<ui5-table-header-cell id="selection-cell"
aria-selected="{{_isSelected}}"
aria-label="{{_i18nSelection}}"
fixed
ui5-table-selection-component
>
{{#if _isMultiSelect}}
Expand Down
7 changes: 7 additions & 0 deletions packages/main/src/TableHeaderRow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class TableHeaderRow extends TableRowBase {
/**
* Sticks the `ui5-table-header-row` to the top of a table.
*
* Note: If used in combination with overflowMode "Scroll", the table needs a defined height for the sticky header to work as expected.
*
* @default false
* @public
*/
Expand Down Expand Up @@ -97,6 +99,11 @@ class TableHeaderRow extends TableRowBase {
get _i18nRowPopin() {
return TableRowBase.i18nBundle.getText(TABLE_ROW_POPIN);
}

get _cells() {
DonkeyCo marked this conversation as resolved.
Show resolved Hide resolved
// only add if not null #selection-cell if it is not null to the array
DonkeyCo marked this conversation as resolved.
Show resolved Hide resolved
return [this.shadowRoot?.querySelector("#selection-cell"), ...this.cells];
}
}

TableHeaderRow.define();
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/TableRow.hbs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{#if _hasRowSelector}}
<ui5-table-cell id="selection-cell" aria-selected="{{_isSelected}}" ui5-table-selection-component>
<ui5-table-cell id="selection-cell" aria-selected="{{_isSelected}}" fixed ui5-table-selection-component>
{{#if _isMultiSelect}}
<ui5-checkbox id="selection-component" tabindex="-1"
?checked="{{_isSelected}}"
Expand Down
12 changes: 8 additions & 4 deletions packages/main/src/TableRowBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,16 @@ abstract class TableRowBase extends UI5Element {
return this.cells.filter(c => c._popin);
}

get _i18nRowSelector(): string {
return TableRowBase.i18nBundle.getText(TABLE_ROW_SELECTOR);
get _stickyCells() {
const selectionCell = this.shadowRoot?.querySelector("#selection-cell"),
navigatedCell = this.shadowRoot?.querySelector("#navigated-cell");

// filter out null/undefined
return [selectionCell, ...this.cells, navigatedCell].filter(cell => cell?.hasAttribute("fixed"));
}

get isTableRowBase() {
return true;
get _i18nRowSelector(): string {
return TableRowBase.i18nBundle.getText(TABLE_ROW_SELECTOR);
}
}

Expand Down
6 changes: 4 additions & 2 deletions packages/main/src/TableSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,11 @@ class TableSelection extends UI5Element implements ITableFeature {
return;
}

if (!eventOrigin.hasAttribute("ui5-table-row") || !this._rangeSelection || isShift(e) || !isSelectionCheckbox(e)) {
if (!eventOrigin.hasAttribute("ui5-table-row") || !this._rangeSelection || !isShift(e)) {
// Stop range selection if a) Shift is relased or b) the event target is not a row or c) the event is not from the selection checkbox
this._stopRangeSelection();
if (isSelectionCheckbox(e)) {
this._stopRangeSelection();
}
}

if (this._rangeSelection) {
Expand Down
58 changes: 58 additions & 0 deletions packages/main/src/TableUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,67 @@ 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 => {
while (element) {
const { overflowY } = window.getComputedStyle(element);
if (overflowY === "auto" || overflowY === "scroll") {
return element;
}

if (element.parentNode instanceof ShadowRoot) {
element = element.parentNode.host as HTMLElement;
} else {
element = element.parentElement as HTMLElement;
}
}

return document.scrollingElement as HTMLElement || document.documentElement;
};

const scrollElementIntoView = (scrollContainer: HTMLElement, element: HTMLElement, stickyElements: HTMLElement[], isRtl: boolean) => {
if (stickyElements.length === 0) {
return;
}

const elementRect = element.getBoundingClientRect();
const inline = isRtl ? "right" : "left";

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) {
DonkeyCo marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}

return { x, y };
}, { x: elementRect[inline], y: elementRect.top });

const scrollX = elementRect[inline] - stickyX;
const scrollY = elementRect.top - stickyY;

if (scrollX === 0 && scrollY === 0) {
// avoid unnecessary scroll call, when nothing changes
return;
}

scrollContainer.scrollBy({
top: scrollY,
left: scrollX,
});
};

export {
isInstanceOfTable,
isSelectionCheckbox,
isHeaderSelector,
findRowInPath,
findVerticalScrollContainer,
scrollElementIntoView,
};
6 changes: 6 additions & 0 deletions packages/main/src/themes/TableCellBase.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,10 @@
:host(#selection-cell) {
width: auto;
min-width: auto;
background-color: inherit;
}

:host([fixed]) {
position: sticky;
z-index: 1;
}
2 changes: 1 addition & 1 deletion packages/main/src/themes/TableHeaderRow.css
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
:host([sticky]) {
position: sticky;
top: var(--ui5_grid_sticky_top, 0);
z-index: 1;
z-index: 2;
}

#popin-cell {
Expand Down
6 changes: 6 additions & 0 deletions packages/main/src/themes/TableRow.css
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@
grid-row: span 2;
min-width: 0;
padding: 0;
position: sticky;
right: 0;
}

:dir(rtl)#navigated-cell {
left: 0;
}

:host([navigated]) #navigated-cell {
Expand Down
16 changes: 15 additions & 1 deletion packages/main/src/themes/TableRowBase.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
display: grid;
grid-template-columns: subgrid;
grid-column: 1 / -1;
border-bottom: var(--sapList_BorderWidth) solid var(--sapList_BorderColor);
min-height: var(--_ui5_list_item_base_height);
box-sizing: border-box;
border-bottom: var(--sapList_BorderWidth) solid var(--sapList_BorderColor);
}

:host([tabindex]:focus) {
Expand All @@ -13,8 +14,21 @@

#selection-cell {
padding: 0;
left: 0;
}

:dir(rtl)#selection-cell {
right: 0;
}

#selection-component {
vertical-align: middle;
}

/** Focus outline for the selection cell */
:host([tabindex]:focus) #selection-cell {
outline: none;
box-shadow: var(--_ui5_table_shadow_border_top),
var(--_ui5_table_shadow_border_left),
var(--_ui5_table_shadow_border_bottom); /* There is a 1px difference between row and cell, therefore the outline is also 1px difference between their outline offsets */
}
9 changes: 9 additions & 0 deletions packages/main/src/themes/base/Table-parameters.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,13 @@
--_ui5_table_cell_valign: center;
--_ui5_table_cell_min_width: 2.75rem;
--_ui5_table_navigated_cell_width: 0.25rem;
--_ui5_table_shadow_border_left: inset var(--sapContent_FocusWidth) 0 var(--sapContent_FocusColor);
--_ui5_table_shadow_border_right: inset calc(-1 * var(--sapContent_FocusWidth)) 0 var(--sapContent_FocusColor);
--_ui5_table_shadow_border_top: inset 0 var(--sapContent_FocusWidth) var(--sapContent_FocusColor);
--_ui5_table_shadow_border_bottom: inset 0 -1px var(--sapContent_FocusColor);
}

[dir="rtl"] {
--_ui5_table_shadow_border_left: inset calc(-1 * var(--sapContent_FocusWidth)) 0 var(--sapContent_FocusColor);
--_ui5_table_shadow_border_right: inset var(--sapContent_FocusWidth) 0 var(--sapContent_FocusColor);
}
12 changes: 4 additions & 8 deletions packages/main/test/pages/Table.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,20 @@
</style>
</head>

<body>
<body dir="rtl">
<!-- toolbar with ui5-bar -->
<ui5-bar design="Header" accessible-name-ref="title" style="position: sticky; top: 0; z-index: 2; height: 50px;">
<ui5-title tabindex="0" level="H3" id="title" slot="startContent">My Selectable Products (3)</ui5-title>
<ui5-slider id="slider" min="0" max="100" step="1" value="100"
label-interval="0"></ui5-slider>
</ui5-bar>

<!-- <div style="height: 300px; overflow: auto;"> -->

<ui5-table id="table" overflow-mode="Scroll" sticky-top="50px" accessible-name-ref="title">
<ui5-table id="table" overflow-mode="Scroll" height="500px" sticky-top="0px" accessible-name-ref="title">
<ui5-table-growing id="growing" type="Scroll" slot="features"></ui5-table-growing>
<ui5-table-selection id="selection" selected="0 2" slot="features"></ui5-table-selection>
<ui5-table-header-row slot="headerRow">
<ui5-table-header-row slot="headerRow" sticky>
<ui5-table-header-cell id="produtCol" width="300px"><span>Product</span></ui5-table-header-cell>
<ui5-table-header-cell id="supplierCol">Supplier</ui5-table-header-cell>
<ui5-table-header-cell id="supplierCol" width="400px">Supplier</ui5-table-header-cell>
<ui5-table-header-cell id="dimensionsCol" importance="-1" min-width="300px">Dimensions</ui5-table-header-cell>
<ui5-table-header-cell id="weightCol" popin-text="Weight">Weight</ui5-table-header-cell>
<ui5-table-header-cell id="priceCol" min-width="220px">Price</ui5-table-header-cell>
Expand Down Expand Up @@ -86,8 +84,6 @@
</ui5-table-row>
</ui5-table>

<!-- </div> -->

<ui5-input value="after table" data-sap-ui-fastnavgroup="true"></ui5-input>
<script>
const slider = document.getElementById("slider");
Expand Down
6 changes: 3 additions & 3 deletions packages/main/test/pages/TableFixedHeader.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<ui5-slider id="slider" min="0" max="100" step="1" value="100"
label-interval="0"></ui5-slider>
</ui5-bar>
<ui5-table id="table0" overflow-mode="Scroll" sticky-top="50px" accessible-name-ref="title" no-data-text="No data found">
<ui5-table id="table0" overflow-mode="Popin" sticky-top="50px" accessible-name-ref="title" no-data-text="No data found">
<ui5-table-header-row sticky slot="headerRow">
<ui5-table-header-cell id="colA" min-width="300px"><span>ColumnA</span></ui5-table-header-cell>
<ui5-table-header-cell id="colB" min-width="200px">Column B</ui5-table-header-cell>
Expand Down Expand Up @@ -49,7 +49,7 @@
</ui5-table>
</div>

<ui5-table id="table1" overflow-mode="Scroll" sticky-top="0" accessible-name-ref="title" no-data-text="No data found" style="height: 300px; overflow: auto;">
<ui5-table id="table1" overflow-mode="Popin" sticky-top="0" accessible-name-ref="title" no-data-text="No data found" style="height: 300px; overflow: auto;">
<ui5-table-header-row sticky slot="headerRow">
<ui5-table-header-cell id="colA" min-width="300px"><span>ColumnA</span></ui5-table-header-cell>
<ui5-table-header-cell id="colB" min-width="200px">Column B</ui5-table-header-cell>
Expand Down Expand Up @@ -85,7 +85,7 @@
<ui5-slider id="slider" min="0" max="100" step="1" value="100"
label-interval="0"></ui5-slider>
</ui5-bar>
<ui5-table id="table2" overflow-mode="Scroll" sticky-top="50px" accessible-name-ref="title" no-data-text="No data found">
<ui5-table id="table2" overflow-mode="Popin" sticky-top="50px" accessible-name-ref="title" no-data-text="No data found">
<ui5-table-header-row sticky slot="headerRow">
<ui5-table-header-cell id="colA" min-width="300px"><span>ColumnA</span></ui5-table-header-cell>
<ui5-table-header-cell id="colB" min-width="200px">Column B</ui5-table-header-cell>
Expand Down
Loading