Change text color on red buttons#333
Conversation
WalkthroughUpdated the cancel button in the trade detail screen to consistently use the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/trades/screens/trade_detail_screen.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/trades/screens/trade_detail_screen.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/trades/screens/trade_detail_screen.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/trades/screens/trade_detail_screen.dart
🔇 Additional comments (2)
lib/features/trades/screens/trade_detail_screen.dart (2)
642-645: LGTM! White text improves visibility on red buttons.The explicit
foregroundColor: Colors.whiteensures proper contrast against the red background (AppTheme.red1), improving readability and accessibility.
747-750: LGTM! Consistent styling ensures visibility.The explicit
foregroundColor: Colors.whiteon the dispute button matches the cancel button styling and ensures proper text contrast on the red background.
| widgets.add(_buildCancelButton( | ||
| context, | ||
| ref, | ||
| buttonText, | ||
| S.of(context)!.acceptCancelButton, | ||
| cancelMessage, | ||
| action, | ||
| )); |
There was a problem hiding this comment.
Verify button text semantics for different cancel scenarios.
The button now always displays acceptCancelButton regardless of whether the user is initiating a cancel or accepting a peer's cooperative cancel request. This may create semantic confusion:
- When the user initiates a cancel (lines 286, 289), the button text "Accept Cancel" suggests accepting rather than initiating
- Only when accepting a peer's cancel (line 284) does "Accept Cancel" match the action
Consider using conditional button text that accurately reflects the action:
cancelButtonorconfirmCancelButtonwhen initiatingacceptCancelButtonwhen accepting a peer's cancel request
Apply this diff to restore context-appropriate button text:
+ String buttonText;
+ if (tradeState.action ==
+ actions.Action.cooperativeCancelInitiatedByPeer) {
+ buttonText = S.of(context)!.acceptCancelButton;
+ } else {
+ buttonText = S.of(context)!.cancelButton;
+ }
+
widgets.add(_buildCancelButton(
context,
ref,
- S.of(context)!.acceptCancelButton,
+ buttonText,
cancelMessage,
action,
));Note: Ensure cancelButton exists in your localization files. If it doesn't, you may need to add it or use an appropriate alternative like confirmButton.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/features/trades/screens/trade_detail_screen.dart around lines 292 to 298,
the cancel button text is always using S.of(context)!.acceptCancelButton which
is semantically wrong for the case where the user is initiating a cancel; change
the call to choose the label conditionally: use
S.of(context)!.acceptCancelButton when handling an incoming peer cancel
acceptance, and use S.of(context)!.cancelButton (or
S.of(context)!.confirmCancelButton / S.of(context)!.confirmButton if
cancelButton is not available) when the user is initiating a cancel; update the
_buildCancelButton invocation to pass the appropriate localized string based on
the same condition used to determine the action, and add the missing
localization key if necessary.
Summary by CodeRabbit
Release Notes