-
Notifications
You must be signed in to change notification settings - Fork 0
flutter: transactions Phase A — search/filter bar + grouping scaffold #65
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
…nce.share/shareXFiles in QR + ShareService; remove local stub
…hare_plus directly
…g and DeleteFamilyDialog
…or) + fix const-eval in common widgets
…pture and fix duplicates
…ignores; fix template onCancel and batch dialogs snackbars
…vigator capture and mounted guards
…rs (accounts/dashboard types, const-eval, family settings void, email Address stub); wire A/B/C entrances
…oup toggle in TransactionList; optional filter icon in RecentTransactions
…sactions; finalize TransactionList search bar impl
- Add onFilter parameter to constructor - Fixes compilation error: getter 'onFilter' isn't defined 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Resolved git merge conflicts that were polluting all feature branches: - theme_management_screen.dart: 5 conflicts (ScaffoldMessenger patterns) - transaction_provider.dart: 2 conflicts (duplicate enum & method definitions) - family_activity_log_screen.dart: 1 conflict (statistics loading) All conflicts resolved by: - Preferring messenger variable pattern over repeated ScaffoldMessenger.of() - Removing duplicate enum and method definitions - Using direct stats return instead of _parseActivityStatistics() Added comprehensive PR fix report documenting all 5 PRs analyzed. This fix will automatically benefit all PRs (#65, #66, #68, #69, #70) as they rebase/merge from main. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Resolved 15 file conflicts by accepting main's clean version. Same pattern as PR #65 - duplicate variable declarations removed.
# Conflicts: # jive-flutter/lib/screens/admin/template_admin_page.dart # jive-flutter/lib/screens/auth/login_screen.dart # jive-flutter/lib/screens/family/family_activity_log_screen.dart # jive-flutter/lib/screens/theme_management_screen.dart # jive-flutter/lib/services/family_settings_service.dart # jive-flutter/lib/services/share_service.dart # jive-flutter/lib/ui/components/accounts/account_list.dart # jive-flutter/lib/ui/components/transactions/transaction_list.dart # jive-flutter/lib/widgets/batch_operation_bar.dart # jive-flutter/lib/widgets/common/right_click_copy.dart # jive-flutter/lib/widgets/custom_theme_editor.dart # jive-flutter/lib/widgets/dialogs/accept_invitation_dialog.dart # jive-flutter/lib/widgets/dialogs/delete_family_dialog.dart # jive-flutter/lib/widgets/qr_code_generator.dart # jive-flutter/lib/widgets/theme_share_dialog.dart
2f6ab76 to
7a4f9ce
Compare
- Update _TestTransactionController to accept Ref parameter - Use StateNotifierProvider pattern for test controller instantiation - Remove duplicate _buildSearchBar from SwipeableTransactionList - All transaction tests now passing (3/3) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Pull Request Overview
Adds Phase A UI scaffolding for transactions list: introduces optional search bar, grouping toggle, and filter placeholder while adapting tests to new controller constructor requiring a Ref. Also updates a test controller pattern to use Riverpod.
- Added search/filter/group controls to TransactionList (non-breaking via optional callbacks).
- Updated transaction controller tests to construct via ProviderContainer after constructor signature change.
- Accidental inclusion of environment-specific .dart_tool/package_config.json path changes.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| test/transactions/transaction_controller_grouping_test.dart | Adjusts test controller to accept Ref and uses ProviderContainer/provider for instantiation. |
| lib/ui/components/transactions/transaction_list.dart | Introduces search bar UI, new callback properties, grouping toggle and filter placeholder. |
| .dart_tool/package_config.json | Environment-specific path changes (likely unintended; should not be committed). |
Comments suppressed due to low confidence (1)
jive-flutter/lib/ui/components/transactions/transaction_list.dart:1
- TextField has no controller, so invoking onClearSearch cannot clear the visible query unless an external rebuild resets it. Introduce a TextEditingController and call controller.clear() in the clear handler or convert to a controlled input to keep UI state in sync.
// 交易列表组件
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Phase A: lightweight search/group controls | ||
| final ValueChanged<String>? onSearch; | ||
| final VoidCallback? onClearSearch; | ||
| final VoidCallback? onToggleGroup; |
Copilot
AI
Oct 8, 2025
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.
New public callback properties lack doc comments. Add Dart doc comments (///) describing when each callback is invoked and expected side effects to aid maintainability and API clarity.
| Expanded( | ||
| child: TextField( | ||
| decoration: InputDecoration( | ||
| hintText: '搜索 描述/备注/收款方…', |
Copilot
AI
Oct 8, 2025
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.
Hard-coded user-facing string should use the app's localization mechanism (e.g., AppLocalizations) instead of a literal to support i18n.
| ), | ||
| const SizedBox(width: 8), | ||
| IconButton( | ||
| tooltip: groupByDate ? '切换为平铺' : '按日期分组', |
Copilot
AI
Oct 8, 2025
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.
Hard-coded tooltip string should be localized via the project's localization system.
| icon: Icon(groupByDate ? Icons.view_agenda_outlined : Icons.calendar_today_outlined), | ||
| ), | ||
| IconButton( | ||
| tooltip: '筛选', |
Copilot
AI
Oct 8, 2025
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.
Hard-coded tooltip string should be localized for internationalization consistency.
| tooltip: '筛选', | ||
| onPressed: () { | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar(content: Text('筛选功能开发中')), |
Copilot
AI
Oct 8, 2025
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.
Hard-coded SnackBar message should be localized to maintain consistency with internationalization practices.
| hintText: '搜索 描述/备注/收款方…', | ||
| prefixIcon: const Icon(Icons.search), | ||
| suffixIcon: onClearSearch != null | ||
| ? IconButton(icon: const Icon(Icons.clear), onPressed: onClearSearch) |
Copilot
AI
Oct 8, 2025
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.
Clear IconButton lacks a tooltip or semantic label, which can hinder screen reader users. Wrap with Tooltip and/or set semantic properties (e.g., Tooltip + Semantics(label: ...)).
| ? IconButton(icon: const Icon(Icons.clear), onPressed: onClearSearch) | |
| ? IconButton( | |
| icon: const Icon(Icons.clear), | |
| onPressed: onClearSearch, | |
| tooltip: '清除搜索', | |
| ) |
| { | ||
| "name": "flutter", | ||
| "rootUri": "file:///opt/homebrew/share/flutter/packages/flutter", | ||
| "rootUri": "file:///Users/huazhou/flutter-sdk/packages/flutter", |
Copilot
AI
Oct 8, 2025
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.
Environment-specific absolute path in generated .dart_tool file should not be committed; revert this file and add/ensure .dart_tool is excluded from version control to avoid local path churn and leaking local usernames. Other similar lines (288, 312, 318, 792, 1057) should also be reverted.
Merged after inheriting main's messenger pattern fixes and test updates from PR #65
Scaffold Phase A for transactions list: adds optional search bar, group toggle, and filter entry (non-breaking).