Skip to content

Fix tray popup height measurement#826

Merged
shanselman merged 1 commit into
openclaw:mainfrom
RBrid:user/rbrid/AppPopupHeight
Jun 26, 2026
Merged

Fix tray popup height measurement#826
shanselman merged 1 commit into
openclaw:mainfrom
RBrid:user/rbrid/AppPopupHeight

Conversation

@RBrid

@RBrid RBrid commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • measure the tray popup against its final client width instead of estimating content height
  • apply pixel-exact AppWindow sizing so wrapped rows determine the required height deterministically
  • add coverage for DIP-to-pixel conversion and the tray sizing contract

Validation

  • .\build.ps1
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore
  • Copilot autoreview clean
  • Hanselman review: no blocking findings

Screenshots

Before fix:
image

After fix:
image

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 11:03 PM ET / 03:03 UTC.

Summary
The PR changes tray popup sizing to measure the final client width, resize the AppWindow in pixels, and add shared/tray sizing regression coverage.

Reproducibility: no. I did not establish a high-confidence local Windows tray reproduction in this read-only Linux environment. Current-main source still has the old measurement path, and the before/after screenshots provide credible medium-confidence evidence for the visible clipping problem.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 5 files, +120/-33. The patch is focused but crosses shared sizing math, WinUI tray runtime code, and two test projects.
  • Proof artifacts: 2 screenshots inspected. The images show the before/after top-level tray popup behavior this PR changes.
  • Current checks: 2 passing, 4 in progress, 2 skipped. Repository validation is still the remaining merge gate for the current head.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Wait for current-head repository validation to complete before merge.
  • Optionally capture a cascading flyout with wrapped rows if maintainers want visual proof beyond the top-level popup.

Mantis proof suggestion
A native visual pass would usefully verify both top-level and cascading WinUI tray popup sizing after the runtime measurement change. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: open the tray popup with wrapped rows and a cascading flyout, then verify both windows size without clipping.

Risk before merge

  • [P1] The screenshots directly prove the top-level tray popup improvement, but cascading flyouts share the SizeToContent path and are not separately shown.
  • [P1] Current-head repository validation is still in progress, so maintainer merge review should wait for those checks to settle.

Maintainer options:

  1. Land After Current-Head Validation (recommended)
    Maintainers can accept the remaining visual proof gap once the Build and Test jobs complete successfully on the current head.
  2. Request Cascading Flyout Proof
    Ask for a screenshot, recording, or Mantis capture showing a wrapped-row cascading flyout before merge.

Next step before merge

  • No automated repair lane is needed because there is no discrete actionable patch defect; the remaining action is maintainer merge review and current-head validation.

Security
Cleared: The diff changes local WinUI sizing interop and tests only, with no dependency, CI, packaging, credential, network, or supply-chain surface changed.

Review details

Best possible solution:

Land the focused pixel-exact tray popup sizing fix after current-head validation settles, with optional native visual proof for cascading flyouts if maintainers want extra UI confidence.

Do we have a high-confidence way to reproduce the issue?

No, I did not establish a high-confidence local Windows tray reproduction in this read-only Linux environment. Current-main source still has the old measurement path, and the before/after screenshots provide credible medium-confidence evidence for the visible clipping problem.

Is this the best way to solve the issue?

Yes. Measuring the full RootGrid at the final client width and applying pixel AppWindow sizing is the narrow maintainable fix for wrapped content determining height; cascade-specific visual proof would improve confidence but does not change the implementation direction.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 84d2e6ab80c8.

Label changes

Label justifications:

  • P2: This is a normal-priority user-visible tray UI bug fix with limited blast radius and focused regression coverage.
  • merge-risk: 🚨 other: The diff changes native tray popup sizing behavior, and cascading flyouts share the changed sizing path without direct visual proof.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Before/after screenshots directly show the top-level tray popup no longer clipping the Close row after the sizing change.
  • proof: sufficient: Contributor real behavior proof is sufficient. Before/after screenshots directly show the top-level tray popup no longer clipping the Close row after the sizing change.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Before/after screenshots directly show the top-level tray popup no longer clipping the Close row after the sizing change.
Evidence reviewed

What I checked:

  • AGENTS policy read: The full repository AGENTS.md was read; its build/shared-test/tray-test validation expectations apply, but this cleanup review remained read-only and did not run artifact-producing validation. (AGENTS.md:1, 84d2e6ab80c8)
  • Current main still has old measurement path: Current main measures MenuPanel at widthViewUnits, stores height in view units, and calls SetWindowSize, so the PR's central behavior is not already implemented on main. (src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs:953, 84d2e6ab80c8)
  • PR implements pixel-exact final-client-width sizing: At the PR head, SizeToContent computes target pixel width, subtracts non-client width, measures RootGrid at the resulting client width, computes pixel height, and calls ResizeWindowToPixelSize. (src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs:958, eaf9ab57de74)
  • Focused regression coverage added: The PR adds a markup contract asserting GetClientRect, RootGrid.Measure at the final width, pixel resize, and removal of the old MenuPanel.Measure string. (tests/OpenClaw.Tray.Tests/TrayMenuWindowMarkupTests.cs:112, eaf9ab57de74)
  • DIP-to-pixel helper coverage added: The PR adds ConvertViewUnitsToPixels and shared/tray tests for standard DPI, scaled DPI rounding up, and invalid/non-positive inputs. (src/OpenClaw.Shared/MenuSizingHelper.cs:18, eaf9ab57de74)
  • Real behavior screenshots inspected: The before screenshot shows the Close row clipped at the bottom of the top-level tray menu; the after screenshot shows the full top-level menu including Close visible after the sizing change. (eaf9ab57de74)

Likely related people:

  • shanselman: Git history shows Scott Hanselman authored recent DPI menu sizing and tray submenu/flyout work in the same window surface. (role: recent tray sizing and flyout contributor; confidence: high; commits: 32830a0527a2, e63228e87e4b, 02cd3b3bb61c; files: src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs, src/OpenClaw.Shared/MenuSizingHelper.cs)
  • NichUK: Merged history for the tray overflow fix introduced MenuSizingHelper and work-area clamping for tray menu height. (role: introduced related overflow sizing behavior; confidence: high; commits: b511e509e049; files: src/OpenClaw.Shared/MenuSizingHelper.cs, src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs)
  • ranjeshj: Recent history changed tray subflyout reuse and measurement reliability in the same TrayMenuWindow code path. (role: recent adjacent tray flyout contributor; confidence: medium; commits: 2ca192d0dabc; files: src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs)
  • kenehong: History shows several tray layout, icon, permissions, and flyout UX refinements before this sizing change. (role: adjacent tray UX contributor; confidence: medium; commits: 8abe93a04732, bb71ae04cc64, 259f270b3837; files: src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs)
  • RBrid: Beyond authoring this PR, prior merged companion application refactoring touched the tray menu window surface. (role: prior adjacent contributor; confidence: medium; commits: f2fa038bd080; files: src/OpenClaw.Tray.WinUI/Windows/TrayMenuWindow.xaml.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jun 25, 2026
@RBrid RBrid marked this pull request as ready for review June 25, 2026 23:52
@shanselman shanselman force-pushed the user/rbrid/AppPopupHeight branch from eacd375 to 8f601c5 Compare June 26, 2026 02:24
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. label Jun 26, 2026
@shanselman shanselman force-pushed the user/rbrid/AppPopupHeight branch from 8f601c5 to 0472422 Compare June 26, 2026 02:43
Measure the tray popup against its final client width and apply pixel-exact window sizing so wrapped content determines height deterministically.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman shanselman force-pushed the user/rbrid/AppPopupHeight branch from 0472422 to eaf9ab5 Compare June 26, 2026 03:00
@shanselman shanselman merged commit 5602c96 into openclaw:main Jun 26, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants