-
Notifications
You must be signed in to change notification settings - Fork 153
fix(panel): position panel under tray icon on all entry paths #353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d114de5
ae43e80
995a7fd
b3555aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,64 @@ use tauri_nspanel::{ | |
| CollectionBehavior, ManagerExt, PanelLevel, StyleMask, WebviewWindowExt, tauri_panel, | ||
| }; | ||
|
|
||
| fn monitor_contains_physical_point( | ||
| origin_x: f64, | ||
| origin_y: f64, | ||
| width: f64, | ||
| height: f64, | ||
| point_x: f64, | ||
| point_y: f64, | ||
| ) -> bool { | ||
| point_x >= origin_x | ||
| && point_x < origin_x + width | ||
| && point_y >= origin_y | ||
| && point_y < origin_y + height | ||
| } | ||
|
|
||
| unsafe fn set_panel_frame_top_left(panel: &tauri_nspanel::NSPanel, x: f64, y: f64) { | ||
| let point = tauri_nspanel::NSPoint::new(x, y); | ||
| let _: () = objc2::msg_send![panel, setFrameTopLeftPoint: point]; | ||
| } | ||
|
|
||
| fn set_panel_top_left_immediately( | ||
| window: &tauri::WebviewWindow, | ||
| app_handle: &AppHandle, | ||
| panel_x: f64, | ||
| panel_y: f64, | ||
| primary_logical_h: f64, | ||
| ) { | ||
| let Ok(panel_handle) = app_handle.get_webview_panel("main") else { | ||
| return; | ||
| }; | ||
|
|
||
| let target_x = panel_x; | ||
| let target_y = primary_logical_h - panel_y; | ||
|
|
||
| if objc2_foundation::MainThreadMarker::new().is_some() { | ||
| unsafe { | ||
| set_panel_frame_top_left(panel_handle.as_panel(), target_x, target_y); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| let (tx, rx) = std::sync::mpsc::channel(); | ||
| let panel_handle = panel_handle.clone(); | ||
|
|
||
| if let Err(error) = window.run_on_main_thread(move || { | ||
| unsafe { | ||
| set_panel_frame_top_left(panel_handle.as_panel(), target_x, target_y); | ||
| } | ||
| let _ = tx.send(()); | ||
| }) { | ||
| log::warn!("Failed to position panel on main thread: {}", error); | ||
| return; | ||
| } | ||
|
|
||
| if rx.recv().is_err() { | ||
| log::warn!("Failed waiting for panel position on main thread"); | ||
| } | ||
| } | ||
|
|
||
| /// Macro to get existing panel or initialize it if needed. | ||
| /// Returns Option<Panel> - Some if panel is available, None on error. | ||
| macro_rules! get_or_init_panel { | ||
|
|
@@ -30,10 +88,31 @@ macro_rules! get_or_init_panel { | |
| // Export macro for use in other modules | ||
| pub(crate) use get_or_init_panel; | ||
|
|
||
| /// Show the panel (initializing if needed). | ||
| /// Retrieve the tray icon rect and position the panel beneath it. | ||
| /// No-ops gracefully if the tray icon or its rect is unavailable. | ||
| fn position_panel_from_tray(app_handle: &AppHandle) { | ||
| let Some(tray) = app_handle.tray_by_id("tray") else { | ||
| log::debug!("position_panel_from_tray: tray icon not found"); | ||
| return; | ||
| }; | ||
| match tray.rect() { | ||
| Ok(Some(rect)) => { | ||
| position_panel_at_tray_icon(app_handle, rect.position, rect.size); | ||
| } | ||
| Ok(None) => { | ||
| log::debug!("position_panel_from_tray: tray rect not available yet"); | ||
| } | ||
| Err(e) => { | ||
| log::warn!("position_panel_from_tray: failed to get tray rect: {}", e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Show the panel (initializing if needed), positioned under the tray icon. | ||
| pub fn show_panel(app_handle: &AppHandle) { | ||
| if let Some(panel) = get_or_init_panel!(app_handle) { | ||
| panel.show_and_make_key(); | ||
| position_panel_from_tray(app_handle); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -50,6 +129,7 @@ pub fn toggle_panel(app_handle: &AppHandle) { | |
| } else { | ||
| log::debug!("toggle_panel: showing panel"); | ||
| panel.show_and_make_key(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. src-tauri/src/panel.rs:74: Severity: medium Other Locations
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| position_panel_from_tray(app_handle); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -115,17 +195,6 @@ pub fn position_panel_at_tray_icon( | |
| ) { | ||
| let window = app_handle.get_webview_window("main").unwrap(); | ||
|
|
||
| // Tray icon events on macOS report coordinates in a hybrid physical space where | ||
| // each monitor region uses its own scale (logical_pos × scale = physical origin). | ||
| // On mixed-DPI setups this creates overlapping regions, making it impossible to | ||
| // reliably determine the correct monitor from tray coordinates alone. | ||
| // | ||
| // Instead, we use NSEvent::mouseLocation() which returns the cursor position in | ||
| // macOS's unified logical (point) coordinate space — always unambiguous regardless | ||
| // of how many monitors or scale factors are involved. We find which monitor | ||
| // contains the cursor, then convert the tray icon's physical coordinates to | ||
| // logical coordinates within that monitor. | ||
|
|
||
| let (icon_phys_x, icon_phys_y) = match &icon_position { | ||
| Position::Physical(pos) => (pos.x as f64, pos.y as f64), | ||
| Position::Logical(pos) => (pos.x, pos.y), | ||
|
|
@@ -135,12 +204,6 @@ pub fn position_panel_at_tray_icon( | |
| Size::Logical(s) => (s.width, s.height), | ||
| }; | ||
|
|
||
| // Get the cursor's logical position via NSEvent — this is in macOS's flipped | ||
| // coordinate system (origin at bottom-left of primary screen). | ||
| let mouse_logical = objc2_app_kit::NSEvent::mouseLocation(); | ||
|
|
||
| // Convert from macOS bottom-left origin to top-left origin used by Tauri. | ||
| // Primary screen height (in points) defines the flip axis. | ||
| let monitors = window.available_monitors().expect("failed to get monitors"); | ||
| let primary_logical_h = window | ||
| .primary_monitor() | ||
|
|
@@ -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); | ||
|
|
||
| let mouse_x = mouse_logical.x; | ||
| let mouse_y = primary_logical_h - mouse_logical.y; | ||
|
|
||
| // Find the monitor containing the cursor in logical space (no ambiguity). | ||
| let mut found_monitor = None; | ||
| for m in &monitors { | ||
| let pos = m.position(); | ||
| let scale = m.scale_factor(); | ||
| let logical_w = m.size().width as f64 / scale; | ||
| let logical_h = m.size().height as f64 / scale; | ||
|
|
||
| let logical_x = pos.x as f64 / scale; | ||
| let logical_y = pos.y as f64 / scale; | ||
| let x_in = mouse_x >= logical_x && mouse_x < logical_x + logical_w; | ||
| let y_in = mouse_y >= logical_y && mouse_y < logical_y + logical_h; | ||
|
|
||
| if x_in && y_in { | ||
| found_monitor = Some(m.clone()); | ||
| break; | ||
| } | ||
| } | ||
| let icon_center_x = icon_phys_x + (icon_phys_w / 2.0); | ||
| let icon_center_y = icon_phys_y + (icon_phys_h / 2.0); | ||
|
|
||
| let found_monitor = monitors.iter().find(|monitor| { | ||
| let origin = monitor.position(); | ||
| let size = monitor.size(); | ||
| monitor_contains_physical_point( | ||
| origin.x as f64, | ||
| origin.y as f64, | ||
| size.width as f64, | ||
| size.height as f64, | ||
| icon_center_x, | ||
| icon_center_y, | ||
| ) | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mixed-DPI monitor detection may select wrong monitorLow 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 Reviewed by Cursor Bugbot for commit 995a7fd. Configure here. |
||
|
|
||
| let monitor = match found_monitor { | ||
| Some(m) => m, | ||
| Some(m) => m.clone(), | ||
| None => { | ||
| log::warn!( | ||
| "No monitor found for cursor at ({:.0}, {:.0}), using primary", | ||
| mouse_x, | ||
| mouse_y | ||
| "No monitor found for tray rect center at ({:.0}, {:.0}), using primary", | ||
| icon_center_x, | ||
| icon_center_y | ||
| ); | ||
| match window.primary_monitor() { | ||
| Ok(Some(m)) => m, | ||
|
|
@@ -187,16 +244,13 @@ pub fn position_panel_at_tray_icon( | |
| }; | ||
|
|
||
| let target_scale = monitor.scale_factor(); | ||
| let mon_logical_x = monitor.position().x as f64; | ||
| let mon_logical_y = monitor.position().y as f64; | ||
|
|
||
| // Convert tray icon physical coords to logical within the identified monitor. | ||
| // Physical origin of this monitor in the hybrid tray coordinate space: | ||
| let phys_origin_x = mon_logical_x * target_scale; | ||
| let phys_origin_y = mon_logical_y * target_scale; | ||
| let mon_phys_x = monitor.position().x as f64; | ||
| let mon_phys_y = monitor.position().y as f64; | ||
| let mon_logical_x = mon_phys_x / target_scale; | ||
| let mon_logical_y = mon_phys_y / target_scale; | ||
|
|
||
| let icon_logical_x = mon_logical_x + (icon_phys_x - phys_origin_x) / target_scale; | ||
| let icon_logical_y = mon_logical_y + (icon_phys_y - phys_origin_y) / target_scale; | ||
| let icon_logical_x = mon_logical_x + (icon_phys_x - mon_phys_x) / target_scale; | ||
| let icon_logical_y = mon_logical_y + (icon_phys_y - mon_phys_y) / target_scale; | ||
| let icon_logical_w = icon_phys_w / target_scale; | ||
| let icon_logical_h = icon_phys_h / target_scale; | ||
|
|
||
|
|
@@ -220,5 +274,5 @@ pub fn position_panel_at_tray_icon( | |
| let nudge_up: f64 = 6.0; | ||
| let panel_y = icon_logical_y + icon_logical_h - nudge_up; | ||
|
|
||
| let _ = window.set_position(tauri::LogicalPosition::new(panel_x, panel_y)); | ||
| set_panel_top_left_immediately(&window, app_handle, panel_x, panel_y, primary_logical_h); | ||
| } | ||


There was a problem hiding this comment.
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.