Skip to content

fix(panel): position panel under tray icon on all entry paths#353

Open
robinebers wants to merge 4 commits intomainfrom
robinebers/fix-panel-positioning
Open

fix(panel): position panel under tray icon on all entry paths#353
robinebers wants to merge 4 commits intomainfrom
robinebers/fix-panel-positioning

Conversation

@robinebers
Copy link
Copy Markdown
Owner

@robinebers robinebers commented Apr 7, 2026

Description

This PR fixes a long-standing bug where the panel wouldn't be positioned correctly unless you click on the TrayIcon first.

Type of Change

  • Bug fix
  • New feature
  • New provider plugin
  • Documentation
  • Performance improvement
  • Other (describe below)

Testing

  • I ran bun run build and it succeeded
  • I ran bun run test and all tests pass
  • I tested the change locally with bun tauri dev

Checklist

  • I read CONTRIBUTING.md
  • My PR targets the main branch
  • I did not introduce new dependencies without justification

Note

Medium Risk
Touches macOS window positioning logic and introduces an unsafe Objective-C call dispatched onto the main thread, which could cause crashes/hangs or misplacement on edge display configurations if incorrect.

Overview
Ensures the panel is always positioned under the tray icon across all entry paths by fetching TrayIcon::rect() on show/toggle and no-oping gracefully when unavailable.

Updates monitor selection/coordinate conversion to use the tray rect’s physical center to pick the correct display on multi-monitor setups, then computes logical panel coordinates from that monitor.

Replaces window.set_position(...) with a main-thread NSPanel setFrameTopLeftPoint: call (with a blocking wait when off-main-thread) to apply positioning immediately and reduce first-open flicker.

Reviewed by Cursor Bugbot for commit b3555aa. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes macOS panel placement so it always opens under the tray icon from any entry point. Also targets the correct monitor on multi‑display setups and removes first‑open flicker with synchronous frame updates.

  • Bug Fixes
    • Added position_panel_from_tray; now called from show_panel/toggle_panel to read TrayIcon::rect() and reposition immediately after showing (no‑op if missing).
    • Corrected monitor selection by using the tray rect center in physical space and converting coordinates correctly, fixing multi‑monitor/mixed‑DPI placement.
    • Replaced tao’s async window.set_position() with a main‑thread NSPanel setFrameTopLeftPoint: call plus a blocking wait to apply the move synchronously and avoid flicker.

Written for commit b3555aa. Summary will update on new commits.

… flicker

The global shortcut and tray menu actions (Show Stats, Settings, About)
never called position_panel_at_tray_icon, so the panel appeared at macOS's
default window position on first open. Additionally, the existing
positioning used tao's async set_position (exec_async), causing a visible
flicker when show_and_make_key fired before the queued move was applied.

Fix: call setFrameTopLeftPoint: synchronously via msg_send on the NSPanel
before showing, and add positioning to toggle_panel and show_panel.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added the rust Pull requests that update rust code label Apr 7, 2026
@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Apr 7, 2026

🤖 Augment PR Summary

Summary: Fix macOS panel positioning so it consistently opens under the tray icon across all entry paths.

Key changes:

• Add position_panel_from_tray to fetch TrayIcon::rect() and delegate to position_panel_at_tray_icon.

• Call tray-based positioning before show_and_make_key() in show_panel and toggle_panel.

• Replace tao/tauri async positioning with a synchronous NSPanel setFrameTopLeftPoint: call to avoid flicker.

Notes: Uses a CoreGraphics screen-height flip to map top-left logical coords into AppKit screen coordinates.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

} else {
log::debug!("toggle_panel: showing panel");
position_panel_from_tray(app_handle);
panel.show_and_make_key();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src-tauri/src/panel.rs:74: toggle_panel now repositions via position_panel_from_tray(), but position_panel_at_tray_icon() picks the target monitor via NSEvent::mouseLocation(); if the cursor is on a different monitor than the tray icon, the physical→logical conversion can use the wrong scale/offset and misplace the panel.

Severity: medium

Other Locations
  • src-tauri/src/panel.rs:56

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is intentional. If there's a cursor there, I guess the display exists lol.


if let Ok(panel_handle) = app_handle.get_webview_panel("main") {
let ns_panel = panel_handle.as_panel();
let screen_height = CGDisplayPixelsHigh(CGMainDisplayID()) as f64;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src-tauri/src/panel.rs:259: CGDisplayPixelsHigh(CGMainDisplayID()) is a pixel height for the main display, but panel_x/panel_y are computed in logical points (and may refer to a non-primary monitor), so screen_height - panel_y can produce incorrect Y coordinates on Retina/multi-monitor setups.

Severity: high

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

let ns_panel = panel_handle.as_panel();
let screen_height = CGDisplayPixelsHigh(CGMainDisplayID()) as f64;
let point = tauri_nspanel::NSPoint::new(panel_x, screen_height - panel_y);
let _: () = objc2::msg_send![ns_panel, setFrameTopLeftPoint: point];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src-tauri/src/panel.rs:261: The direct objc2::msg_send![..., setFrameTopLeftPoint:] call must run on the AppKit main thread; if position_panel_at_tray_icon() is ever invoked from a non-main callback (e.g., global shortcut), this risks crashes/undefined behavior.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src-tauri/src/panel.rs">

<violation number="1" location="src-tauri/src/panel.rs:259">
P1: The new NSPanel positioning mixes pixel and logical-point coordinate spaces, which can misplace the panel vertically on high-DPI displays.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 995a7fd. Configure here.

icon_center_x,
icon_center_y,
)
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed-DPI monitor detection may select wrong monitor

Low Severity

The removed code explicitly documented that tray icon coordinates live in a "hybrid physical space where each monitor region uses its own scale," creating overlapping physical regions on mixed-DPI setups. It warned this made it "impossible to reliably determine the correct monitor from tray coordinates alone" and used NSEvent::mouseLocation() in logical space as a workaround. The new code does exactly what the old code warned against — using monitors.iter().find() with the tray icon center in physical coordinates. On a mixed-DPI setup (e.g., Retina + non-Retina), physical bounds can overlap, and find() may match the wrong monitor first, yielding an incorrect target_scale and mispositioned panel. In practice, the tray is almost always on the primary display, which is likely (but not guaranteed) to be first in the iterator, so the risk is low.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 995a7fd. Configure here.

@validatedev validatedev requested a review from Copilot April 7, 2026 20:20
@validatedev
Copy link
Copy Markdown
Collaborator

@codex review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes macOS panel placement so the panel consistently appears under the tray icon regardless of how it’s opened (tray click vs. other entry paths), including better handling for multi-monitor setups and reducing first-open flicker by moving the panel synchronously.

Changes:

  • Add position_panel_from_tray() and call it from show_panel() / toggle_panel() to position on every show path.
  • Update monitor selection to use the tray rect center and convert tray rect physical coordinates into monitor-relative logical coordinates.
  • Replace window.set_position(...) with a main-thread NSPanel setFrameTopLeftPoint: call to apply positioning immediately.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +59 to +60
if rx.recv().is_err() {
log::warn!("Failed waiting for panel position on main thread");
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rx.recv() blocks indefinitely while waiting for the main-thread reposition closure. If the app is shutting down or the main thread can’t service the queued task, this can hang the caller (e.g., global shortcut handler). Use a bounded wait (e.g., recv_timeout) and/or ensure the closure always runs before blocking, and log/continue if it times out.

Suggested change
if rx.recv().is_err() {
log::warn!("Failed waiting for panel position on main thread");
match rx.recv_timeout(std::time::Duration::from_millis(500)) {
Ok(()) => {}
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
log::warn!("Timed out waiting for panel position on main thread");
}
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
log::warn!("Failed waiting for panel position on main thread");
}

Copilot uses AI. Check for mistakes.
Comment on lines 208 to 213
@@ -149,35 +212,29 @@ pub fn position_panel_at_tray_icon(
.map(|m| m.size().height as f64 / m.scale_factor())
.unwrap_or(0.0);
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

primary_logical_h defaults to 0.0 when primary_monitor() is unavailable, but it’s now used to flip coordinates for the Cocoa setFrameTopLeftPoint: call. If this happens, target_y becomes incorrect and the panel will be positioned off-screen. Consider early-returning (or falling back to an NSScreen-derived height) when the primary monitor can’t be determined, rather than proceeding with 0.0.

Copilot uses AI. Check for mistakes.
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

Labels

rust Pull requests that update rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants