Fix: Disable long-press menus and update poll provider#205
Fix: Disable long-press menus and update poll provider#205kumarpalsinh25 merged 2 commits intomainfrom
Conversation
This commit introduces several fixes and minor refactorings: - Disables the long-press context menu for event and poll items when they are displayed in lists outside of their main detail screens to prevent unintended actions. - Updates the "Today's Focus" widget to use `activePollListProvider` instead of the more specific `activePollsWithPendingResponseProvider` for displaying polls. - Reorders the tabs on the Polls List Screen from `Draft, Active, Completed` to `Active, Draft, Completed` for better user experience. - Bumps the `cloud_firestore` pod version to `6.1.1` in `Podfile.lock`.
📝 WalkthroughWalkthroughUI widgets were adjusted: event and poll title long-press handlers made conditional; home today-focus now consumes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/features/polls/screens/polls_list_screen.dart (1)
110-114: Improved code formatting for readability.The multi-line formatting of
EmptyStateListWidgetconstructor calls improves readability with no functional changes.Also applies to: 126-130, 142-146
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
lib/features/events/widgets/event_widget.dartlib/features/home/widgets/today_focus/todays_focus_widget.dartlib/features/home/widgets/today_focus/todays_item_widget.dartlib/features/polls/screens/polls_list_screen.dartlib/features/polls/widgets/poll_widget.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). (7)
- GitHub Check: test
- GitHub Check: linux
- GitHub Check: ios
- GitHub Check: windows
- GitHub Check: android
- GitHub Check: macos
- GitHub Check: Test zoe_native plugin
🔇 Additional comments (3)
lib/features/home/widgets/today_focus/todays_item_widget.dart (1)
54-54: LGTM! Icon parameter now properly utilized.The header icon now correctly uses the provided
iconparameter instead of the hardcoded value, making the widget properly reusable.lib/features/polls/widgets/poll_widget.dart (1)
124-132: LGTM! Long-press correctly disabled in list views.The conditional logic properly restricts the long-press menu to detail screens only, preventing unintended actions when polls are displayed in lists outside their main detail screen.
lib/features/polls/screens/polls_list_screen.dart (1)
58-60: LGTM! Tab reordering implemented correctly.The tabs now display in the order "Active, Draft, Completed" as intended, with the
TabBarViewchildren properly aligned to match the new tab sequence.Also applies to: 92-94
| final activePollsWithPendingResponse = ref.watch( | ||
| activePollsWithPendingResponseProvider, | ||
| ); | ||
| final activePollsList = ref.watch(activePollListProvider); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*.dart" -type f | head -20Repository: acterglobal/zoe
Length of output: 973
🏁 Script executed:
# Search for provider definitions
rg -n 'final.*activePollListProvider|final.*activePollsWithPendingResponseProvider' --type=dartRepository: acterglobal/zoe
Length of output: 557
🏁 Script executed:
# Search more broadly for these providers
rg -B5 -A10 'activePollListProvider|activePollsWithPendingResponseProvider' --type=dartRepository: acterglobal/zoe
Length of output: 50371
🏁 Script executed:
# Find the provider definitions in poll_providers.dart
rg -B5 -A15 'final activePollList|final activePollsWithPendingResponse' lib/features/polls/providers/poll_providers.dartRepository: acterglobal/zoe
Length of output: 662
🏁 Script executed:
# Check the todays_focus_widget_test to see if it's still using the old provider
head -60 test/features/home/widgets/today_focus/todays_focus_widget_test.dart | grep -A5 -B5 'activePoll'Repository: acterglobal/zoe
Length of output: 345
🏁 Script executed:
# Get the full provider definitions more clearly
sed -n '310,330p' lib/features/polls/providers/poll_providers.dartRepository: acterglobal/zoe
Length of output: 663
🏁 Script executed:
# Search for activePollsWithPendingResponse definition
rg -B2 -A20 'activePollsWithPendingResponseProvider' lib/features/polls/providers/poll_providers.dart | head -50Repository: acterglobal/zoe
Length of output: 41
🏁 Script executed:
# Check if tests in todays_focus_widget_test.dart need to be updated
grep -n 'activePollsWithPendingResponseProvider\|activePollListProvider' test/features/home/widgets/today_focus/todays_focus_widget_test.dart | head -20Repository: acterglobal/zoe
Length of output: 1394
Update tests to match the provider change in the widget.
The widget now uses activePollListProvider, which returns all active polls (including those the user has already voted on), not just pending ones. However, the test file test/features/home/widgets/today_focus/todays_focus_widget_test.dart still overrides activePollsWithPendingResponseProvider in 18+ test cases. Update all test overrides to use activePollListProvider instead to match the actual widget implementation.
Additionally, confirm that displaying all active polls (not just pending) is the intended behavior for the Today's Focus section.
🤖 Prompt for AI Agents
In lib/features/home/widgets/today_focus/todays_focus_widget.dart around line 24
and in test/features/home/widgets/today_focus/todays_focus_widget_test.dart, the
widget now reads from activePollListProvider but the tests still override
activePollsWithPendingResponseProvider; update all test overrides (18+
instances) to override activePollListProvider instead, adjust any mocked return
values to reflect that it now returns all active polls (including
already-voted), and run/adjust assertions to confirm the widget displays all
active polls as intended for Today's Focus.
Introduce a new boolean property `isTitleLongPressEnabled` to the `EventWidget`. This property is derived from the inverse of `showSheetName` and is used to conditionally enable or disable the `onTapLongPressText` gesture, which shows the event menu. This makes the condition for enabling the long press more explicit and self-contained within the widget's logic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/events/widgets/event_widget.dart (1)
20-20: Consider makingisTitleLongPressEnabledan explicit constructor parameter.The new field improves clarity at the usage site (line 110), but it remains semantically coupled to
showSheetNamethrough the derived relationship. Having a public field that's always computed from another parameter is unconventional and prevents callers from independently controlling long-press behavior.Consider decoupling these concerns by making
isTitleLongPressEnabledan explicit constructor parameter with a default value, similar to theisDetailScreenpattern mentioned in past reviews forpoll_widget.dart.🔎 Proposed refactor to decouple the parameters
class EventWidget extends ConsumerWidget { final String eventId; final EdgeInsetsGeometry? margin; final bool showSheetName; - final bool isTitleLongPressEnabled; + final bool enableLongPress; const EventWidget({ super.key, required this.eventId, this.margin, this.showSheetName = true, - }) : isTitleLongPressEnabled = !showSheetName; + this.enableLongPress = false, + });Then update line 110 to use
enableLongPressdirectly:- onTapLongPressText: isTitleLongPressEnabled + onTapLongPressText: enableLongPress ? () => showEventMenu(This approach allows callers to independently control sheet name display and long-press behavior, improving flexibility and clarity.
Also applies to: 27-27
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/events/widgets/event_widget.dart
🔇 Additional comments (1)
lib/features/events/widgets/event_widget.dart (1)
110-117: LGTM: Conditional long-press logic is correctly implemented.The ternary operator properly enables/disables the long-press gesture by passing either the
showEventMenuclosure ornulltoonTapLongPressText. This approach follows Flutter conventions wherenullcallbacks disable gestures.
1 participant
This commit introduces several fixes and minor refactorings:
activePollListProviderinstead of the more specificactivePollsWithPendingResponseProviderfor displaying polls.Draft, Active, CompletedtoActive, Draft, Completedfor better user experience.cloud_firestorepod version to6.1.1inPodfile.lock.Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.