feat(ui5-grid): implementing new grid component
- introduces a new table/grid component
- enhanced accessibility (roles are in the light dom)
- new API mirroring the structure of the HTML table
Hi @DonkeyCo row-click instead of row-press is also more consistent with the existing components and events.
*The PR needs to be merged with the latest code in main as there are many unrelated files, shown as changed
Belize is removed
so you can remove any files related to the Belize theme
GridRow
- We have to check if the
keyproperty could be an issue in React. there it's kind of system property as in React all repetitive elements like table rows and list items are forced to havekeyset for performance reason and as far I remember the key attribute is even not rendered and might cause some problems with the component state.
GridRow / GridSelection
- I find it odd that I can't just set
selectedon theui5-grid-rowelement, which is usually very convenient when the component like this is used in any framework template.
<ui5-grid>
<ui5-grid-row selected></ui5-grid-row>
</ui5-grid>
<ui5-grid>
{
this.rows.map((row) => {
<ui5-grid-row selected={{row.selected}}></ui5-grid-row>
});
}
</ui5-grid>
If I got it right, the same can be achieved now via ui5-grid-selection element passing indexes of rows separated with spaces, to its selected property:
<ui5-grid>
<ui5-grid-row></ui5-grid-row>
<ui5-grid-selection selected="0 1 2" slot="features"></ui5-grid-selection>
</ui5-grid>
const selectedRowsIndexes = this.rows.map((row, idx) => {
if (row.selected) {
return idx;
}
}).join(" ");
<ui5-grid>
{
this.rows.map((row) => {
<ui5-grid-row></ui5-grid-row>
});
}
<ui5-grid-selection selected="{selectedRowsIndexes}" slot="features"></ui5-grid-selection>
</ui5-grid>
Keyboard handling
It's really improved, the cell based navigation is completely new I only noticed that the cell outline overlaps with the content of the cell. I consider it minor, but if it's something easy to fix
Since tag
- put
@since 2.0on all Grid* classes (at least the public ones)
Futures that are currently missing compared to the current Table,
that can be added later-on if we find it important
- Grouping of rows,
- TableRow "navigated"
@ilhan007 thanks for the review. Regarding your comments:
*The PR needs to be merged with the latest code in main as there are many unrelated files, shown as changed
Belize is removed
so you can remove any files related to the Belize theme
Should be solved with the upcoming commit.
GridRow
- We have to check if the
keyproperty could be an issue in React. there it's kind of system property as in React all repetitive elements like table rows and list items are forced to havekeyset for performance reason and as far I remember the key attribute is even not rendered and might cause some problems with the component state.
Yeah. Now that you mention it, it probably won't work. I've seen other components use primaryKey instead of key, maybe this is better? @aborjinik maybe you want to chime in as well?
GridRow / GridSelection
- I find it odd that I can't just set
selectedon theui5-grid-rowelement, which is usually very convenient when the component like this is used in any framework template.<ui5-grid> <ui5-grid-row selected></ui5-grid-row> </ui5-grid><ui5-grid> { this.rows.map((row) => { <ui5-grid-row selected={{row.selected}}></ui5-grid-row> }); } </ui5-grid>If I got it right, the same can be achieved now via
ui5-grid-selectionelement passing indexes of rows separated with spaces, to itsselectedproperty:<ui5-grid> <ui5-grid-row></ui5-grid-row> <ui5-grid-selection selected="0 1 2" slot="features"></ui5-grid-selection> </ui5-grid>const selectedRowsIndexes = this.rows.map((row, idx) => { if (row.selected) { return idx; } }).join(" "); <ui5-grid> { this.rows.map((row) => { <ui5-grid-row></ui5-grid-row> }); } <ui5-grid-selection selected="{selectedRowsIndexes}" slot="features"></ui5-grid-selection> </ui5-grid>
I see the use case, where selected directly on the row could be nice, but comparing our selection API to other components, we are not the only ones doing this via a property. Vaadin is also doing this similarly. Boils down to personal preference, but I think it's just nicer to decouple selection from the rows itself and have a central place to query it.
Keyboard handling
It's really improved, the cell based navigation is completely new I only noticed that the cell outline overlaps with the content of the cell. I consider it minor, but if it's something easy to fix
Will have a look at this issue.
Since tag
- put
@since 2.0on all Grid* classes (at least the public ones)
Fair point. I'll add that in the upcoming commit.
Futures that are currently missing compared to the current Table,
that can be added later-on if we find it important
- Grouping of rows,
- TableRow "navigated"
We have a BLI already for navigation indication (as well as row actions), grouping is not on the list yet, but I'll bring it up with the colleagues.
