feat: filter offers by maker's account age in order book#581
Conversation
- New Days of use filter in the order book filter dialog, reading the days field from the rating tag of kind 38383 events. - Slider 0..20 with the same gray palette as the reputation and premium filters, adjacent text input accepts higher values (clamped to 0..9999). - Right-side label switches from Days: 20 to the typed value when the user enters a higher number. - New minDaysFilterProvider in home_order_providers.dart; offers are excluded when rating.days < minDays. - Localization keys daysOfUse and days added to en, es, it, de and fr.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a "days of use" minimum filter: a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OrderFilterUI as "OrderFilter UI"
participant StateProv as "minDaysFilterProvider"
participant FilterProv as "filteredOrdersProvider"
participant Orders as "Orders Source"
User->>OrderFilterUI: adjust slider / enter number
OrderFilterUI->>StateProv: update minDays value
StateProv-->>FilterProv: notify change
FilterProv->>Orders: read all orders
FilterProv-->>FilterProv: apply existing filters + minDays constraint
FilterProv-->>OrderFilterUI: emit filtered orders
OrderFilterUI-->>User: render updated order list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (3)
lib/l10n/intl_it.arb (1)
1075-1076: Same duplication concern as inintl_es.arb.
"daysLabel": "Giorni"already exists at Line 738. The new"days": "Giorni"duplicates it. Consider consolidating to a single key to reduce translation maintenance across locales.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/l10n/intl_it.arb` around lines 1075 - 1076, The ARB file contains duplicate Italian keys for the same label ("days" and existing "daysLabel"); remove the redundant "days": "Giorni" entry and update any code that references "days" to use the canonical "daysLabel" key (or vice versa if you prefer a different canonical name), ensuring only one key ("daysLabel") remains to avoid duplicate translations and simplify maintenance.lib/shared/widgets/order_filter.dart (1)
774-779: Empty text input snapsminDaysto 0 mid-edit.When the user clears the TextField to retype a value,
int.tryParse('')returnsnullandminDaysis immediately set to 0, which also snaps the slider visually. Consider leavingminDaysunchanged while the field is empty (only commit on non-empty parse), or debounce the update, to avoid flicker during editing.🔧 Suggested tweak
onChanged: (text) { - final parsed = int.tryParse(text); - setState(() { - minDays = parsed == null ? 0 : parsed.clamp(0, 9999); - }); + if (text.isEmpty) return; + final parsed = int.tryParse(text); + if (parsed == null) return; + setState(() { + minDays = parsed.clamp(0, 9999); + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/shared/widgets/order_filter.dart` around lines 774 - 779, The onChanged handler in OrderFilter snaps minDays to 0 when the field is emptied because int.tryParse('') returns null; change the handler so it only calls setState to update minDays when the input is non-empty and successfully parses (i.e., if text.isNotEmpty and parsed != null then set minDays = parsed.clamp(0,9999)), otherwise leave minDays unchanged during editing (or implement a short debounce before committing), ensuring the slider isn't forced to 0 mid-edit; update the onChanged closure that currently uses int.tryParse and setState to perform this conditional update.lib/l10n/intl_es.arb (1)
1016-1017: Consider reusing the existingdaysLabelkey for "Días".The file already defines
"daysLabel": "Días"at Line 672 (used in trade detail screens). The new"days": "Días"duplicates the same string/concept. Consider either reusingdaysLabelin the new filter UI or consolidating both call sites to a single key to avoid translation drift across locales.As per learnings: "In Flutter ARB localization files under lib/l10n, include metadata blocks (prefixed with @) only for keys that have placeholders." — compliance here is fine since neither new key has placeholders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/l10n/intl_es.arb` around lines 1016 - 1017, Duplicate Spanish string: the new "days" key duplicates the existing "daysLabel" key; remove the redundant "days" entry and update the new filter UI to use the existing "daysLabel" key (or consolidate both call sites to "daysLabel") so translations stay centralized; ensure any code or ARB references that currently use "days" are changed to "daysLabel" and delete the unused "days" key from the ARB.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/shared/widgets/order_filter.dart`:
- Around line 670-694: The right-side label currently mixes endpoint and
current-value semantics by showing "${S.of(context)!.days}: ${minDays > 20 ?
minDays : 20}" while the left label is hardcoded "Days: 0", which confuses
users; update the Row in order_filter.dart so both labels consistently represent
either slider endpoints (e.g., left "0" and right "20") or remove the right
label entirely and rely on the Slider's built-in label (Slider.label using
minDays.toString()) plus the numeric TextField to show the current value; also
replace the hardcoded "0" text with a localized/programmable number (e.g.,
0.toString() or a formatted/localized number via S.of(context) or NumberFormat)
so digits are localizable.
---
Nitpick comments:
In `@lib/l10n/intl_es.arb`:
- Around line 1016-1017: Duplicate Spanish string: the new "days" key duplicates
the existing "daysLabel" key; remove the redundant "days" entry and update the
new filter UI to use the existing "daysLabel" key (or consolidate both call
sites to "daysLabel") so translations stay centralized; ensure any code or ARB
references that currently use "days" are changed to "daysLabel" and delete the
unused "days" key from the ARB.
In `@lib/l10n/intl_it.arb`:
- Around line 1075-1076: The ARB file contains duplicate Italian keys for the
same label ("days" and existing "daysLabel"); remove the redundant "days":
"Giorni" entry and update any code that references "days" to use the canonical
"daysLabel" key (or vice versa if you prefer a different canonical name),
ensuring only one key ("daysLabel") remains to avoid duplicate translations and
simplify maintenance.
In `@lib/shared/widgets/order_filter.dart`:
- Around line 774-779: The onChanged handler in OrderFilter snaps minDays to 0
when the field is emptied because int.tryParse('') returns null; change the
handler so it only calls setState to update minDays when the input is non-empty
and successfully parses (i.e., if text.isNotEmpty and parsed != null then set
minDays = parsed.clamp(0,9999)), otherwise leave minDays unchanged during
editing (or implement a short debounce before committing), ensuring the slider
isn't forced to 0 mid-edit; update the onChanged closure that currently uses
int.tryParse and setState to perform this conditional update.
🪄 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: CHILL
Plan: Pro
Run ID: f9c6d277-8a76-44b2-b754-08c8e44c2000
📒 Files selected for processing (7)
lib/features/home/providers/home_order_providers.dartlib/l10n/intl_de.arblib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_fr.arblib/l10n/intl_it.arblib/shared/widgets/order_filter.dart
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Request Changes
1. Right-side label is hardcoded to 20 — bug in order_filter.dart
In the days filter slider, the right-side label uses:
Text("${S.of(context)!.days}: ${minDays > 20 ? minDays : 20}")This means:
minDays = 0→ shows"Days: 20"minDays = 5→ shows"Days: 20"minDays = 25→ shows"Days: 25"
When the user slides to any value ≤ 20, the right label still displays 20 instead of the current slider value. This is confusing — the label should reflect the actual selected value, not the slider maximum.
Fix: Change to ${minDays > 20 ? minDays : minDays} or simply show the current value consistently. If the intent was to show the slider max, use a different label like "Max: 20" instead of reusing "Days".
2. Text input does not reset on invalid/empty input
In the TextField.onChanged callback:
onChanged: (text) {
final parsed = int.tryParse(text);
setState(() {
minDays = parsed == null ? 0 : parsed.clamp(0, 9999);
});
}Two problems:
a. If the user types non-numeric text (e.g. "abc"), parsed is null, minDays resets to 0, but _daysController.text still shows "abc". The slider jumps to 0 while the text field displays garbage.
Fix: Update _daysController.text after clamping so the field always reflects the parsed value.
b. If the user clears the field (""), int.tryParse("") returns null, minDays becomes 0, but the text field stays empty. The slider label shows "0" while the input is blank.
Fix: Normalize empty input to "0" in the controller.
Suggested fix for both:
onChanged: (text) {
final parsed = int.tryParse(text);
final clamped = parsed == null ? 0 : parsed.clamp(0, 9999);
setState(() {
minDays = clamped;
});
if (_daysController.text != clamped.toString()) {
_daysController.text = clamped.toString();
_daysController.selection = TextSelection.collapsed(
offset: _daysController.text.length,
);
}
}3. UX concern — slider label vs. text input value mismatch
When the user types 100 in the text field, the slider thumb stays at 20 (clamped) but the slider label shows "100" because label: minDays.toString(). This is technically correct but visually jarring — the thumb is at the far right and the label says 100.
Not blocking, but consider capping the slider label to 20+ or showing a visual indicator that the value exceeds the slider range.
✅ What is good
- The filtering logic in
filteredOrdersProvideris correct:(o.rating?.days ?? 0) >= minDays - Null-safe handling of missing rating tags (falls back to
0days, which correctly excludes the offer when a minimum is set) _daysController.dispose()is called — no memory leak- Filter reset (Clear button) correctly resets both state and controller
- Translations are complete across all 5 locales
- Regex input formatter prevents non-numeric input beyond 4 digits
Summary
| Item | Status |
|---|---|
| Filtering logic | ✅ Correct |
| Slider 0–20 range | ✅ Correct |
| Text input 0–9999 clamp | ✅ Correct |
| Translations (5 locales) | ✅ Complete |
| Controller disposal | ✅ Present |
| Right-side label | 🔴 Bug — hardcoded to 20 |
| Invalid input sync | 🔴 Bug — controller out of sync |
| Slider/value mismatch |
Required actions:
- Fix the right-side label to show the actual
minDaysvalue, not hardcoded20. - Sync
_daysController.textafter parsing/clamping inonChanged.
2a. Non-numeric input can't happen. The FilteringTextInputFormatter.allow(RegExp(r'^\d{0,4}$')) rejects non-digits before onChanged is called, so parsed == null from text like "abc" isn't a reachable state. 2b. Empty field on clear — intentional. Forcing "0" into the controller mid-edit would interrupt natural editing (e.g., clearing "5" to type "30"). The state is internally consistent: empty field + slider at 0 + left label "Days: 0". Applying with empty field = minDays = 0 = no filter, which is the expected behavior.
|
There was a problem hiding this comment.
I re-reviewed this PR taking into account the previous discussion and Catrya's responses. The earlier request-changes point about the right-side days label does not justify blocking this PR. The current behavior is intentional: the right label acts as a saturation cue for values typed above the slider range, while the current selected value is already surfaced by the slider indicator and the text field. The digit-localization point is also correctly scoped as out of range for this PR.
On the implementation itself, the change is coherent and appropriately small:
minDaysFilterProvideris introduced cleanly and applied infilteredOrdersProviderwithout interfering with the existing currency, payment method, rating, or premium filters.- The filtering logic is straightforward: offers are excluded when
rating.days < minDays, and missing ratings naturally behave as0, which is reasonable for a minimum-account-age filter. - The UI keeps the same visual language as the other filters and correctly synchronizes slider, text field, clear, and apply behaviors.
- The clamp boundaries are sensible: slider 0..20, typed input 0..9999.
- Localization coverage is included for the affected languages.
CI is passing and I do not see correctness issues that should block merge. Approved.
1 participant
fix #574
Summary by CodeRabbit
New Features
Localization