Adjust behavior on clear date

[piotr-iohk]

Description

Adjusting behavior on clear date in activity list as per comment

⚠️ please merge with iOS counterpart (synonymdev/bitkit-ios#241)

Preview

QA Notes

synonymdev/bitkit-e2e-tests#54

[Copilot]

Pull request overview

This PR adjusts the behavior when clearing the date filter in the activity list by keeping the date range selector sheet open after clearing, instead of immediately dismissing it. This allows users to immediately select a new date range without having to reopen the sheet.

Key Changes

• Removes automatic sheet dismissal when clearing date filters
• The sheet now remains open after clearing, allowing users to apply a new selection immediately

[claude]

Code Review - PR #481: Adjust behavior on clear date

Summary

This PR removes the app.hideSheet() call when clearing the date range in the Date Range Selector Sheet. This change keeps the sheet open after clearing the date selection, allowing users to select a new date range without having to reopen the sheet.

✅ Positive Observations

1. Clear Intent & Good UX Decision

• The change improves user experience by keeping the sheet open after clearing dates, which is consistent with typical date picker behavior
• Users can immediately select a new date range without having to reopen the sheet
• The change aligns with the iOS counterpart (PR Add systemBarsPadding to receive sheet #241)

2. Code Quality

• The change is minimal and focused (single line removal)
• The existing code follows Kotlin and Compose best practices
• Proper use of StateFlow for reactive state management (_filters, startDate, endDate)
• ViewModelScope is correctly used in ActivityListViewModel.kt:145-147

3. Architecture Compliance

• Follows MVVM pattern correctly - ViewModel (ActivityListViewModel) handles business logic while the Composable handles UI
• Proper separation of concerns: clearDateRange() in ViewModel updates state, UI reacts to state changes
• No architecture violations introduced

📝 Observations & Suggestions

1. Test Coverage

• ⚠️ No test files found for DateRangeSelectorSheet or ActivityListViewModel
• Suggestion: Consider adding UI tests to verify:
  • Sheet remains open after clearing date range
  • Date range state is properly cleared
  • Apply/Clear button states update correctly

2. Consistency Check

• ✅ The "Apply" button (line 124-129) correctly closes the sheet with app.hideSheet()
• ✅ The "Clear" button now only clears state without closing the sheet, which is the intended behavior per the iOS PR reference

3. State Management

• The local state (startDate, endDate at lines 152-153) is properly cleared in the button's onClick handler (lines 442-444)
• The ViewModel's state is updated via clearDateRange() call
• This dual update ensures UI consistency

🔍 Potential Considerations

1. User Workflow Analysis
The current behavior after this change:

• User opens date picker → Selects dates → Clicks Apply → Sheet closes ✅
• User opens date picker → Selects dates → Clicks Clear → Sheet stays open → Can select new dates or close manually ✅
• This is good UX and follows common date picker patterns

2. Minor Enhancement Opportunity (Optional, not blocking)
Consider whether the BackHandler (lines 114-116) should also clear the date selection when users dismiss the sheet via back gesture/button. Currently:

• BackHandler only closes sheet without clearing local state
• This means if a user selects dates but backs out, the selection persists in local state (though not applied to ViewModel)
• This is likely acceptable behavior, but worth considering

🔒 Security & Performance

• ✅ No security concerns identified
• ✅ No performance issues - the change removes a call, slightly improving performance
• ✅ No null safety issues or potential crashes

📊 Code Standards (per .github/copilot-instructions.md)

• ✅ No !! operator used (good null safety)
• ✅ String resources properly used via stringResource()
• ✅ Compose best practices followed (remember blocks, proper state management)
• ✅ Modifiers in correct position (last parameter)
• ✅ No ViewModels injected into wrong layers

✅ Recommendation

APPROVE - This is a clean, focused change that improves UX without introducing any issues. The lack of automated tests is noted but not blocking for a single-line UI behavior change. The change is:

• Intentional and well-motivated
• Consistent with platform conventions
• Safe and backwards compatible
• Properly coordinated with iOS counterpart

The implementation correctly maintains the sheet open while properly clearing both local and ViewModel state, allowing users to immediately make a new selection.

---

Review completed following repository conventions from .github/copilot-instructions.md