Repository navigation
fix: Properly dispose timer to avoid visual double ticks in time tracking - #173
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe pull request modifies EditCardModal.razor to add null guards and explicit cleanup for Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
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.
Pull request overview
This PR fixes a resource disposal issue in the EditCardModal component by ensuring that the timer is properly stopped and disposed when the component is disposed, preventing visual double ticks in time tracking.
- Refactored the
_pasteObjRefdisposal logic to use positive null checks instead of early returns - Added proper disposal of
_timerto stop it before disposing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ticky.Web/Components/Dialogs/EditCardModal.razor (1)
595-613: Resource leak: Timer not disposed before recreating.The
UpdateTimer()method creates a new timer at line 609 without disposing the existing one. If this method is called multiple times while a time record is active (e.g., throughUpdateCard()→ state changes), multiple timer instances will accumulate, each firingOnTimerTickand causing excess UI updates and memory leaks.Additionally, lines 601-602 stop the timer but don't dispose it when no active record exists.
🔎 Proposed fix
private void UpdateTimer() { var targetRecord = _card?.TimeRecords?.FirstOrDefault(x => x.UserId.Equals(_user.Id) && x.EndedAt == null); if(_card is null || targetRecord is null) { if (_timer is not null) + { _timer.Stop(); + _timer.Dispose(); + _timer = null; + } return; } _ongoingTimer = DateTime.Now - targetRecord.StartedAt; StateHasChanged(); + // Dispose existing timer before creating a new one + if (_timer is not null) + { + _timer.Stop(); + _timer.Dispose(); + } + _timer = new System.Timers.Timer(1000); _timer.Elapsed += OnTimerTick; _timer.AutoReset = true; _timer.Start(); }
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
Ticky.Web/Components/Dialogs/EditCardModal.razor
⏰ 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: Analyze (csharp)
🔇 Additional comments (2)
Ticky.Web/Components/Dialogs/EditCardModal.razor (2)
516-520: LGTM: Consistent disposal pattern.The refactor from early return to guarded disposal is functionally equivalent and creates consistency with the timer disposal pattern added below.
522-527: Proper timer cleanup prevents resource leaks.The timer disposal correctly stops and releases the timer resource when the component is disposed, directly addressing the "double ticks" issue mentioned in the PR title.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… fix/properly-dispose-timer
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.