-
Notifications
You must be signed in to change notification settings - Fork 17
feat: enhance walkthrough highlighting and localize payment methods #271
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
Conversation
- Add green highlighting for "create your own offer" text in walkthrough across all languages
- Fix Spanish walkthrough highlighting: "Facturas Retenidas" now displays correctly
- Localize payment method selection to display and send user's language to Mostro
- Add Spanish/Italian translations for "Cash in person" ("Efectivo"/"Contanti di persona") and "Other" ("Otro"/"Altro")
- Payment methods now sent to Mostro in user's selected language instead of English
WalkthroughLocalizes payment method labels and dialog logic in order screens; adds a "create offer" highlight step to the walkthrough with an accompanying multilingual pattern and adjusts a security highlight term; introduces new localization keys (cashInPerson, other) for English, Spanish, and Italian. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as PaymentMethodsSection
participant D as SelectionDialog
U->>P: Open payment methods dialog
P->>P: translate available methods via _translatePaymentMethod(...)
P->>D: show dialog with translated methods (including translated "Other")
D->>D: user toggles checkboxes (compare to translated "Other")
D->>P: submit selections (custom method sanitized & appended if present)
P-->>U: update displayed methods (localized labels)
sequenceDiagram
participant U as User
participant W as WalkthroughScreen
participant H as HighlightConfig
U->>W: View walkthrough page
W->>W: build text with step flags (privacy, security, chat, orderBook, createOffer)
W->>H: request matching pattern (includes createOffer)
H-->>W: pattern match result
W-->>U: render highlighted text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
lib/features/order/widgets/payment_methods_section.dart (1)
74-76: Selected methods are displayed without localization.
displayTextjoinsselectedMethodsdirectly, which may be English while the UI is localized. This results in mixed-language UI and is inconsistent with the localized options list.Apply this diff:
- final displayText = selectedMethods.isEmpty - ? S.of(context)!.selectPaymentMethods - : selectedMethods.join(', '); + final translatedSelected = selectedMethods + .map((m) => _translatePaymentMethod(m, context)) + .toList(); + final displayText = translatedSelected.isEmpty + ? S.of(context)!.selectPaymentMethods + : translatedSelected.join(', ');
🧹 Nitpick comments (1)
lib/features/order/widgets/payment_methods_section.dart (1)
15-27: Localizing payment method labels via a helper is fine; consider future-proofing away from raw English keys.Current switch-cases depend on English input strings, which is brittle if backend keys change or new methods are added. Consider canonical IDs (enum or backend keys) mapped to localization keys to avoid string matching.
I can sketch a small
PaymentMethodmodel (id + label builder) to centralize this and remove string switches if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
lib/features/order/screens/add_order_screen.dart(1 hunks)lib/features/order/widgets/payment_methods_section.dart(4 hunks)lib/features/walkthrough/screens/walkthrough_screen.dart(3 hunks)lib/features/walkthrough/utils/highlight_config.dart(2 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
When adding a localization key, add it to all three ARB files: intl_en.arb, intl_es.arb, and intl_it.arb
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
After modifying ARB files, regenerate localization outputs (e.g., dart run build_runner build -d or flutter gen-l10n)
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
lib/**/@(screens|widgets)/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods
Files:
lib/features/order/screens/add_order_screen.dartlib/features/order/widgets/payment_methods_section.dartlib/features/walkthrough/screens/walkthrough_screen.dart
lib/!(generated)/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible
Files:
lib/features/order/screens/add_order_screen.dartlib/features/order/widgets/payment_methods_section.dartlib/features/walkthrough/utils/highlight_config.dartlib/features/walkthrough/screens/walkthrough_screen.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/order/screens/add_order_screen.dartlib/features/order/widgets/payment_methods_section.dartlib/features/walkthrough/screens/walkthrough_screen.dart
🧠 Learnings (2)
📚 Learning: 2025-08-15T13:19:31.968Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-15T13:19:31.968Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : When adding a localization key, add it to all three ARB files: intl_en.arb, intl_es.arb, and intl_it.arb
Applied to files:
lib/l10n/intl_it.arblib/l10n/intl_es.arb
📚 Learning: 2025-08-15T13:19:31.968Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-15T13:19:31.968Z
Learning: Applies to lib/!(generated)/**/*.dart : Pass BuildContext to methods that need localization rather than accessing localization without context
Applied to files:
lib/features/order/widgets/payment_methods_section.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
lib/l10n/intl_en.arb (1)
278-280: Localization keys verified — please regenerate l10n outputs
- Confirmed that
cashInPersonandotherexist in all ARB files (intl_en.arb,intl_es.arb,intl_it.arb).- Please run your localization generation command (e.g.
flutter gen-l10nordart run build_runner build -d) locally to produce the updated S getters.lib/l10n/intl_it.arb (1)
239-241: Italian translations added correctly; consistent with EN/ES.
cashInPerson→ “Contanti di persona”,other→ “Altro”. Looks consistent with the PR goals and UI usage.lib/l10n/intl_es.arb (1)
230-232: Spanish translations added correctly; consistent with EN/IT.
cashInPerson→ “Efectivo”,other→ “Otro”. Matches the intended labels and will localize the payment-method UI properly.lib/features/walkthrough/utils/highlight_config.dart (2)
16-17: Spanish security term updated correctly for highlight matching.Switching to “Facturas Retenidas” matches current walkthrough text and should fix the highlight in ES.
28-30: createOffer highlight wired in WalkthroughScreen
Confirmed thatHighlightConfig.createOfferis referenced inlib/features/walkthrough/screens/walkthrough_screen.dart(lines 44–45) and its pattern is compiled viaRegExp(config.pattern, caseSensitive: true).lib/features/walkthrough/screens/walkthrough_screen.dart (3)
23-24: New create-offer step flag is correctly threaded.Adding
isCreateOfferStepto the builder signature looks good and keeps the API consistent with other steps.
43-45: Correct highlight config selection for the new step.Selecting
HighlightConfig.createOfferwhenisCreateOfferStepis true is exactly what's needed to drive the new green highlight.
155-156: Create-offer page uses localized string and new highlight flag — good.This wires the new step into the final page and ensures the green highlight applies to the “create your own offer” text.
lib/features/order/widgets/payment_methods_section.dart (3)
80-87: Translating available methods before rendering is correct.This ensures the chooser presents localized labels regardless of whether the source data is fiat-specific or defaulted.
130-133: Good: ensure localized “Other” is always offered.Appending the localized “Other” when missing keeps the UX consistent across currencies and data sources.
166-174: Toggling custom field visibility based on the localized “Other” is correct.Comparing against the localized sentinel avoids language-specific edge cases.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/features/order/screens/add_order_screen.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/@(screens|widgets)/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods
Files:
lib/features/order/screens/add_order_screen.dart
lib/!(generated)/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible
Files:
lib/features/order/screens/add_order_screen.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/order/screens/add_order_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-08-15T13:19:31.968Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-15T13:19:31.968Z
Learning: Applies to lib/!(generated)/**/*.dart : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Applied to files:
lib/features/order/screens/add_order_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
lib/features/order/screens/add_order_screen.dart (1)
299-303: Good fix: localized "Other" removal matches guidelines and past feedback.Switching to S.of(context)!.other with trim/lowercase normalization removes hardcoded literals ("Other"/"Otro"/"Altro") and aligns with our localization guideline. This directly addresses the earlier review feedback and prevents future locale drift.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/walkthrough/screens/walkthrough_screen.dart (1)
23-23: Add create-offer step flag: good addition; consider tightening the APIThe new
isCreateOfferStepflag integrates cleanly and is backward compatible. To avoid “boolean soup” as steps grow, consider replacing multiple booleans with a single enum parameter (e.g.,HighlightStep) for clarity and to prevent conflicting flags being set simultaneously.Example approach:
enum HighlightStep { firstStep, privacy, security, chat, orderBook, createOffer } // ... Widget _buildHighlightedText( String text, { HighlightStep step = HighlightStep.firstStep, }) { /* map step -> HighlightConfig via switch */ }I can provide a targeted refactor across call sites if you want to proceed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/features/walkthrough/screens/walkthrough_screen.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/@(screens|widgets)/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods
Files:
lib/features/walkthrough/screens/walkthrough_screen.dart
lib/!(generated)/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible
Files:
lib/features/walkthrough/screens/walkthrough_screen.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/walkthrough/screens/walkthrough_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-08-15T13:19:31.968Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-15T13:19:31.968Z
Learning: Applies to lib/!(generated)/**/*.dart : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Applied to files:
lib/features/walkthrough/screens/walkthrough_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
lib/features/walkthrough/screens/walkthrough_screen.dart (3)
43-45: Create-offer mapping added in highlight resolution: LGTMThe resolution chain now covers the new step and maintains a clear priority order. This aligns with the PR goal to highlight “create your own offer.”
47-51: Case-insensitive, Unicode-aware highlighting: resolved and correctSwitching to
caseSensitive: false, unicode: trueaddresses prior feedback and should reliably match localized strings (e.g., Spanish “Facturas Retenidas” and the create-offer copy) regardless of capitalization or diacritics.
159-160: All create-offer localization and highlight patterns are correctly covered
HighlightConfig.createOffer.pattern(lib/features/walkthrough/utils/highlight_config.dart:29) includes English, Spanish (crear tu propia oferta), and Italian (creare la tua offerta).- The
createYourOwnOfferkey exists inlib/l10n/intl_en.arb,intl_es.arb, andintl_it.arb.- “Facturas Retenidas” is present in
HighlightConfig.security.pattern(lib/features/walkthrough/utils/highlight_config.dart:16).No further changes required.
Summary by CodeRabbit