Feat #168: implement search query field with generic type refactor#173
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughBottomPicker and its state class are converted to generic types parameterized by item type T, adding search/filter UI support via new fields (itemBuilder, filterPredicate, textInputAction, searchFieldDecoration) and typed callback dispatch. SimplePicker disables the CupertinoPicker magnifier. The example app is updated to use a new CountryModel dataset with StatefulWidget conversions and typed picker calls, and the SDK constraint is raised. ChangesGeneric typed BottomPicker with search filtering
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BottomPickerState
participant SimplePicker
User->>BottomPickerState: type text into search TextField
BottomPickerState->>BottomPickerState: filterPredicate(item, text) filters originalItemList
BottomPickerState->>SimplePicker: build rows from displayedItemList via itemBuilder
User->>SimplePicker: select item
SimplePicker->>BottomPickerState: onChange updates selectedItem
User->>BottomPickerState: submit or dismiss
BottomPickerState->>BottomPickerState: onSubmit/onDismiss dispatched with typed selectedItem as T
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
example/lib/main.dart (1)
245-292: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading parameter name in
onChangecallback.
onChange: (int index) { print(index); }names the parameterindex, but per the genericBottomPicker<T>contract the callback actually receives the selected item value, not its position in the list. It only looks correct here becausehugeList[i] == i. Naming itvalue/itemwould avoid confusing readers about the new generic callback semantics this PR introduces.✏️ Suggested rename
- onChange: (int index) { - print(index); + onChange: (int value) { + print(value); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/lib/main.dart` around lines 245 - 292, The `onChange` callback in `_displayLongListPicker` uses a misleading parameter name: it receives the selected item value from `BottomPicker<int>`, not the item’s position. Rename the callback parameter from `index` to something like `item` or `value` in this method so it matches the generic `BottomPicker<T>` contract and avoids confusion with the separate list index used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@example/lib/main.dart`:
- Around line 15-49: The ExampleApp state still carries dead state in the
_ExampleAppState class: the displayedList field is initialized from hugeList but
is never read or updated now that search/filtering is handled inside
BottomPicker and BottomPickerState. Remove the unused displayedList member from
_ExampleAppState and keep the state focused on the active fields such as
buttonWidth and hugeList.
In `@lib/bottom_picker.dart`:
- Around line 793-797: The dismiss callback in dispose() is still casting
timer/dateTime values to T, which can break BottomPicker<String>.timer(...) and
.dateTime(...) at runtime. Update the BottomPicker API so non-simple modes
cannot use an incompatible generic type: either make the timer and dateTime
constructors non-generic, or split the callback/value types so dispose() calls
onDismiss with the correct concrete type without any cast. Use the
BottomPickerType branching in bottom_picker.dart to locate the fix.
- Line 916: The non-simple picker handlers are no longer emitting updates
because the widget.onChange callback is commented out, while the API still
implies DateTime/int change notifications. Restore the typed onChange calls in
the timer, time, dateTime, and year picker flow where widget.onChange is used,
or if this behavior is intentional, update the widget.onChange contract and
related docs/comments to match the new behavior. Use the existing
widget.onChange references in BottomPicker to locate and re-enable the correct
callback emissions.
- Line 901: The initial picker index passed from SimplePicker is using
widget.selectedItemIndex directly even when displayedItemList has been filtered
and may be shorter. Update the SimplePicker initialization logic to derive the
index from the filtered list, using the matching item’s position or 0 when there
are no matches, so FixedExtentScrollController.initialItem always stays within
range and picker setup works for empty-filter cases.
- Around line 756-760: Initialize the timer state in initState for the
BottomPickerType.timer path so selectedTimerDuration is set from
widget.initialTimerDuration before any user interaction. Update the
BottomPickerState.initState logic alongside the existing simple and date/time
branches, and make sure dispose and the submit flow use this initialized value
instead of dispatching a null selectedTimerDuration when the wheel was never
touched.
- Around line 887-899: The picker row builder is re-deriving the original index
with originalItemList!.indexOf(item), which is both O(n) per row and incorrect
for duplicate values. Update the displayedItemList mapping in the bottom picker
widget so each row carries its original index alongside the item, and use that
preserved index in widget.itemBuilder and in onChange instead of searching
originalItemList. Keep the fix localized to the picker list-building logic and
the selectedItemIndex assignment in BottomPicker.
---
Outside diff comments:
In `@example/lib/main.dart`:
- Around line 245-292: The `onChange` callback in `_displayLongListPicker` uses
a misleading parameter name: it receives the selected item value from
`BottomPicker<int>`, not the item’s position. Rename the callback parameter from
`index` to something like `item` or `value` in this method so it matches the
generic `BottomPicker<T>` contract and avoids confusion with the separate list
index used elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: af242c7c-2b7d-4172-baf7-f616fd09be55
⛔ Files ignored due to path filters (1)
example/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
example/lib/country_data.dartexample/lib/main.dartexample/pubspec.yamllib/bottom_picker.dartlib/widgets/simple_picker.dart
koukibadr
force-pushed
the
feat#168-implement-search-query-field
branch
from
00ed027 to
1be9cd1
Compare
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores