Skip to content

Commit

Permalink
fix: Fix loading state behavior
Browse files Browse the repository at this point in the history
This makes sure there is no incorrect intermediate "no results" empty
state which happens when API has fulfilled its request but the table
hasn't been yet updated with the new rows.
  • Loading branch information
gkarat committed Jul 24, 2023
1 parent 6a6c6a8 commit e25be14
Showing 1 changed file with 46 additions and 29 deletions.
75 changes: 46 additions & 29 deletions src/components/GroupsTable/GroupsTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ const groupsTableFiltersConfig = {

const GroupsTable = () => {
const dispatch = useDispatch();
const { rejected, uninitialized, loading, data } = useSelector(
const { rejected, uninitialized, loading, fulfilled, data } = useSelector(
(state) => state.groups
);
const [rowsGenerated, setRowsGenerated] = useState(false);
const location = useLocation();
const [filters, setFilters] = useState({
...GROUPS_TABLE_INITIAL_STATE,
Expand Down Expand Up @@ -171,6 +172,7 @@ const GroupsTable = () => {
selected: selectedIds.includes(group.id),
}));
setRows(newRows);
setRowsGenerated(!uninitialized && !loading); // set to true only after the API call is complete

if (selectedIds.length === 1) {
setSelectedGroup({
Expand All @@ -180,7 +182,7 @@ const GroupsTable = () => {
} else {
setSelectedGroup(undefined);
}
}, [groups, selectedIds]);
}, [loading, selectedIds]);

// TODO: convert initial URL params to filters

Expand Down Expand Up @@ -242,35 +244,50 @@ const GroupsTable = () => {
const onPerPageSelect = (event, perPage) =>
setFilters({ ...filters, perPage, page: 1 }); // will also reset the page to first

const tableRows = useMemo(
() =>
uninitialized || loading
? generateLoadingRows(GROUPS_TABLE_COLUMNS.length, filters.perPage)
: rejected || rows.length === 0
? [
{
fullWidth: true,
cells: [
{
title: rejected ? (
// TODO: don't render the primary button (requires change in FF)
<ErrorState />
) : (
<NoEntitiesFound
entities="groups"
onClearAll={onResetFilters}
/>
),
props: {
colSpan: GROUPS_TABLE_COLUMNS.length + 1,
},
const tableRows = useMemo(() => {
if (uninitialized || loading || !rowsGenerated) {
return generateLoadingRows(GROUPS_TABLE_COLUMNS.length, filters.perPage);
}

if (fulfilled) {
if (rows.length === 0) {
return [
{
fullWidth: true,
cells: [
{
title: (
<NoEntitiesFound
entities="groups"
onClearAll={onResetFilters}
/>
),
props: {
colSpan: GROUPS_TABLE_COLUMNS.length + 1,
},
],
},
],
},
];
} else {
return rows; // the happy path
}
}

return [
{
fullWidth: true,
cells: [
{
title: <ErrorState />, // TODO: don't render the primary button (requires change in FF)
props: {
colSpan: GROUPS_TABLE_COLUMNS.length + 1,
},
]
: rows,
[uninitialized, loading, rejected, rows, filters.perPage]
);
},
],
},
];
}, [uninitialized, loading, rejected, rows, filters.perPage]);

// TODO: use ouiaSafe to indicate the loading state for e2e tests

Expand Down

0 comments on commit e25be14

Please sign in to comment.