Refactor window handling, improve theme support, and update dependencies#1807
Refactor window handling, improve theme support, and update dependencies#1807ItsEeleeya wants to merge 166 commits into
Conversation
The header height now matches that of windows with Toolbars on macOS 26
…ier + fix onboarding decorations
The header height now matches that of windows with Toolbars on macOS 26
…ier + fix onboarding decorations
…a/Cap into next-base-improvements
…lugin_window_state's deny list
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
Co-authored-by: tembo[bot] <208362400+tembo[bot]@users.noreply.github.com>
| .find(|(path, _)| path == project_path) | ||
| .map(|(_, id)| *id) | ||
| .unwrap_or_else(|| { | ||
| panic!("editor window id should be registered before calling CapWindow::id ({project_path:?})") |
There was a problem hiding this comment.
Tiny thing: this panic! will crash the app if an Editor window ever hits CapWindow::id before registration (race / unexpected call order). If this is strictly a debug-time invariant, maybe debug_assert! + a logged fallback (or returning Option/Result from id) would be safer for release builds.
| return; | ||
| }; | ||
| // SAFETY: Tauri runs this on the main thread | ||
| let mtm = unsafe { MainThreadMarker::new_unchecked() }; |
There was a problem hiding this comment.
Since this closure is already run via run_on_main_thread, could we avoid new_unchecked() here and use the safe constructor?
| let mtm = unsafe { MainThreadMarker::new_unchecked() }; | |
| let mtm = MainThreadMarker::new().expect("Running on main thread"); |
…electOverlay and related code
| ctx.font = "16px system-ui"; | ||
| ctx.fillText("😀", 0, 24); | ||
| } catch {} | ||
| } catch { } |
There was a problem hiding this comment.
Minor formatting nit: catch { } looks like it’ll fight Prettier/linting (usually catch {}).
| } catch { } | |
| } catch {} |
| @@ -4996,7 +4841,6 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||
| hotkeys::OnEscapePress, | |||
| upload::UploadProgressEvent, | |||
| import::VideoImportProgress, | |||
There was a problem hiding this comment.
Since SetCaptureAreaPending is removed from the exported events here, can you also regenerate/update the generated bindings in apps/desktop/src/utils/tauri.ts? It still declares SetCaptureAreaPending and maps setCaptureAreaPending: "set-capture-area-pending", which will be stale / potentially break listeners at runtime.
…trols overlay (unsupported for now) + fmt
| RecordingOptionsProvider, | ||
| useRecordingOptions, | ||
| } from "./(window-chrome)/OptionsContext"; | ||
| import { getCurrentWindow } from "@tauri-apps/api/window"; |
There was a problem hiding this comment.
Looks like getCurrentWindow is unused here (only imported). If so, can drop it to keep noUnusedLocals happy.
| import { getCurrentWindow } from "@tauri-apps/api/window"; |
| props.useBackdropFilter, | ||
| "bg-gray-3 opacity-80": !props.useBackdropFilter, | ||
| "bg-gray-3 not-contrast-more:opacity-80": | ||
| !props.useBackdropFilter, |
There was a problem hiding this comment.
Not sure not-contrast-more: is a supported variant here. If the goal is “dim unless high-contrast”, this keeps it explicit:
| !props.useBackdropFilter, | |
| "bg-gray-3 opacity-80 contrast-more:opacity-100": | |
| !props.useBackdropFilter, |
| CapWindow::ScreenshotEditor { path } => { | ||
| let state = manager.state::<ScreenshotEditorWindowIds>(); | ||
| let s = state.ids.lock().unwrap(); | ||
| let id = s.iter().find(|(p, _)| p == path).unwrap().1; |
There was a problem hiding this comment.
Minor, but since the Editor branch now has a clearer invariant failure message, it’d be nice to avoid the bare unwrap() here too (makes crash reports much easier to reason about).
| let id = s.iter().find(|(p, _)| p == path).unwrap().1; | |
| let id = s | |
| .iter() | |
| .find(|(p, _)| p == path) | |
| .expect("screenshot editor window id should be registered before calling CapWindow::id") | |
| .1; |
| let _ = MACOS_NATIVE_TERMINATE_APP.set(app.clone()); | ||
|
|
||
| app.run_on_main_thread(|| { | ||
| let mtm = MainThreadMarker::new().expect("Running on main"); |
There was a problem hiding this comment.
Minor, but I’d avoid a hard panic here (even if the invariant “should” hold) and just bail with a log.
| let mtm = MainThreadMarker::new().expect("Running on main"); | |
| let Some(mtm) = MainThreadMarker::new() else { | |
| tracing::error!("Expected to be on the main thread"); | |
| return; | |
| }; |
| _ => return, | ||
| }; | ||
|
|
||
| let _ = app.opener().open_url(url, None::<&str>); |
There was a problem hiding this comment.
Since this is user-triggered, might be worth at least logging failures so it’s debuggable.
| let _ = app.opener().open_url(url, None::<&str>); | |
| if let Err(e) = app.opener().open_url(url, None::<&str>) { | |
| tracing::warn!("Failed to open URL {url}: {e}"); | |
| } |
| import "unfonts.css"; | ||
| import "./styles/theme.css"; | ||
|
|
||
| import { createEventListener } from "@solid-primitives/event-listener"; |
There was a problem hiding this comment.
Looks like createEventListener is unused in this file now; I think it can be dropped to avoid a TS unused import warning.
2 participants
Refactor window handling, improve theme support, and modernize native integration
Overview
This PR refactors a few things with small improvements as a base for upcoming PRs. The changes modernize how windows are revealed, streamline macOS integration with native AppKit APIs, and better theme handling.
Key Changes
Window Management Refactoring
AutoRevealWindowOnReady: Automatically shows windows once content is loadedRevealWindowWithSuspense: Defers visibility for windows like Settings until routing and child mounting completesShowCapWindow→CapWindowthroughout the codebaseAppDelegateas Tauri now supports custom traffic lights insetdata-tauri-drag-region="deep"attribute (eliminates scattered manual markers)onMouseDown={showCropOptionsMenu}in Editor crop section withonClickas keyboard/touch fallbackTheme/Appearance Improvements
AppTheme→Appearance(with serde fallback for backward compatibility)Code Organization
src-tauri/src/display_utils.rsout ofwindows.rsMonitorExtandCursorMonitorInfotypes organizedpanel_manager.rs: Now macOS-only with streamlined logicmenu.rsmoduleNative Platform Integration
objc2ecosystem crates (app-kit, foundation, permissions)objc2-app-kitversionWebviewWindowExttrait withwith_nswindow_on_main()utility for direct AppKit window accesstauri_plugin_window_statedenylist)UI/UX Enhancements
scale-50class)⌘+,shortcutDependencies & Plugins
tauri-plugin-prevent-default: Enables default context menu only during developmentTechnical Notes
objc2crates instead of legacy objc/cocoaSuggested changelog entires:
⌘+,shortcut.Greptile Summary
This PR refactors desktop window handling and native integration. The main changes are:
ShowCapWindowtoCapWindowacross Rust and generated bindings.themetoappearancewith persisted-store compatibility.Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
appearancewhile accepting the old persisted key.Reviews (3): Last reviewed commit: "Merge branch 'main' into pr/1807" | Re-trigger Greptile
Context used: