feat: Quick Preview click-outside-to-close and single-instance toggle - #5
Conversation
Click-outside (FR-002): - Layer-shell path anchors the surface to all four edges; a transparent backdrop Box (gtk::Overlay main child) closes the window on any click, while the centered 900x700 panel holds the viewer as an overlay sibling, so clicks on the image never reach the backdrop gesture and drag-select / middle-drag pan keep working. - Window CSS makes the anchored surface transparent; the panel restores an opaque background with rounded corners. - Fallback path (no layer shell) closes on focus loss with a was-active latch instead of the originally sketched GestureClick, which would also have fired for clicks inside the preview (PHASED_PLAN amended). Single-instance toggle (FR-002): - GApplication now runs with HANDLES_COMMAND_LINE; the invoking process resolves everything (clap, stdin, canonicalization) and forwards a canonical --key=value argv (new ipc module). Single-token framing is deliberate: GLib's local GOptionContext strips a bare -- separator even in pass-through mode. - Dispatch in the primary: same-file preview invocation toggles the preview closed, different-file replaces its content, full-viewer invocations open a new window (closing any preview first, since the anchored keyboard-exclusive overlay would block it). - This retires the run_with_args(&[]) workaround: GLib only ever sees the sanitized argv we build.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
Summary by CodeRabbit
WalkthroughAdds an argv-based IPC codec for single-instance forwarding, reworks the GTK application entrypoint to use ChangesSingle-instance IPC and Quick Preview dismissal
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as New CLI invocation
participant Primary as Primary instance
participant Dispatch as dispatch()
participant State as AppState
participant Preview as quick preview window
CLI->>Primary: connect_command_line(argv)
Primary->>Primary: ipc::from_argv(argv)
Primary->>Dispatch: dispatch(opts)
Dispatch->>State: check existing PreviewHandle
alt existing preview matches requested file
Dispatch->>Preview: close()
Preview->>State: clear handle on close-request
else new preview or different file
Dispatch->>Preview: load file / present()
Preview->>State: store handle
end
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e4f61ad74
ℹ️ 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".
| window.close(); | ||
| } else { | ||
| // Explicit request for a different file: show it. | ||
| controller.load_file(&opts.file); |
There was a problem hiding this comment.
When an existing Quick Preview is reused for a different file, this path only calls load_file, but ViewerController keeps the ocr_lang captured by the first preview and start_ocr reads that stored value. A later invocation such as quickview --quick-preview --lang deu other.png while an English preview is open will still OCR other.png with English, so the documented CLI language override is ignored for all replacement previews until the window is closed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — verified: the replace path only called load_file while start_ocr reads the ocr_lang captured at controller construction, so a forwarded --lang was ignored until the preview closed. Fixed in f4716da: ViewerController::set_ocr_lang is now applied before load_file in the reuse branch (in-flight jobs keep the language they started with, consistent with the existing job-id supersession).
A different-file invocation reused the ViewerController's original ocr_lang, silently ignoring the new invocation's --lang. Found by Codex review on PR #5.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4716da9bc
ℹ️ 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".
| panel.set_size_request(PANEL_WIDTH, PANEL_HEIGHT); | ||
| panel.add_css_class("qv-preview-panel"); | ||
| panel.append(&viewer.widget()); |
There was a problem hiding this comment.
For images whose natural size is larger than 900×700, this panel is still allowed to grow: set_size_request only establishes a minimum size, and GtkOverlay positions centered overlay children at their natural size. Because viewer.widget() measures from the loaded image dimensions, a large screenshot/photo can make the layer-shell panel cover or overflow the whole surface, so the image is cropped at 1:1 and clicks that should hit the backdrop instead hit the viewer. Force the overlay child allocation to PANEL_WIDTH×PANEL_HEIGHT (or otherwise cap/clip the viewer) rather than relying on a size request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — confirmed: the canvas's measure reports the image's natural size (natural_size_for_measure with for_size = -1), and gtk::Overlay allocates centered overlay children at natural size, so any image over 900×700 swallowed the surface. Fixed in follow-up PR #8 with nested adw::Clamps capping the panel's maximum at 900×700 (the size request already held the minimum).
niri, Hyprland, and Sway snippets using the newest-screenshot recipe (compositors don't know the file-manager selection; see caveat C). Documents the toggle behavior (one bind opens and closes, PR #5) and the layer-rule-vs-window-rule matching distinction: layer rules match the namespace 'quickview', window rules match the app ID and only apply on the no-layer-shell fallback path. niri snippet validated with 'niri validate'.
* fix: clamp Quick Preview panel to its intended size The canvas measures at the image's natural size and gtk::Overlay allocates centered overlay children at natural size, so any image larger than 900x700 grew the panel over the whole layer surface: no backdrop left to click and the image rendered 1:1 cropped instead of fit. Nested adw::Clamp widgets (horizontal + vertical) cap the maximum while the existing size request keeps the minimum, pinning the panel at exactly 900x700. Found by Codex review on PR #5 post-merge. * fix: pin clamp tightening threshold to maximum size AdwClamp's default tightening_threshold (400) makes the clamp's own natural size ease out well past maximum_size to give the child a run-in, and gtk::Overlay allocates centered children at natural size — so a large image could still push the panel to ~1900x1300. Threshold == maximum removes the easing margin entirely. Found by Codex on PR #8.
1 participant
Completes the FR-002 functional pair from Phase 6 (see
docs/PHASED_PLAN.md).Click-outside-to-close
Layer-shell path: the surface is now anchored to all four edges and made transparent via CSS. A backdrop
gtk::Box(main child of agtk::Overlay) closes the window on any click; the centered 900×700 panel holds the viewer as an overlay sibling, so clicks on the image are never seen by the backdrop gesture — OCR drag-select and middle-drag pan are unaffected.Fallback path (no layer shell): closes on focus loss with a was-active latch (only a true→false transition closes, guarding compositors that map the window unfocused). This deviates from the PHASED_PLAN sketch on purpose: a
GestureClickon the window would also fire for clicks inside the preview and break drag-select (FR-005). The plan doc is amended in this PR.UX changes to be aware of:
Single-instance toggle
GApplicationnow runs withHANDLES_COMMAND_LINE. The invoking process still does all resolution (clap, stdin path, canonicalization) and forwards a canonical synthetic argv (--mode=… --lang=… --file=…, newipcmodule with round-trip tests). Single-token--key=valueframing is deliberate: GLib's localGOptionContextpass strips a bare--separator even in pass-through mode (found empirically during smoke testing — the first framing attempt used-- <path>and lost the separator in transit).Dispatch semantics in the primary:
This retires the
run_with_args(&[])workaround: GLib only ever sees the sanitized argv we build ourselves. Lifecycle note: the first invocation's process now lives until its last window closes and serves subsequent invocations; remotes exit immediately with 0.Testing
cargo fmt/clippy -D warnings/cargo test --allclean (4 new ipc codec tests, 27 total).