UI dedup: shared DeviceDashboard + TransientNotice helper + spin-keyframe cleanup (audit #33/#34/#35) - #65
Merged
Merged
Conversation
app.css:285-286 already defines a global `.spin { animation: spin 1s linear
infinite; }` and `@keyframes spin`. Eight component-scoped CSS files (the six
Imaging pages plus DatabaseUpdate and FileUnblocker) each redefined both,
byte-identical. @Keyframes names aren't scoped and the scoped `.spin` rule is
identical to the global one, so the markup falls back to app.css with no visual
change. Removes 24 redundant lines.
Review finding #35 (2026-06-14 security/code-review audit).
…ge wipe) The five Settings pages each duplicated a status-message pattern (_message + _messageIsError + a ShowMessage method) whose auto-dismiss used an uncancelled `Task.Delay(...).ContinueWith(...)`. With no CTS, an earlier message's timer fired and wiped a newer message shown within the dismiss window. Extract one TransientNotice helper that holds the message/class/icon and backs the auto-dismiss with a single CancellationTokenSource cancelled on every Show(), so a stale timer can no longer clobber a current message. The five pages now bind their alert markup to the shared Notice and delegate their ShowMessage/ShowSaveMessage bodies to it (signatures and call sites unchanged; inline _message writes routed through Notice.Show/Clear). Helper is unit-tested, including the later-message-survives-earlier-timer case. The four Android dashboards already had a correct per-page CTS; their SetStatus will adopt this same helper when they are consolidated (finding #33). Review finding #34 (2026-06-14 security/code-review audit).
…eDashboard PixelDashboard, TabletDashboard and WatchDashboard were ~95% identical (~260 lines each) and their three .razor.css files were byte-identical. They now render a single shared DeviceDashboard component parameterized by Title/Icon/DeviceType/DeviceKind; the three pages are thin @page wrappers (852 lines removed). The shared scoped CSS lives once on the component (PixelDashboard.razor.css renamed to DeviceDashboard.razor.css; the two duplicates deleted). Status messages now use the TransientNotice helper from #34. Fixes the Watch dashboard's DeviceKind, which was wrongly "phone" (folds in #41) — the Watch wrapper passes "watch". The richer Google TV dashboard is a genuine superset (power/reboot/shizuku/ screensaver/timeout/launcher/projectivy) and stays standalone. Verified: build clean; new bUnit characterization tests render all three kinds' header/icon; full suite green; and a real run (Playwright, a registered phone device) showed the consolidated dashboard rendering correctly. The unrelated ScrcpyMirror.FocusIframe JS-interop circuit error seen in that run is pre-existing (unchanged component, identical usage) and only fires when ws-scrcpy-web isn't reachable — logged separately, out of scope here. Review finding #33 (2026-06-14 security/code-review audit).
bilbospocketses
enabled auto-merge (squash)
June 18, 2026 19:01
bilbospocketses
added a commit
that referenced
this pull request
Jun 18, 2026
…ent-scan test) (#66) * docs: changelog + technical-guide for PRs #64/#65 (audit #23/#24/#33/#34/#35) The #64 (CameraScan #23/#24) and #65 (UI dedup #33/#34/#35) findings landed without their CHANGELOG entries. Adds the [Unreleased] Changed/Fixed bullets, and updates the TECHNICAL_GUIDE Pages section (Phone/Tablet/Watch now render a shared DeviceDashboard) + the scanner test list (SubnetMathTests). * test(cameras): de-flake the concurrent-scan guard test StartScanAsync_ConcurrentStarts_StartExactlyOneScan used a fixed Task.Delay(150) before asserting one scan had started, which flaked on loaded CI runners where the 32 Task.Run bodies had not been scheduled yet (started == 0, seen on PR #66). Wait until a scan actually begins (poll up to ~10s) then settle briefly before asserting exactly one was admitted. Verified 4/4 stable locally.
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.
Continuing the 2026-06-14 audit tail (Item 43) — the UI duplication refactors. Follows the network/robustness cluster (#63) and the CameraScan pair (#64).
#35 — drop duplicate spin keyframes
Eight component-scoped CSS files (the six Imaging pages + DatabaseUpdate + FileUnblocker) each redefined
.spin { animation: spin 1s linear infinite; }and@keyframes spin, byte-identical to the globalapp.css:285-286.@keyframesnames aren't scoped and the scoped.spinis identical to the global rule, so the markup falls back to app.css with no visual change. Removes 24 redundant lines.#34 — shared TransientNotice helper (fixes a real bug)
The five Settings pages each duplicated a status-message pattern whose auto-dismiss used
Task.Delay(...).ContinueWith(...)with no CTS, so an earlier message's timer wiped a newer message shown within the dismiss window. Extracted oneTransientNoticehelper backing the auto-dismiss with a single CancellationTokenSource cancelled on everyShow(). The pages bind their alert markup to the sharedNoticeand delegate theirShowMessagebodies to it (signatures and call sites unchanged). Unit-tested, including the later-message-survives-earlier-timer case.The four Android dashboards already had a correct per-page CTS; their status now uses this same helper via #33's shared component.
Behavior note: two messages that previously persisted until the next action (DependencyManagement's "could not resolve download", Jellyfin's compose parse result) now auto-dismiss like every other notice — an intended consistency change.
#33 — consolidate Phone/Tablet/Watch dashboards
PixelDashboard/TabletDashboard/WatchDashboardwere ~95% identical (~260 lines each) with byte-identical scoped CSS. They now render a single sharedDeviceDashboardparameterized by Title/Icon/DeviceType/DeviceKind; the three pages are thin@pagewrappers (852 lines removed). The shared scoped CSS lives once on the component. The richer Google TV dashboard is a genuine superset (power/reboot/shizuku/screensaver/timeout/launcher/projectivy) and stays standalone.Also fixes the Watch dashboard's
DeviceKind, which was wrongly"phone"(folds in #41) —ScrcpyMirrorputs&deviceKind=on the stream URL, so it was a real wrong-hint bug.Testing & review
TransientNotice+DeviceDashboardbUnit characterization tests).ScrcpyMirror.FocusIframeJS interop — is pre-existing (unchanged component, identical usage) and only fires when ws-scrcpy-web is unreachable; tracked separately, out of scope here.Clear()), one is the intended auto-dismiss change noted above, one cosmetic (async-without-await, signatures preserved deliberately).Ledger: [[reference_control_menu_security_review]]. Remaining tail after this: #11/#13–#15/#17/#20/#21/#27–#29/#36 + #9/#10-rem + the MINORs #37–#58.