fix: deep uniformity + performance — tokens, bulk collections, toasts#543
Conversation
|
Warning Review limit reached
More reviews will be available in 8 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between fed5030c4032b766d61bcd81291410067590087c and c0566c7. 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis pull request modernizes the SysManager application through coordinated performance, user experience, and visual improvements: observable collection batching reduces notification overhead in ViewModels, toast notifications confirm operation completion across workflows, UI views migrate from hardcoded colors to theme-resource tokens, a minor async optimization improves Windows Update scheduling, and release notes document all changes. ChangesPerformance, UX, and Visual Modernization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
40f717e to
33748a7
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SysManager/SysManager/Views/UninstallerView.xaml (1)
194-205:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete the Accent tokenization.
Line 197 uses the hardcoded value
#6366F1, which is the same accent color tokenized on line 184. Both elements are part of the same "Source" indicator template and should use the same token for consistency.🎨 Proposed fix to complete tokenization
<Style TargetType="Ellipse"> - <Setter Property="Fill" Value="`#6366F1`"/> + <Setter Property="Fill" Value="{StaticResource Accent}"/> <Style.Triggers>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SysManager/SysManager/Views/UninstallerView.xaml` around lines 194 - 205, The Ellipse inside the "Source" indicator template hardcodes the accent color in the Setter (Ellipse.Style -> Style -> Setter Property="Fill" Value="`#6366F1`"); replace that hardcoded value with the same accent resource/token used by the other element in the same template (i.e., swap Value="`#6366F1`" for the existing StaticResource/DynamicResource token name used for accent color in the template) so both indicators reference the identical accent token.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 22: The changelog shows four releases ([1.10.1], [1.10.0], [1.9.1],
[1.9.0]) all with the same date and [1.10.1] contains a "UI uniformity audit"
that may not be a patch; verify and correct the versioning and dates: confirm
the actual release dates for each tag and update each header to the correct date
or consolidate entries if they represent a single release, and reevaluate
whether the [1.10.1] UI changes should be promoted to a minor bump (e.g.,
1.11.0) instead of a patch to preserve semantic versioning; adjust the headers
and move the UI audit under the appropriate version section accordingly.
In `@SysManager/SysManager/ViewModels/DeepCleanupViewModel.cs`:
- Line 190: ScanCoreAsync always shows a completion toast
(ToastService.Instance.Show) which causes a duplicate notification when
CleanAsync finishes and then calls ScanCoreAsync; modify ScanCoreAsync to take
an optional boolean parameter (e.g., bool suppressToast = false) and only call
ToastService.Instance.Show when suppressToast is false, then update CleanAsync
to call ScanCoreAsync with suppressToast = true so only the CleanAsync
completion toast is shown.
---
Outside diff comments:
In `@SysManager/SysManager/Views/UninstallerView.xaml`:
- Around line 194-205: The Ellipse inside the "Source" indicator template
hardcodes the accent color in the Setter (Ellipse.Style -> Style -> Setter
Property="Fill" Value="`#6366F1`"); replace that hardcoded value with the same
accent resource/token used by the other element in the same template (i.e., swap
Value="`#6366F1`" for the existing StaticResource/DynamicResource token name used
for accent color in the template) so both indicators reference the identical
accent token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ecd29fc4-f6b7-4e97-bba7-2f4de17fc09e
📥 Commits
Reviewing files that changed from the base of the PR and between 0507eff and fed5030c4032b766d61bcd81291410067590087c.
📒 Files selected for processing (13)
CHANGELOG.mdSysManager/SysManager/ViewModels/AppAlertsViewModel.csSysManager/SysManager/ViewModels/AppBlockerViewModel.csSysManager/SysManager/ViewModels/DeepCleanupViewModel.csSysManager/SysManager/ViewModels/DriversViewModel.csSysManager/SysManager/ViewModels/NetworkRepairViewModel.csSysManager/SysManager/ViewModels/ServicesViewModel.csSysManager/SysManager/ViewModels/ShortcutCleanerViewModel.csSysManager/SysManager/ViewModels/WindowsUpdateViewModel.csSysManager/SysManager/Views/AppBlockerView.xamlSysManager/SysManager/Views/DashboardView.xamlSysManager/SysManager/Views/DnsHostsView.xamlSysManager/SysManager/Views/UninstallerView.xaml
📜 Review details
🔇 Additional comments (18)
SysManager/SysManager/Views/AppBlockerView.xaml (3)
9-10: LGTM!
24-25: LGTM!
63-73: LGTM!Also applies to: 81-84
SysManager/SysManager/Views/DashboardView.xaml (4)
14-14: LGTM!
91-92: LGTM!Also applies to: 107-135
157-175: LGTM!
182-314: LGTM!Also applies to: 320-349, 361-361
SysManager/SysManager/Views/DnsHostsView.xaml (1)
120-123: LGTM!SysManager/SysManager/Views/UninstallerView.xaml (1)
184-184: LGTM!CHANGELOG.md (1)
9-21: LGTM!SysManager/SysManager/ViewModels/AppAlertsViewModel.cs (1)
10-10: LGTM!Also applies to: 25-25, 93-95, 99-99
SysManager/SysManager/ViewModels/AppBlockerViewModel.cs (1)
21-21: LGTM!Also applies to: 45-45
SysManager/SysManager/ViewModels/ShortcutCleanerViewModel.cs (1)
9-9: LGTM!Also applies to: 23-23, 56-57, 70-70, 79-79, 118-118
SysManager/SysManager/ViewModels/DeepCleanupViewModel.cs (1)
298-298: LGTM!SysManager/SysManager/ViewModels/DriversViewModel.cs (1)
71-71: LGTM!SysManager/SysManager/ViewModels/NetworkRepairViewModel.cs (1)
95-98: LGTM!SysManager/SysManager/ViewModels/ServicesViewModel.cs (1)
91-91: LGTM!SysManager/SysManager/ViewModels/WindowsUpdateViewModel.cs (1)
67-67: LGTM!
| - **Task.Delay(1)** → Task.Yield() in WindowsUpdateViewModel. | ||
| - **UninstallerView** — hardcoded `#6366F1` replaced with `{StaticResource Accent}`. | ||
|
|
||
| ## [1.10.1] - 2026-05-26 |
There was a problem hiding this comment.
Verify version numbers and release dates.
Four distinct versions ([1.10.1], [1.10.0], [1.9.1], [1.9.0]) all share the same release date (2026-05-26), which is unusual and may indicate:
- Version numbering mistakes
- Releases that should be consolidated
- Missing date corrections
Additionally, the content under [1.10.1] (lines 24-34) describes "UI uniformity audit" which sounds more like a minor feature/refactor than a patch-level bug fix, potentially misaligning with semantic versioning conventions where x.x.PATCH should represent backwards-compatible bug fixes only.
Consider:
- Consolidating same-day releases if they represent a single logical release
- Updating dates if releases actually occurred on different days
- Reviewing whether [1.10.1] changes are truly patch-level or should be a minor version bump
Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` at line 22, The changelog shows four releases ([1.10.1],
[1.10.0], [1.9.1], [1.9.0]) all with the same date and [1.10.1] contains a "UI
uniformity audit" that may not be a patch; verify and correct the versioning and
dates: confirm the actual release dates for each tag and update each header to
the correct date or consolidate entries if they represent a single release, and
reevaluate whether the [1.10.1] UI changes should be promoted to a minor bump
(e.g., 1.11.0) instead of a patch to preserve semantic versioning; adjust the
headers and move the UI audit under the appropriate version section accordingly.
| @@ -187,6 +187,7 @@ private async Task ScanCoreAsync() | |||
| var total = cats.Sum(c => c.TotalSizeBytes); | |||
| ScanSummary = $"Found {FormatHelper.FormatSize(total)} across {cats.Count} categories. Untick anything you want to keep."; | |||
| ScanStatusLine = "Scan complete."; | |||
| ToastService.Instance.Show("Deep cleanup scan complete", $"{FormatHelper.FormatSize(total)} across {cats.Count} categories"); | |||
There was a problem hiding this comment.
Avoid double toast notifications after a successful clean.
CleanAsync shows a completion toast (Line 231) and then calls ScanCoreAsync (Line 233), which now always shows another scan-complete toast (Line 190). This creates two back-to-back toasts for one user action.
Suggested minimal adjustment
-private async Task ScanCoreAsync()
+private async Task ScanCoreAsync(bool showToast = true)
{
...
- ToastService.Instance.Show("Deep cleanup scan complete", $"{FormatHelper.FormatSize(total)} across {cats.Count} categories");
+ if (showToast)
+ ToastService.Instance.Show("Deep cleanup scan complete", $"{FormatHelper.FormatSize(total)} across {cats.Count} categories");
...
}
...
-await ScanCoreAsync();
+await ScanCoreAsync(showToast: false);Also applies to: 231-233
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/DeepCleanupViewModel.cs` at line 190,
ScanCoreAsync always shows a completion toast (ToastService.Instance.Show) which
causes a duplicate notification when CleanAsync finishes and then calls
ScanCoreAsync; modify ScanCoreAsync to take an optional boolean parameter (e.g.,
bool suppressToast = false) and only call ToastService.Instance.Show when
suppressToast is false, then update CleanAsync to call ScanCoreAsync with
suppressToast = true so only the CleanAsync completion toast is shown.
- DashboardView: 30+ hardcoded hex → StaticResource tokens - AppBlockerView: full modernization (Display, Card, button styles, DataGrid) - DnsHostsView: DataGrid grid-lines, accessibility, text token - ObservableCollection → BulkObservableCollection on AppAlerts, AppBlocker, ShortcutCleaner (single Reset notification vs N Add events) - Add toast notifications on Drivers, Services, ShortcutCleaner, DeepCleanup, NetworkRepair completions - Task.Delay(1) → Task.Yield() in WindowsUpdateViewModel
fed5030 to
eb2838e
Compare
eb2838e to
c0566c7
Compare
1 participant
Tokens, BulkObservableCollection, toast notifications, structural modernization.
Summary by CodeRabbit