Add loading skeleton for DataTable and localize empty state messages#118
Add loading skeleton for DataTable and localize empty state messages#118dreglad wants to merge 2 commits intomarmelab:mainfrom
Conversation
| }) => { | ||
| const { perPage } = useListContext(); | ||
| const effectiveRowsCount = rowsCount ?? perPage ?? 5; | ||
|
|
There was a problem hiding this comment.
This skeleton should only display if the data takes more than 1 second to load. Otherwise I find skeletons distracting (they add an unnecessary blink). Use the useTimeout hook for that, as in https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/list/datatable/DataTableLoading.tsx.
| columnsCount = 5, | ||
| rowsCount, | ||
| }: { | ||
| columnsCount?: number; |
There was a problem hiding this comment.
columnsCount should be required
| rowsCount?: number; | ||
| }) => { | ||
| const { perPage } = useListContext(); | ||
| const effectiveRowsCount = rowsCount ?? perPage ?? 5; |
There was a problem hiding this comment.
Using perPage will reserve a too large space IMO. Use rowsCount and default it to 5.
| return ( | ||
| <div className="rounded-md border animate-pulse"> | ||
| <Table> | ||
| <TableHeader> |
There was a problem hiding this comment.
you need to account for bulk actions, too
| </TableRow> | ||
| </TableHeader> | ||
| <TableBody> | ||
| {Array.from({ length: effectiveRowsCount }).map((_, i) => ( |
There was a problem hiding this comment.
Could you dim the row more and more as the number of rows increases?
| @@ -286,9 +288,12 @@ const isPromise = (value: any): value is Promise<any> => | |||
| value && typeof value.then === "function"; | |||
|
|
|||
| const DataTableEmpty = () => { | |||
There was a problem hiding this comment.
Please add stories (in src/stories/data-table.stories.tsx) so that we can test this in isolation. Stories should should the various cases (fast loading, short loading, variable number of children, with and without bulk actions, etc).
|
|
||
| describe("DataTable", () => { | ||
| it("should render a skeleton when isPending is true", async () => { | ||
| render( |
There was a problem hiding this comment.
Render the component from the stories instead (see below)
| it("should render a skeleton when isPending is true", async () => { | ||
| render( | ||
| <ListContextProvider value={{ perPage: 25 } as ListControllerSuccessResult}> | ||
| <DataTable isPending={true}> |
There was a problem hiding this comment.
you should also test in the context of a data provider that actually loads data after a while
| <CommandList> | ||
| <CommandEmpty>No matching item found.</CommandEmpty> | ||
| <CommandEmpty> | ||
| {translate("ra.page.no_matches", { |
There was a problem hiding this comment.
make the key ra.input.autocomplete.no_matches instead
| <Alert> | ||
| <AlertDescription>No results found.</AlertDescription> | ||
| <AlertDescription> | ||
| {translate("ra.page.no_results", { _: "No results found." })} |
There was a problem hiding this comment.
use ra.navigation.no_results instead, and pass the resource name as translation param as in https://github.com/marmelab/react-admin/blob/340d4ab733bed1fb0705abcda3e9ea6eb8e0deea/packages/ra-ui-materialui/src/list/ListNoResults.tsx#L42-L46
|
Hi, any updates? |
2 participants
Summary
This PR introduces a visible loading skeleton for the
DataTablecomponent and localizes internal empty state labels that were previously hardcoded.Motivation
Currently, the DataTable displays a blank screen while fetching data from the API. While the library supports a loading state, it seems primarily intended for component lazy-loading, which doesn't account for the wait times typical of remote data fetching.
By adding a pulsing skeleton, we significantly improve the perceived performance and provide immediate visual feedback, ensuring the UI feels responsive during network requests.
Changes
DataTableSkeletonthat renders automatically during data fetching.perPagesetting fromListContextand matches the defined column count for a seamless UI transition.DataTableEmptyandAutocompleteInputto use standard React Admin translation keys (ra.page.no_resultsandra.page.no_matches), removing hardcoded labels.data-table.spec.tsxto verify the skeleton behavior and correct row rendering.Design Decision
I considered adding an opt-in prop for the skeleton to avoid changing existing UI expectations. However, I decided to prioritize API simplicity and integrated it as the default loading state. I believe this change is safe and aligns with modern design standards, though I'm open to making it configurable if the maintainers prefer.