Conversation
Replace remaining light-mode hardcoded colors with the canonical design tokens from Resources/DesignTokens.xaml: - MigrationStatusWindow: surface, border, secondary text, and accent brushes now come from the shared dictionary. Switched the Window Background to Transparent so the rounded Border (already in place but previously hidden behind opaque white) actually shows. - FirmwareDialog: replaced the semi-transparent white scrim shown during firmware upload (#ccffffff) with a new Scrim token (#CC000000) and gave the overlay labels TextPrimary so they read on the dark scrim. - DesignTokens: added a Scrim brush (#CC000000) so future overlays can reference the token instead of inlining hex.
Contributor
Review Summary by QodoApply dark theme to MigrationStatusWindow and firmware upload overlay
WalkthroughsDescription• Replace hardcoded light-mode colors with dark theme design tokens • Add new Scrim overlay token for consistent dark overlays • Fix MigrationStatusWindow rounded corners by making background transparent • Improve firmware upload overlay readability with dark scrim and text styling Diagramflowchart LR
A["Hardcoded Colors<br/>#ccffffff, #3498db, #555555"] -->|Replace with tokens| B["Design Tokens<br/>Surface, Accent, TextSecondary"]
C["White Window Background"] -->|Change to Transparent| D["Visible Rounded Corners"]
E["Light Scrim Overlay"] -->|New Scrim Token| F["Dark #CC000000 Overlay"]
B --> G["Dark Theme UI"]
D --> G
F --> G
File Changes1. Daqifi.Desktop/Resources/DesignTokens.xaml
|
Contributor
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
- MigrationStatusWindow: revert Window.Background back to opaque Surface (from Transparent). Avoids the layered-window perf cost Qodo flagged while keeping the color-token fix. Original visual behavior preserved (rounded-corner reveal was scope creep). - FirmwareDialog: split the two overlay Label declarations across multiple lines so they comply with the 120-char rule.
Contributor
Author
Responses to Qodo's review summary1. Long XAML 2. Layered window perf risk (advisory) — Agreed and reverted. Changed |
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 17.6%
Daqifi.Desktop.Common - 30.8%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two remaining light-mode UI leaks are visibly out of place against the unified dark theme that landed across the design-philosophy sweep (#479, #484, #485, #486, #488, #492, #494, #495). Cleaning them up before the v3.2.0 release (#503) so first-launch upgraders and anyone uploading firmware see the dark surface they expect.
MigrationStatusWindow.xaml— was a fully white window with a#3498dbprogress bar. This shows on first launch after the 3.1.x → 3.2.0 EnsureCreated → EF Migrations upgrade flow (feat: switch from EnsureCreated() to EF Migrations for schema updates #469). Now usesSurface/BorderDim/TextSecondary/AccentfromResources/DesignTokens.xaml. Also flipped theWindow.BackgroundtoTransparentso the existing roundedBorder(CornerRadius="4", previously occluded by the opaque white window background) actually renders.FirmwareDialog.xaml— the upload-in-progress scrim was#ccffffff(80% white), which washed out the dark dialog underneath while firmware was uploading. Replaced with a newScrimtoken (#CC000000) and addedForeground="{StaticResource TextPrimary}"to the twoLabels sitting on the scrim so they remain readable.Tokens used / added
Used existing tokens from
Resources/DesignTokens.xaml:Surface,BorderDim,TextSecondary,TextPrimary,AccentAdded one new token (placed under a new "Overlays" section):
Scrim—#CC000000, matches the#CC000000overlay used inLoggedDataPanePrototype.xaml:1011for the same blocking-overlay role. Existing scrims using inline#A0000000were intentionally left alone — replacing them is broader than this hot-fix.Test plan
MigrationStatusWindowactually render (Window.Background is now Transparent).MetroProgressBarinside the firmware overlay still reads correctly against the dark scrim (default MahApps Accent should be visible — please eyeball during a real upload).Notes / unverified
I'm on macOS so I couldn't run the WPF app (
net10.0-windows). Changes are XAML-only and follow the established pattern fromConnectionDialog.xaml(#488) —StaticResourcelookup walks Window.Resources → Application.Resources, whereDesignTokens.xamlis merged inApp.xaml. SinceApplication.InitializeComponentruns beforeOnStartup(whereMigrationStatusWindowis shown), the brushes should resolve. Worth a sanity check on Windows before merging.