Skip to content

Fix FloatBar DPI-aware content sizing - #245

Merged
Finesssee merged 1 commit into
nesszer:mainfrom
klken88:fix/floatbar-dpi-layout
Aug 4, 2026
Merged

Fix FloatBar DPI-aware content sizing#245
Finesssee merged 1 commit into
nesszer:mainfrom
klken88:fix/floatbar-dpi-layout

Conversation

@klken88

@klken88 klken88 commented Jul 31, 2026

Copy link
Copy Markdown

Summary

Describe what changed and why.

Related issue

Fixes #

Affected areas

Check every area this PR changes or could affect:

  • Tray panel
  • Settings UI
  • Config file / settings persistence
  • CLI
  • Provider-specific behavior
  • Installer / release packaging
  • Startup / background behavior
  • Documentation
  • Other:

Validation

Hosted PR check runs on Blacksmith Windows when CI_BUDGET_MODE is not off (see .github/workflows/pr-check.yml and CONTEXT.md). Still run the local slice and list commands/results below. If a check is not relevant, say why.

UI / tray proof

For UI, tray, settings, or visual behavior changes, use CUA Driver for visual proof. If CUA Driver cannot be used, explain why and attach equivalent manual proof.

  • Not applicable
  • CUA Driver visual proof attached
  • CUA Driver could not be used; equivalent manual proof and explanation attached

Notes for reviewers

Call out risky areas, follow-up work, or anything reviewers should focus on.


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.

@Finesssee

Copy link
Copy Markdown
Collaborator

Thanks for the PR, I will review this.

@Finesssee

Copy link
Copy Markdown
Collaborator

Thermo-nuclear code-quality review (automated; @Finesssee for merge gating)

Verdict: Approve on code quality (minor nits). Block on process before merge — see the last section.

The change is small, direct, and behaviorally correct. Switching set_size from LogicalSize to PhysicalSize and multiplying the measured CSS px by devicePixelRatio in FloatBar.tsx is the right model for per-monitor DPI: the native window ends up exactly content-sized in physical pixels, and flex: none keeps the content from re-expanding to fill it. The DPI-1.5 test (rect 100×20 + padding 8 → 162×42 physical) exercises the new path and would fail under the old LogicalSize code. No new abstraction, no file-size concern, no scattered special-casing.

Nits (non-blocking):

  • window.rs: width.ceil().clamp(1.0, u32::MAX as f64) as u32 — the ceil() is redundant; FloatBar.tsx already sends Math.ceil(...). The lower clamp to 1.0 is the meaningful part (prevents a 0-size window); the u32::MAX upper clamp is belt-and-suspenders since f64 as u32 already saturates. Fine to keep as boundary defense — just noting it.
  • FloatBar.tsx: the new window.addEventListener("resize", resizeToContent) fires on any webview resize, not only DPI changes. The existing lastResizeRef ±1 dedup guard breaks the self-feedback loop (set_size → resize event → re-measure → same px → early return), so it's safe; the listener is just slightly broader than its stated purpose. No change needed.
  • No simpler reframing is visible — keeping LogicalSize and not multiplying would reintroduce the bug under WebView2 per-monitor scaling.

Process (required before merge per AGENTS.md):

  • PR body is the unfilled template ("Describe what changed and why" / "Fixes #" blank, validation checkboxes unchecked). Please fill it in.
  • This is a visual/DPI change. AGENTS.md requires CUA Driver proof after a fresh rebuild for UI/tray/float-bar PRs; none is attached. Please attach before/after screenshots at ≥1.0 and 1.5 scale (or state why CUA couldn't run + attach manual proof).
  • Confirm the float-bar window still pins theme(Some(Dark)) and other webviews don't flip under auto after the PhysicalSize change (the known shared-process-profile hazard noted in AGENTS.md).

@Finesssee
Finesssee merged commit 4622c89 into nesszer:main Aug 4, 2026
2 checks passed
@Finesssee

Copy link
Copy Markdown
Collaborator

Merged 🎉 Thank you @klken88 — clean fix. CUA-verified on a real desktop debug build (scale 1.0): the float bar renders at full size, sits at its content rect, and toggles cleanly (proof-245 artifacts: settings-menu.png, floatbar-window.png). Note for the record: the shipped math (CSS px × devicePixelRatio → PhysicalSize) is also covered by your jsdom test at 1.5 scale; a live 1.5-scale run needs a scaled monitor, but the 1.0 non-regression plus the unit test cover the change. Ships in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants