-
Notifications
You must be signed in to change notification settings - Fork 0
Fix data fetching in content management #17
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
Changes from all commits
67e94ab
0017d1d
9a0ca80
6b4961d
2bb5cce
e6ace78
4e90352
2d7f111
5e906da
43e7155
fa42e40
2a4a70b
a476def
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import 'package:ht_dashboard/content_management/bloc/content_management_bloc.dar | |
import 'package:ht_dashboard/l10n/app_localizations.dart'; | ||
import 'package:ht_dashboard/l10n/l10n.dart'; | ||
import 'package:ht_dashboard/router/routes.dart'; | ||
import 'package:ht_dashboard/shared/constants/pagination_constants.dart'; | ||
import 'package:ht_dashboard/shared/constants/app_spacing.dart'; | ||
import 'package:ht_dashboard/shared/widgets/failure_state_widget.dart'; | ||
import 'package:ht_dashboard/shared/widgets/loading_state_widget.dart'; | ||
|
@@ -23,14 +24,12 @@ class CategoriesPage extends StatefulWidget { | |
} | ||
|
||
class _CategoriesPageState extends State<CategoriesPage> { | ||
static const int _rowsPerPage = 10; | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
context.read<ContentManagementBloc>().add( | ||
const LoadCategoriesRequested(limit: _rowsPerPage), | ||
); | ||
const LoadCategoriesRequested(limit: kDefaultRowsPerPage), | ||
); | ||
} | ||
|
||
@override | ||
|
@@ -53,8 +52,8 @@ class _CategoriesPageState extends State<CategoriesPage> { | |
return FailureStateWidget( | ||
message: state.errorMessage ?? l10n.unknownError, | ||
onRetry: () => context.read<ContentManagementBloc>().add( | ||
const LoadCategoriesRequested(limit: _rowsPerPage), | ||
), | ||
const LoadCategoriesRequested(limit: kDefaultRowsPerPage), | ||
), | ||
); | ||
} | ||
|
||
|
@@ -82,20 +81,23 @@ class _CategoriesPageState extends State<CategoriesPage> { | |
source: _CategoriesDataSource( | ||
context: context, | ||
categories: state.categories, | ||
isLoading: state.categoriesStatus == ContentManagementStatus.loading, | ||
hasMore: state.categoriesHasMore, | ||
l10n: l10n, | ||
), | ||
rowsPerPage: _rowsPerPage, | ||
availableRowsPerPage: const [_rowsPerPage], | ||
rowsPerPage: kDefaultRowsPerPage, | ||
availableRowsPerPage: const [kDefaultRowsPerPage], | ||
onPageChanged: (pageIndex) { | ||
final newOffset = pageIndex * _rowsPerPage; | ||
final newOffset = pageIndex * kDefaultRowsPerPage; | ||
if (newOffset >= state.categories.length && | ||
state.categoriesHasMore) { | ||
state.categoriesHasMore && | ||
state.categoriesStatus != ContentManagementStatus.loading) { | ||
context.read<ContentManagementBloc>().add( | ||
LoadCategoriesRequested( | ||
startAfterId: state.categoriesCursor, | ||
limit: _rowsPerPage, | ||
), | ||
); | ||
LoadCategoriesRequested( | ||
startAfterId: state.categoriesCursor, | ||
limit: kDefaultRowsPerPage, | ||
), | ||
); | ||
} | ||
}, | ||
empty: Center(child: Text(l10n.noCategoriesFound)), | ||
|
@@ -117,16 +119,27 @@ class _CategoriesDataSource extends DataTableSource { | |
_CategoriesDataSource({ | ||
required this.context, | ||
required this.categories, | ||
required this.isLoading, | ||
required this.hasMore, | ||
required this.l10n, | ||
}); | ||
|
||
final BuildContext context; | ||
final List<Category> categories; | ||
final bool isLoading; | ||
final bool hasMore; | ||
final AppLocalizations l10n; | ||
|
||
@override | ||
DataRow? getRow(int index) { | ||
if (index >= categories.length) { | ||
// This can happen if hasMore is true and the user is on the last page. | ||
// If we are loading, show a spinner. Otherwise, we've reached the end. | ||
if (isLoading) { | ||
return DataRow2( | ||
cells: List.generate(3, (_) => const DataCell(Center(child: CircularProgressIndicator()))), | ||
); | ||
} | ||
return null; | ||
} | ||
final category = categories[index]; | ||
|
@@ -152,8 +165,8 @@ class _CategoriesDataSource extends DataTableSource { | |
onPressed: () { | ||
// Dispatch delete event | ||
context.read<ContentManagementBloc>().add( | ||
DeleteCategoryRequested(category.id), | ||
); | ||
DeleteCategoryRequested(category.id), | ||
); | ||
}, | ||
), | ||
], | ||
|
@@ -164,10 +177,20 @@ class _CategoriesDataSource extends DataTableSource { | |
} | ||
|
||
@override | ||
bool get isRowCountApproximate => false; | ||
bool get isRowCountApproximate => hasMore; | ||
|
||
@override | ||
int get rowCount => categories.length; | ||
int get rowCount { | ||
// If we have more items to fetch, we add 1 to the current length. | ||
// This signals to PaginatedDataTable2 that there is at least one more page, | ||
// which enables the 'next page' button. | ||
if (hasMore) { | ||
// When loading, we show an extra row for the spinner. | ||
// Otherwise, we just indicate that there are more rows. | ||
return isLoading ? categories.length + 1 : categories.length + kDefaultRowsPerPage; | ||
} | ||
return categories.length; | ||
} | ||
Comment on lines
+183
to
+193
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning
|
||
|
||
@override | ||
int get selectedRowCount => 0; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reloading the first page after a CUD operation can be a poor user experience if the user has paginated. Consider optimistic updates or invalidation with smart refetching to maintain the user's position.