Feat: Added ShadPagination. Implementation of #237#581
Feat: Added ShadPagination. Implementation of #237#581asare-21 wants to merge 44 commits intonank1ro:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds a new ShadPagination widget and ShadPaginationController with full pagination UI and logic, introduces ShadPaginationTheme (declaration + generated mixin), wires theme into ShadThemeData and variants, exports the component, adds example/playground pages and route, and adds comprehensive tests and changelog entries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
lib/src/components/pagination.dart (1)
513-514: Consider using dart:math for min/max functions.The widget defines local
minandmaxhelper methods. Dart'sdart:mathlibrary provides these as top-level functions. Using the standard library functions would be more idiomatic and eliminate the need for custom implementations.🔎 Proposed change
+import 'dart:math' show max, min; import 'package:flutter/material.dart'; import 'package:shadcn_ui/shadcn_ui.dart'; // ... rest of file ... - T min<T extends num>(T a, T b) => a < b ? a : b; - T max<T extends num>(T a, T b) => a > b ? a : b; }playground/lib/pages/pagination.dart (3)
9-9: Addconstkeyword for consistency.
PaginationExamplehas a const constructor, so the instantiation should useconst.🔎 Proposed fix
@override Widget build(BuildContext context) { - return PaginationExample(); + return const PaginationExample(); }
40-40: Remove or update the stale comment.The comment "Add your pagination widget implementation here" is misleading since the implementation is already present on Line 41.
🔎 Proposed fix
const SizedBox(height: 20), - // Add your pagination widget implementation here ShadPagination(
45-47: Consider usingdebugPrintinstead ofWhile
debugPrintis the Flutter-recommended alternative as it handles longer outputs and is automatically stripped in release builds.🔎 Proposed fix
onPageChanged: (value) { - print('Page changed to ${value + 1}'); + debugPrint('Page changed to ${value + 1}'); },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CHANGELOG.mdlib/shadcn_ui.dartlib/src/components/pagination.dartlib/src/theme/components/pagination.dartlib/src/theme/components/pagination.g.theme.dartlib/src/theme/data.dartlib/src/theme/themes/base.dartlib/src/theme/themes/default_theme_no_secondary_border_variant.dartlib/src/theme/themes/default_theme_variant.dartplayground/.gitignoreplayground/.metadataplayground/lib/pages/pagination.dartplayground/lib/router.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-13T09:57:53.516Z
Learnt from: nank1ro
Repo: nank1ro/flutter-shadcn-ui PR: 352
File: docs/src/content/docs/index.md:129-135
Timestamp: 2025-05-13T09:57:53.516Z
Learning: The shadcn_ui package uses `WidgetStatePropertyAll` in theme configurations (such as `ScrollbarThemeData`) instead of Flutter's native `MaterialStatePropertyAll`. This is intentional and correct according to the package maintainer.
Applied to files:
lib/src/theme/themes/default_theme_no_secondary_border_variant.dartlib/src/theme/themes/base.dartlib/src/theme/data.dartlib/src/theme/components/pagination.dartlib/src/theme/themes/default_theme_variant.dartlib/src/theme/components/pagination.g.theme.dartlib/src/components/pagination.dart
🔇 Additional comments (10)
lib/src/theme/data.dart (3)
23-23: LGTM!The import is correctly placed in alphabetical order among the component imports.
113-113: LGTM!The pagination theme integration follows the established pattern: accepting an optional
paginationThemeparameter in the factory constructor and merging it with the variant's default viaeffectiveVariant.paginationTheme().merge(paginationTheme). This is consistent with all other theme components in this file.Also applies to: 245-248
307-307: LGTM!The internal constructor correctly propagates
shadPaginationThemeas a required super parameter.lib/src/theme/themes/base.dart (3)
22-22: LGTM!Import correctly placed in alphabetical order.
99-99: LGTM!The
shadPaginationThemefield and constructor parameter are correctly integrated intoShadBaseTheme, following the established pattern for theme components.Also applies to: 153-153
210-210: LGTM!The
paginationTheme()abstract method is correctly added toShadThemeVariant, enabling theme variants to provide their pagination theme configuration.playground/lib/pages/pagination.dart (1)
20-27: LGTM!The controller lifecycle is properly managed—initialized as a field and disposed in
dispose()with the correct call order (_controller.dispose()beforesuper.dispose()).lib/src/theme/components/pagination.g.theme.dart (1)
1-399: Generated file - no review required.This is auto-generated code from
theme_extensions_builder_annotation. The implementation correctly handles lerp, copyWith, merge, equality, and hashCode for all theme properties.lib/src/theme/components/pagination.dart (2)
10-53: LGTM!The
ShadPaginationThemeclass follows the established pattern for theme components in this package:
- Uses
@themeGenand@immutableannotations- Mixes in the generated
_$ShadPaginationTheme- Implements the
canMergepattern with private backing field- Provides sensible defaults for non-nullable properties
The comprehensive set of styling properties enables flexible customization of the pagination component.
175-179: LGTM!The static
lerpmethod correctly delegates to the generated mixin implementation, following the established pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/src/components/pagination.dart (2)
501-509: Consider using a Set to track added pages.The loop uses
pages.contains(i), which is O(n) per iteration, making this section O(n²). For typical pagination with small page counts, the performance impact is negligible. However, for better algorithmic complexity, consider tracking added page numbers in aSet<int>and checking membership there.🔎 Potential optimization
List<int?> _generatePageNumbers() { if (widget.totalPages <= 0) return []; final currentPage = controller.selectedIndex + 1; // Convert to 1-based final totalPages = widget.totalPages; final siblingCount = widget.siblingCount; final boundaryCount = widget.boundaryCount; final pages = <int?>[]; + final addedPages = <int>{}; // For small number of pages, show all if (totalPages <= (boundaryCount * 2 + siblingCount * 2 + 3)) { for (var i = 1; i <= totalPages; i++) { pages.add(i); } return pages; } // Always show first boundary pages for (var i = 1; i <= min(boundaryCount, totalPages); i++) { pages.add(i); + addedPages.add(i); } // ... [middle sections would also update addedPages] // Always show last boundary pages for ( var i = max(totalPages - boundaryCount + 1, boundaryCount + 1); i <= totalPages; i++ ) { - if (i > boundaryCount && !pages.contains(i)) { + if (i > boundaryCount && !addedPages.contains(i)) { pages.add(i); + addedPages.add(i); } } return pages; }
654-656: Consider using dart:math for min/max functions.The custom
minandmaximplementations are correct, butdart:mathprovides built-in versions that would reduce code and improve maintainability.🔎 Proposed refactor
Add the import at the top of the file:
import 'package:flutter/material.dart'; import 'package:shadcn_ui/shadcn_ui.dart'; +import 'dart:math';Remove the custom implementations:
- T min<T extends num>(T a, T b) => a < b ? a : b; - T max<T extends num>(T a, T b) => a > b ? a : b;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/src/components/pagination.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-13T09:57:53.516Z
Learnt from: nank1ro
Repo: nank1ro/flutter-shadcn-ui PR: 352
File: docs/src/content/docs/index.md:129-135
Timestamp: 2025-05-13T09:57:53.516Z
Learning: The shadcn_ui package uses `WidgetStatePropertyAll` in theme configurations (such as `ScrollbarThemeData`) instead of Flutter's native `MaterialStatePropertyAll`. This is intentional and correct according to the package maintainer.
Applied to files:
lib/src/components/pagination.dart
🔇 Additional comments (3)
lib/src/components/pagination.dart (3)
83-98: Parameter validation implemented correctly.The assertions added here address the previous review comment requesting validation for pagination configuration parameters. All critical parameters (
totalPages,initialPage,siblingCount,boundaryCount,buttonHeight,compactBreakpoint) are now validated with clear error messages.
272-284: Controller initialization logic correctly addresses past feedback.The initialization now respects a provided controller's state while still supporting
initialPagewhen the controller is at its default. The logic only appliesinitialPagewhen the provided controller'sselectedIndexis 0 andinitialPageis non-zero, avoiding silent overwrites of explicit user state.
4-656: Excellent implementation of the pagination component.The
ShadPaginationwidget is well-architected with:
- Proper resource management (controller lifecycle)
- Comprehensive theming integration
- Responsive design with compact mode support
- Accessible UI (disabled states, tooltips)
- Flexible page number generation with ellipsis logic
- Extensive customization options
The implementation addresses all previously raised concerns and follows the package's established patterns.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
test/src/components/pagination.dart (1)
327-365: Consider resetting test view dimensions after modification.Tests at lines 327-344 and 346-364 modify
tester.view.physicalSizebut don't reset it afterward. While Flutter test framework typically handles cleanup, explicitly resetting the view ensures test isolation and prevents potential interference with subsequent tests.🔎 Suggested approach to reset view
Add an
addTearDowncall or reset the view at the end of each test:testWidgets('switches to compact mode below breakpoint', (tester) async { // Set screen width below default breakpoint (768) tester.view.physicalSize = const Size(500, 800); tester.view.devicePixelRatio = 1.0; + addTearDown(() { + tester.view.resetPhysicalSize(); + tester.view.resetDevicePixelRatio(); + }); await tester.pumpWidget( const MaterialApp( home: Scaffold( body: ShadPagination( totalPages: 10, ), ), ), ); // Should be in compact mode due to screen width expect(find.byType(ShadButton), findsAtLeast(3)); });Apply similar changes to the test at lines 346-364.
lib/src/components/pagination.dart (3)
278-283: Clarify the initialPage vs. external controller precedence.The logic attempts to apply
initialPageonly when the provided controller is at the default value (0), but this creates ambiguous behavior:
- If a user provides a controller at index 0 and wants to use
initialPage=0, the condition passes but nothing happens (correct by coincidence).- If a user provides a controller at index 3 and also passes
initialPage=5, theinitialPageis silently ignored (potentially surprising).- Index 0 is a valid page selection, so treating it as "uninitialized" is assumption-based.
Consider one of these clearer approaches:
- Always respect external controller (recommended): Document that
initialPageonly applies when no controller is provided.- Always apply initialPage: Document that
initialPageoverrides the controller's initial state.- Add an assertion: Fail fast if both a non-null controller and a non-zero
initialPageare provided together.🔎 Option 1 (recommended): Only use initialPage with internal controller
@override void initState() { super.initState(); if (widget.controller == null) { _controller = ShadPaginationController(); _controller.selectedIndex = widget.initialPage; - } else { - // Respect the provided controller's state, but apply initialPage - // if it's at default - if (widget.controller!.selectedIndex == 0 && widget.initialPage != 0) { - widget.controller!.selectedIndex = widget.initialPage; - } } + // When an external controller is provided, respect its state entirely + // and ignore initialPage }Update the
initialPagedocumentation to clarify:/// The initial page index. /// Only used when [controller] is null. If you provide an external /// controller, set its selectedIndex before passing it to ShadPagination. final int initialPage;🔎 Option 3: Fail fast with assertion
Add an assertion in the constructor:
const ShadPagination({ // ... parameters }) : assert(totalPages > 0, 'totalPages must be greater than 0'), // ... other assertions + assert( + controller == null || initialPage == 0, + 'Cannot specify both controller and non-zero initialPage. ' + 'Either provide a controller with pre-set selectedIndex, ' + 'or use initialPage without a controller.', + ), assert(boundaryCount >= 0, 'boundaryCount must be non-negative');
500-509: Consider simplifying the boundary page logic to avoid potential duplicates.The loop at lines 501-509 adds the last boundary pages, but it uses
pages.contains(i)to avoid duplicates. This check is O(n) for each iteration and suggests the logic might add pages that were already included. Using a Set or clearer range logic would be more efficient and clearer.Additionally, the loop range
max(totalPages - boundaryCount + 1, boundaryCount + 1)is complex and could have edge cases that are hard to reason about.🔎 Alternative approach using Set for deduplication
List<int?> _generatePageNumbers() { if (widget.totalPages <= 0) return []; final currentPage = controller.selectedIndex + 1; // Convert to 1-based final totalPages = widget.totalPages; final siblingCount = widget.siblingCount; final boundaryCount = widget.boundaryCount; + final pageSet = <int>{}; final pages = <int?>[]; // For small number of pages, show all if (totalPages <= (boundaryCount * 2 + siblingCount * 2 + 3)) { for (var i = 1; i <= totalPages; i++) { pages.add(i); } return pages; } // Collect all pages that should be shown (without ellipsis) // Add first boundary pages for (var i = 1; i <= min(boundaryCount, totalPages); i++) { - pages.add(i); + pageSet.add(i); } // Add current page and siblings final start = max(currentPage - siblingCount, 1); final end = min(currentPage + siblingCount, totalPages); for (var i = start; i <= end; i++) { - if (i > boundaryCount && i <= totalPages - boundaryCount) { - pages.add(i); - } + pageSet.add(i); } // Add last boundary pages for (var i = max(totalPages - boundaryCount + 1, 1); i <= totalPages; i++) { - if (i > boundaryCount && !pages.contains(i)) { - pages.add(i); - } + pageSet.add(i); } + // Convert set to list with ellipses + final sortedPages = pageSet.toList()..sort(); + for (var i = 0; i < sortedPages.length; i++) { + if (i > 0 && sortedPages[i] - sortedPages[i - 1] > 1 && widget.showEllipsis) { + pages.add(null); // Add ellipsis + } + pages.add(sortedPages[i]); + } - // Add ellipsis logic... return pages; }This approach is clearer: collect all pages in a Set (which handles duplicates automatically), sort them, then insert ellipses where there are gaps.
634-639: Consider disabling the selected page button.Lines 634-639 make the button's
onPressedcallback checkif (!isSelected)before updating, but the button itself is always pressable. This means users can tap the currently selected page, and nothing happens—which might feel unresponsive or buggy.Consider setting
onPressed: nullfor selected pages to visually indicate they're not actionable.🔎 Proposed change
return ShadButton.raw( variant: effectiveVariant, size: widget.buttonSize ?? ShadButtonSize.sm, padding: widget.buttonPadding ?? const EdgeInsets.symmetric(horizontal: 12, vertical: 6), backgroundColor: effectiveBackgroundColor, - onPressed: () { - if (!isSelected) { - controller.selectedIndex = page - 1; - widget.onPageChanged?.call(controller.selectedIndex); - } - }, + onPressed: isSelected + ? null + : () { + controller.selectedIndex = page - 1; + widget.onPageChanged?.call(controller.selectedIndex); + }, child: Container( height: widget.buttonHeight, alignment: Alignment.center, child: Text( page.toString(), style: TextStyle( fontSize: 14, color: isSelected ? effectiveForegroundColor : null, ), ), ), );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
test/src/components/failures/input_masterImage.pngis excluded by!**/*.pngtest/src/components/failures/input_testImage.pngis excluded by!**/*.png
📒 Files selected for processing (2)
lib/src/components/pagination.darttest/src/components/pagination.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-13T09:57:53.516Z
Learnt from: nank1ro
Repo: nank1ro/flutter-shadcn-ui PR: 352
File: docs/src/content/docs/index.md:129-135
Timestamp: 2025-05-13T09:57:53.516Z
Learning: The shadcn_ui package uses `WidgetStatePropertyAll` in theme configurations (such as `ScrollbarThemeData`) instead of Flutter's native `MaterialStatePropertyAll`. This is intentional and correct according to the package maintainer.
Applied to files:
lib/src/components/pagination.dart
🔇 Additional comments (1)
lib/src/components/pagination.dart (1)
326-327: No issues found. TheEdgeInsetsGeometry.horizontalproperty is a standard Flutter API and works as expected. No changes needed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
lib/src/components/pagination.dart
Outdated
| @@ -0,0 +1,707 @@ | |||
| import 'package:flutter/material.dart'; | |||
There was a problem hiding this comment.
Do not import material, tests will fail.
lib/src/components/pagination.dart
Outdated
| @@ -0,0 +1,707 @@ | |||
| import 'package:flutter/material.dart'; | |||
| import 'package:shadcn_ui/shadcn_ui.dart'; | |||
There was a problem hiding this comment.
Do not import the barrel file, it is meant only for library users.
Import each component individually.
| this.initialPage = 0, | ||
| this.padding, | ||
| this.border, | ||
| this.backgroundColor, | ||
| this.radius, | ||
| this.onPageChanged, | ||
| this.showEllipsis = true, | ||
| this.showFirstLastButtons = false, | ||
| this.isCompact = false, | ||
| this.showPageNumbers = true, | ||
| this.siblingCount = 1, | ||
| this.boundaryCount = 1, | ||
| this.buttonVariant, | ||
| this.buttonSize, | ||
| this.buttonPadding, | ||
| this.selectedButtonVariant, | ||
| this.selectedButtonBackgroundColor, | ||
| this.buttonGap = 4.0, | ||
| this.buttonHeight = 40.0, | ||
| this.compactBreakpoint = 768.0, | ||
| this.compactButtonVariant = ShadButtonVariant.ghost, | ||
| this.ellipsisColor, | ||
| this.navigationButtonSize, | ||
| this.navigationButtonVariant, | ||
| this.selectedButtonForegroundColor, | ||
| this.width, | ||
| this.margin = const EdgeInsets.symmetric(horizontal: 16), | ||
| this.previousButtonLabel = 'Previous', | ||
| this.nextButtonLabel = 'Next', |
There was a problem hiding this comment.
Do not pass default values here, follow the contributing guide.
lib/src/components/pagination.dart
Outdated
| const EdgeInsets.all(4); | ||
|
|
||
| final effectiveMargin = | ||
| widget.margin ?? const EdgeInsets.symmetric(horizontal: 16); |
| theme.shadPaginationTheme.compactBreakpoint; | ||
|
|
||
| final isCompactBasedOnBreakpoint = | ||
| widget.isCompact || screenWidth < effectiveCompactBreakpoint; |
| @@ -0,0 +1,52 @@ | |||
| import 'package:flutter/material.dart'; | |||
There was a problem hiding this comment.
Please add a pagination example in the example folder too.
Co-authored-by: Alexandru Mariuti <alex@mariuti.com>
Co-authored-by: Alexandru Mariuti <alex@mariuti.com>
Co-authored-by: Alexandru Mariuti <alex@mariuti.com>
Co-authored-by: Alexandru Mariuti <alex@mariuti.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @lib/src/theme/components/pagination.dart:
- Around line 13-54: Constructor ShadPaginationTheme allows invalid numeric
values that break layouts; add constructor asserts in ShadPaginationTheme to
enforce siblingCount >= 0, boundaryCount >= 0, compactBreakpoint > 0, and
(buttonHeight == null || buttonHeight > 0) and (buttonGap == null || buttonGap
>= 0) so invalid theme states are rejected early; update the
ShadPaginationTheme(...) initializer to include these assert(...) checks
referencing the constructor and the fields siblingCount, boundaryCount,
compactBreakpoint, buttonHeight, and buttonGap.
In @test/src/components/pagination.dart:
- Around line 104-119: The test 'respects initialPage' only asserts that a
button with label '3' exists but doesn't verify it's the selected page; update
the test for ShadPagination to assert selection by either supplying an
onPageChanged callback to capture the active page (use initialPage to assert the
initial selected index) or by querying the actual pagination button widget/state
(e.g., checking a selection flag or selected styling for the widget found by
find.text('3')); modify the test to set onPageChanged or inspect the button
widget to confirm the initially selected page is the one corresponding to
initialPage.
- Around line 251-267: Add a complementary test to verify the next button is
disabled on the last page by instantiating ShadPagination with initialPage: 4
(zero-based) and asserting the ShadButton that contains Icons.chevron_right has
a null onPressed; avoid the fragile .first on find.byIcon by using a more
specific finder such as find.widgetWithIcon(ShadButton, Icons.chevron_right) (or
.last if appropriate) so the test targets the correct button.
- Around line 289-304: The test "boundaryCount controls boundary pages" only
asserts start boundaries; update the test in testWidgets to also assert the end
boundaries for ShadPagination with totalPages: 10 and boundaryCount: 2 by adding
expectations that the labels '9' and '10' are present (e.g.,
expect(find.text('9'), findsOneWidget) and expect(find.text('10'),
findsOneWidget)) so both start and end boundary pages are verified.
- Around line 327-364: Both responsive tests change
tester.view.physicalSize/devicePixelRatio and never reset them, which can
pollute other tests; add a tearDown (or addTearDown) in these test cases (or
globally for the file) to restore the original tester.view.physicalSize and
tester.view.devicePixelRatio after each test, specifically around the
testWidgets named 'switches to compact mode below breakpoint' and 'respects
custom compactBreakpoint' that exercise ShadPagination so subsequent tests run
with the default screen dimensions.
- Around line 234-249: Fix the missing space in the inline comment and replace
the weak assertion in the testWidgets('compact mode shows fewer buttons') for
ShadPagination: ensure the comment reads "// should be visible", and strengthen
the test by asserting exact compact-mode behavior — e.g., assert
find.byType(ShadButton) returns the expected exact count for compact mode (prev,
current, next) and also assert that distant page-number buttons are not present
(e.g., expect(find.text('10'), findsNothing) or expect(find.text('1'),
findsNothing)) or that an ellipsis indicator (Text('...') or appropriate widget)
is present to prove truncation rather than using findsAtLeast(3).
- Around line 269-287: The test 'siblingCount controls visible siblings' for
ShadPagination is missing an assertion for page 8; update the test to also
assert that the text '8' is visible given initialPage: 5, siblingCount: 2 (and
default boundaryCount: 1) by adding an expectation for find.text('8') to be
found (e.g., expect(find.text('8'), findsOneWidget)); keep the rest of the setup
unchanged.
🧹 Nitpick comments (2)
example/lib/pages/pagination.dart (1)
1-15: Consider showcasingonPageChanged/ controller in the example page (currently static).
ShadPagination(totalPages: 10)is fine for a minimal demo, but the example page would be more helpful if it also demonstrated handling page changes (and optionally controller-driven updates).test/src/components/pagination.dart (1)
197-211: Consider using semantic identifiers instead of specific icons.The test verifies first/last buttons by checking for
Icons.chevron_leftandIcons.chevron_right, which couples the test to implementation details. If icon choices change, the test will break even if functionality remains correct.🔧 Alternative approach
Consider using semantic labels or keys for navigation buttons:
// In production code: ShadButton( key: const Key('pagination_previous'), // ... ) // In test: expect(find.byKey(const Key('pagination_previous')), findsOneWidget); expect(find.byKey(const Key('pagination_next')), findsOneWidget);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
example/lib/pages/pagination.dartlib/src/components/pagination.dartlib/src/theme/components/pagination.darttest/src/components/pagination.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/src/components/pagination.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-13T09:57:53.516Z
Learnt from: nank1ro
Repo: nank1ro/flutter-shadcn-ui PR: 352
File: docs/src/content/docs/index.md:129-135
Timestamp: 2025-05-13T09:57:53.516Z
Learning: The shadcn_ui package uses `WidgetStatePropertyAll` in theme configurations (such as `ScrollbarThemeData`) instead of Flutter's native `MaterialStatePropertyAll`. This is intentional and correct according to the package maintainer.
Applied to files:
lib/src/theme/components/pagination.dart
🔇 Additional comments (11)
lib/src/theme/components/pagination.dart (2)
179-183:lerpdelegation to the generated implementation looks good.
8-12: No issues found. The codegen is properly set up with the@themeGenannotation andpart 'pagination.g.theme.dart'directive, the generated mixin structure is correct, and all dartdoc macros likeShadPagination.radiusandShadPagination.paddingare properly defined with matching@template/@endtemplatepairs.test/src/components/pagination.dart (9)
6-85: LGTM! Comprehensive controller test coverage.The controller tests thoroughly verify initialization, state updates, listener notifications, and all navigation methods with proper boundary handling.
88-102: LGTM!Basic rendering test correctly verifies all page numbers are displayed.
121-142: LGTM!Correctly verifies the
onPageChangedcallback with proper 0-based index conversion.
144-165: LGTM!Correctly verifies external controller integration.
167-195: LGTM!Ellipsis visibility tests are well-structured.
213-232: LGTM!Correctly verifies that
showPageNumbers: falsehides all page number text.
306-323: LGTM! Good assertion error coverage.Properly verifies that invalid
totalPagesandinitialPagevalues throw assertion errors.
367-412: LGTM! Excellent edge case coverage.The tests properly handle single page, very large page counts, and dynamic updates. The
_TestPageUpdaterhelper widget is a clean approach to testing dynamic behavior.
415-437: LGTM! Clean helper widget.The
_TestPageUpdaterwidget is well-designed for testing dynamic state changes.
2 participants
This PR worked on #237. I have implemented the pagination widget and followed the contribution guidelines
Issues worked on: #237
Screenshot of the implementation
Testing this PR
To try this branch, add the following to your
pubspec.yaml:Pre-launch Checklist
///).0.18.0and you introduced a breaking change or a new feature, bump it to0.19.0, if you just added a fix or a chore bump it to0.18.1.CHANGELOG.mdfile with a summary of changes made following the format already used.If you need help, consider asking for advice on Discord.
Summary by CodeRabbit
New Features
Theme
Examples
Tests
Chores