From 1725ebed010840596275ab2171e74a2a1b804f9d Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:25:47 +0000 Subject: [PATCH 01/31] feat(workspace): add a pop-out viewer window that follows the selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Pop out button in the Details image pane opens a second window: the full viewer on a dark stage with a Details sidebar beside it, following the table's selection so the table can sit on one monitor and the scan on the other. - Title bar: Following/Pinned toggle, row stepping, "Row 42 of 318", and a "Table is on row N · Jump" chip while pinned. Pins hold row ids. - Sidebar: DetailsPanel without its image pane, resizable 240-720px and collapsible; documents get a Details | Find tab strip (Ctrl+F). - Stage: the viewer in a new Scope::PopOut, plus nothing-selected, missing-file and no-preview states, and a stack with Alt+Left/Right for several selected rows. - One window per project; closes with the project or the main window, and reopens at its saved size on its saved display. --- assets/icons/app-window.svg | 1 + assets/icons/file-x.svg | 1 + assets/icons/pin-filled.svg | 1 + assets/icons/pin.svg | 1 + crates/app/src/assets.rs | 21 +- crates/app/src/main.rs | 2 + crates/settings/src/lib.rs | 5 +- crates/workspace/src/lib.rs | 2 + crates/workspace/src/panels/details.rs | 54 +- crates/workspace/src/pop_out.rs | 1151 ++++++++++++++++++++++++ crates/workspace/src/viewer/find.rs | 10 +- crates/workspace/src/viewer/mod.rs | 158 +++- 12 files changed, 1347 insertions(+), 60 deletions(-) create mode 100644 assets/icons/app-window.svg create mode 100644 assets/icons/file-x.svg create mode 100644 assets/icons/pin-filled.svg create mode 100644 assets/icons/pin.svg create mode 100644 crates/workspace/src/pop_out.rs diff --git a/assets/icons/app-window.svg b/assets/icons/app-window.svg new file mode 100644 index 00000000..749559ab --- /dev/null +++ b/assets/icons/app-window.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/file-x.svg b/assets/icons/file-x.svg new file mode 100644 index 00000000..d5ae477e --- /dev/null +++ b/assets/icons/file-x.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/pin-filled.svg b/assets/icons/pin-filled.svg new file mode 100644 index 00000000..3623c7fb --- /dev/null +++ b/assets/icons/pin-filled.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/pin.svg b/assets/icons/pin.svg new file mode 100644 index 00000000..3b5b2732 --- /dev/null +++ b/assets/icons/pin.svg @@ -0,0 +1 @@ + diff --git a/crates/app/src/assets.rs b/crates/app/src/assets.rs index dd0d257e..67614d35 100644 --- a/crates/app/src/assets.rs +++ b/crates/app/src/assets.rs @@ -1,6 +1,7 @@ //! Our own icons in front of `gpui_component_assets`, which is otherwise the only asset source. //! The bundled icon set has no filled panel glyphs (the title bar's "this dock is open" state) and -//! no picture glyph (visual search), so those SVGs are ours, copied from Lucide. +//! no picture glyph (visual search), and none of the pop-out viewer's window, pin or missing-file +//! glyphs, so those SVGs are ours, copied from Lucide. use std::borrow::Cow; @@ -8,7 +9,7 @@ use gpui::{AssetSource, Result, SharedString}; pub struct Assets; -const OWN: [(&str, &str); 5] = [ +const OWN: [(&str, &str); 9] = [ ( "icons/history.svg", include_str!("../../../assets/icons/history.svg"), @@ -29,6 +30,22 @@ const OWN: [(&str, &str); 5] = [ "icons/image.svg", include_str!("../../../assets/icons/image.svg"), ), + ( + "icons/app-window.svg", + include_str!("../../../assets/icons/app-window.svg"), + ), + ( + "icons/pin.svg", + include_str!("../../../assets/icons/pin.svg"), + ), + ( + "icons/pin-filled.svg", + include_str!("../../../assets/icons/pin-filled.svg"), + ), + ( + "icons/file-x.svg", + include_str!("../../../assets/icons/file-x.svg"), + ), ]; impl AssetSource for Assets { diff --git a/crates/app/src/main.rs b/crates/app/src/main.rs index b2241bdd..20018455 100644 --- a/crates/app/src/main.rs +++ b/crates/app/src/main.rs @@ -221,6 +221,8 @@ impl App { return false; } flush_all_state(cx); + // Its table goes with this window, and on Windows and Linux an open window keeps the app running. + workspace::close_pop_out(cx); true }); diff --git a/crates/settings/src/lib.rs b/crates/settings/src/lib.rs index d6d19bae..e47eb3b5 100644 --- a/crates/settings/src/lib.rs +++ b/crates/settings/src/lib.rs @@ -35,6 +35,9 @@ use crate::path_picker::PathPickerApp; /// `AppSettings` value key for the Settings window's last size (a JSON [`MainWindowBounds`]). pub const SETTINGS_WINDOW_BOUNDS_KEY: &str = "settings_window_bounds"; +/// The same for the pop-out viewer, whose display is the point of it: it reopens on the monitor +/// the archivist moved it to. +pub const POP_OUT_WINDOW_BOUNDS_KEY: &str = "pop_out_window_bounds"; /// Setting key for autosave behavior: `"timed"` (buffered, the default), `"immediate"`, /// or `"off"`. Read by the table crate to decide when a committed cell edit reaches disk. @@ -539,7 +542,7 @@ impl AppSettings { } pub fn set_text(key: &'static str, val: SharedString, cx: &mut App) { - if key != SETTINGS_WINDOW_BOUNDS_KEY { + if key != SETTINGS_WINDOW_BOUNDS_KEY && key != POP_OUT_WINDOW_BOUNDS_KEY { log::debug!("settings: app text changed key={key}"); } Self::update(cx, |s| { diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index 199d4402..f8a8a1b5 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -6,10 +6,12 @@ mod dock_button; pub mod extension; mod panel_registry; mod panels; +mod pop_out; mod skin; mod viewer; mod views; +pub use pop_out::{close as close_pop_out, open as open_pop_out}; pub use viewer::{CloseViewerLayer, Scope as ViewerScope, VIEWER_CONTEXT, open_viewer}; pub use dock_button::DockToggleButton; diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 05d57437..58e3121c 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -7,7 +7,7 @@ use std::time::Duration; use gpui::prelude::FluentBuilder as _; use gpui::*; use gpui_component::{ - ActiveTheme, IconName, Sizable, StyledExt as _, + ActiveTheme, Icon, IconName, Selectable as _, Sizable, StyledExt as _, button::{Button, ButtonVariants}, dock::{BasePanel, DockPlacement, Panel, PanelEvent}, h_flex, @@ -146,6 +146,12 @@ pub struct DetailsPanel { /// What `transport` and `caption` were built from. `retarget` runs on every table change, so /// without it a keystroke in the grid would re-stat the file. file: Option, + /// The rows described instead of the grid's selection, which makes this the pop-out's sidebar: + /// that window can be pinned to an item the grid has moved on from, and its stage already + /// shows the file, so there is no image pane here. + rows: Option>, + /// Repaints the Pop out button when that window opens or closes. + _pop_out_sub: Subscription, } impl DetailsPanel { @@ -202,6 +208,9 @@ impl DetailsPanel { caption: None, _caption_task: None, file: None, + rows: None, + _pop_out_sub: cx + .observe_global::(|_this: &mut Self, cx| cx.notify()), }; this.bind(cx); this @@ -235,7 +244,10 @@ impl DetailsPanel { /// The selected items as source rows in view order — what the whole panel is about, and the /// same list the grid, the gallery and the status bar count. - fn picked(&self, cx: &App) -> Vec { + pub(crate) fn picked(&self, cx: &App) -> Vec { + if let Some(rows) = &self.rows { + return rows.clone(); + } self.state .as_ref() .and_then(|w| w.upgrade()) @@ -259,6 +271,11 @@ impl DetailsPanel { let picked = self.picked(cx); let front = self.front(&picked); self.load_row_history(front, cx); + // The pop-out's stage has the file, its caption and its transport. + if self.rows.is_some() { + self.transport = None; + return; + } let path = front.and_then(|row| { let state = self.state.as_ref()?.upgrade()?; let delegate = state.read(cx).delegate(); @@ -288,6 +305,16 @@ impl DetailsPanel { self.transport = path.and_then(|path| Transport::new(path, cx)); } + /// Describe `rows` from now on, rather than the grid's selection. + pub(crate) fn show_rows(&mut self, rows: Vec, cx: &mut Context) { + if self.rows.as_ref() == Some(&rows) { + return; + } + self.rows = Some(rows); + self.retarget(cx); + cx.notify(); + } + /// Re-read the front item's history, off the UI thread, when the item or the project file has /// changed since. A new item shows its unsaved changes at once and its saved ones on arrival. fn load_row_history(&mut self, front: Option, cx: &mut Context) { @@ -1019,6 +1046,21 @@ fn render_image_frame( }), ) }) + // Beside fullscreen, since both open a bigger view — but for any file, + // because the pop-out also says what it cannot show. One per project. + .child({ + let open = crate::pop_out::is_open(cx); + Button::new("pop-out") + .icon(Icon::empty().path("icons/app-window.svg")) + .ghost() + .small() + .selected(open) + .tooltip(match open { + true => "Show pop-out window", + false => "Open in new window", + }) + .on_click(|_, _, cx| crate::pop_out::open(cx)) + }) .child(action( "open-image", IconName::ExternalLink, @@ -1185,6 +1227,10 @@ impl Render for DetailsPanel { == crate::ViewMode::Gallery; let Some((fields, image_path, lost)) = selection.filter(|(f, _, _)| !f.is_empty()) else { + // The pop-out's stage already says so, in a place a collapsed sidebar cannot hide. + if self.rows.is_some() { + return div().into_any_element(); + } // Says what this panel is for and how to fill it, rather than only reporting that it // is empty — the multi-select gesture is the one thing here nobody discovers by luck. return div() @@ -1568,7 +1614,7 @@ impl Render for DetailsPanel { // region padding itself: the split below sizes its panes against whatever height it is // handed, so a panel that grew 29px when the bottom dock closed re-scaled the image // pane under the pointer. Paid here, the split's height never changes. - .pb(crop) + .when(self.rows.is_none(), |panel| panel.pb(crop)) .key_context(DETAILS_META.name) .track_focus(&self.focus_handle) .id("details-panel") @@ -1601,7 +1647,7 @@ impl Render for DetailsPanel { }) // Dropped entirely in the gallery: the cards are already showing this photo, // so the pane is just less room for the fields. It comes back with the grid. - .when(!gallery, |split| { + .when(!gallery && self.rows.is_none(), |split| { split.child( resizable_panel() .size(px(image_height)) diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs new file mode 100644 index 00000000..dac92476 --- /dev/null +++ b/crates/workspace/src/pop_out.rs @@ -0,0 +1,1151 @@ +//! The pop-out viewer: a second window that shows the selected row's file at full size beside its +//! fields. It follows the grid's selection, so on two monitors the table sits on one screen and the +//! scan fills the other — unless it is pinned, when it holds its item while the grid moves on. +//! +//! The stage is the full-screen viewer itself, mounted in [`Scope::PopOut`], and the sidebar is a +//! [`DetailsPanel`] describing whatever this window shows. Neither knows about the other: this +//! window decides which rows they are both about. + +use std::ops::Range; +use std::path::PathBuf; + +use gpui::prelude::FluentBuilder as _; +use gpui::*; +use gpui_component::{ + ActiveTheme, Disableable as _, Icon, IconName, Root, Selectable as _, Sizable, StyledExt as _, + TitleBar, + button::{Button, ButtonVariants}, + h_flex, + resizable::{ResizableState, h_resizable, resizable_panel}, + table::TableState, + v_flex, +}; +use settings::project::{CurrentProject, RowId}; +use settings::{AppSettings, MainWindowBounds, POP_OUT_WINDOW_BOUNDS_KEY}; +use table::{QrateTableDelegate, Selection, TableChanged, TablePanelHandle, TableStateHandle}; + +use crate::panels::DetailsPanel; +use crate::viewer::{self, Scope, Viewer}; + +/// How wide the sidebar opens, and how far it may be dragged either way — the same range as the +/// viewer's find panel, which it replaces for a document. +const SIDEBAR: Pixels = px(320.); +const SIDEBAR_RANGE: Range = px(240.)..px(720.); +/// The smallest window that still fits a readable page beside the sidebar. +const MIN_SIZE: Size = Size { + width: px(800.), + height: px(600.), +}; +const DEFAULT_SIZE: Size = Size { + width: px(1120.), + height: px(700.), +}; +/// Height of the sidebar's header strip, the Details title or the Details | Find tabs. +const HEADER_H: Pixels = px(30.); +/// The stage's own text colours. Not theme colours, for the reason the backdrop is not one. +const STAGE_FG: u32 = 0xe8e8e8; +const STAGE_MUTED: u32 = 0xa3a3a3; + +/// The open pop-out window. There is one per project, and one project open at a time. +#[derive(Default)] +pub(crate) struct PopOutWindow(Option); + +impl Global for PopOutWindow {} + +pub fn is_open(cx: &App) -> bool { + cx.try_global::() + .is_some_and(|window| window.0.is_some()) +} + +/// Open the pop-out window, or bring the open one forward. +pub fn open(cx: &mut App) { + if let Some(handle) = cx.try_global::().and_then(|window| window.0) + && handle + .update(cx, |_, window, _| window.activate_window()) + .is_ok() + { + return; + } + // Size and display only, like the other windows: the display is the point of this one. + let saved = AppSettings::get(cx) + .values + .get(POP_OUT_WINDOW_BOUNDS_KEY) + .map(|value| value.text()) + .and_then(|raw| serde_json::from_str::(&raw).ok()); + let display = saved.as_ref().and_then(|b| b.display_id).and_then(|raw| { + cx.displays() + .into_iter() + .find(|display| u64::from(display.id()) == raw) + .map(|display| display.id()) + }); + let win_size = saved + .filter(|b| { + b.width.is_finite() + && b.height.is_finite() + && px(b.width) >= MIN_SIZE.width + && px(b.height) >= MIN_SIZE.height + }) + .map_or(DEFAULT_SIZE, |b| size(px(b.width), px(b.height))); + let options = WindowOptions { + window_bounds: Some(WindowBounds::Windowed(Bounds::centered( + display, win_size, cx, + ))), + display_id: display, + window_min_size: Some(MIN_SIZE), + ..TitleBar::window_options() + }; + match cx.open_window(options, |window, cx| { + let view = cx.new(|cx| PopOut::new(window, cx)); + cx.new(|cx| Root::new(view, window, cx)) + }) { + Ok(handle) => cx.set_global(PopOutWindow(Some(handle.into()))), + Err(err) => log::error!("couldn't open the pop-out viewer window: {err:#}"), + } +} + +/// Close the pop-out window, if one is open. For the main window closing: the table this one +/// follows goes with it. +pub fn close(cx: &mut App) { + let Some(handle) = cx.try_global::().and_then(|window| window.0) else { + return; + }; + cx.set_global(PopOutWindow::default()); + handle + .update(cx, |_, window, _| window.remove_window()) + .ok(); +} + +pub struct PopOut { + focus_handle: FocusHandle, + /// The project this window was opened for. It closes when another one is opened. + project: Option, + state: Option>>, + /// The items held while pinned, by id rather than position, so rows added or removed above + /// them cannot move the pin onto a neighbour. `None` while following the grid. + pinned: Option>, + /// What this window shows — the pinned items or the grid's selection — as source rows. + rows: Vec, + /// Which of several items the stage shows. Only the stage steps: the sidebar's shared fields + /// and the grid's selection both keep all of them. + stack: usize, + /// The stack's step arrows only exist under the pointer, so they never cover the page at rest. + stack_hover: bool, + /// The front item's file, when there is one to draw. + viewer: Option>, + details: Entity, + sidebar: bool, + split: Entity, + _table_sub: Option, + /// Repaints when the viewer's pages, controls or find tab change under it. + _viewer_sub: Option, + _subs: Vec, +} + +impl PopOut { + fn new(window: &mut Window, cx: &mut Context) -> Self { + let details = cx.new(|cx| { + let mut details = DetailsPanel::new(window, cx); + details.show_rows(Vec::new(), cx); + details + }); + let me = window.window_handle(); + cx.on_release(move |this: &mut Self, cx| { + // A recording playing here would otherwise go on with nothing on screen to stop it. + if this + .viewer + .as_ref() + .is_some_and(|viewer| viewer.read(cx).transport.is_some()) + { + preview::playback::stop(cx); + } + // Only this window's own entry: a new pop-out may already have replaced it. + if cx + .try_global::() + .is_some_and(|window| window.0 == Some(me)) + { + cx.set_global(PopOutWindow::default()); + } + }) + .detach(); + + let _subs = vec![ + cx.observe_global_in::(window, |this, window, cx| { + this.bind(window, cx) + }), + cx.observe_global_in::(window, |this, window, cx| { + if cx.try_global::().map(|p| &p.file) != this.project.as_ref() { + window.remove_window(); + } + }), + cx.observe_global::(|_, cx| cx.notify()), + // The stage offers to install what a PDF or a video needs; once it lands, draw it. + cx.observe_global_in::(window, |this, window, cx| { + let installed = this.viewer.as_ref().is_some_and(|viewer| { + let viewer = viewer.read(cx); + viewer.needs.is_some() && preview::missing(&viewer.path).is_none() + }); + if installed { + this.viewer = None; + this.sync(window, cx); + } + }), + cx.observe_window_bounds(window, |_, window, cx| { + let bounds = MainWindowBounds::capture_from_window(window, cx); + if let Ok(json) = serde_json::to_string(&bounds) { + AppSettings::set_text(POP_OUT_WINDOW_BOUNDS_KEY, json.into(), cx); + } + }), + ]; + + let focus_handle = cx.focus_handle(); + focus_handle.focus(window, cx); + let mut this = Self { + focus_handle, + project: cx.try_global::().map(|p| p.file.clone()), + state: None, + pinned: None, + rows: Vec::new(), + stack: 0, + stack_hover: false, + viewer: None, + details, + sidebar: true, + split: cx.new(|_| ResizableState::default()), + _table_sub: None, + _viewer_sub: None, + _subs, + }; + this.bind(window, cx); + this + } + + fn bind(&mut self, window: &mut Window, cx: &mut Context) { + self.state = cx.try_global::().map(|h| h.0.clone()); + self._table_sub = self.state.as_ref().and_then(|w| w.upgrade()).map(|table| { + cx.subscribe_in(&table, window, |this, _, _: &TableChanged, window, cx| { + this.sync(window, cx) + }) + }); + self.sync(window, cx); + } + + fn table(&self) -> Option>> { + self.state.as_ref().and_then(|w| w.upgrade()) + } + + /// The item the stage shows: the stack's front card, clamped so a smaller selection never + /// leaves it pointing past the end. + fn front(&self) -> Option { + self.rows + .get(self.stack.min(self.rows.len().checked_sub(1)?)) + .copied() + } + + /// Bring the rows, the stage and the sidebar up to date with the grid. + fn sync(&mut self, window: &mut Window, cx: &mut Context) { + let rows = self.table().map_or_else(Vec::new, |state| { + let delegate = state.read(cx).delegate(); + match &self.pinned { + Some(ids) => ids + .iter() + .filter_map(|id| delegate.row_ids().iter().position(|row| row == id)) + .collect(), + None => delegate.selected_source_rows(), + } + }); + // Every pinned item deleted: there is nothing left to hold, so follow again. + if self.pinned.is_some() && rows.is_empty() { + self.pinned = None; + return self.sync(window, cx); + } + if rows != self.rows { + self.stack = 0; + self.rows = rows.clone(); + } + self.details + .update(cx, |details, cx| details.show_rows(rows, cx)); + + let file = self + .front() + .zip(self.table()) + .and_then(|(row, state)| viewer::previewable(state.read(cx).delegate(), row)); + if self.viewer.as_ref().map(|viewer| &viewer.read(cx).path) != file.as_ref() { + self.viewer = file.map(|file| viewer::build(file, Scope::PopOut, window, cx)); + self._viewer_sub = self + .viewer + .as_ref() + .map(|viewer| cx.observe(viewer, |_, _, cx| cx.notify())); + } + let (file, rest) = self.title(cx); + window.set_window_title(&format!("{file}{rest}")); + cx.notify(); + } + + /// The row ids of `rows`, which is what a pin holds on to. + fn ids(&self, rows: &[usize], cx: &App) -> Vec { + self.table().map_or_else(Vec::new, |state| { + let ids = state.read(cx).delegate().row_ids(); + rows.iter() + .filter_map(|&row| ids.get(row).copied()) + .collect() + }) + } + + /// Step to the neighbouring row with a file to show. Following, that moves the grid, and this + /// window comes along; pinned, it moves only this window. + fn step(&mut self, delta: isize, window: &mut Window, cx: &mut Context) { + if self.pinned.is_none() { + viewer::step_row(delta, cx); + return; + } + let Some(state) = self.table() else { + return; + }; + let target = { + let delegate = state.read(cx).delegate(); + let visible = delegate.visible(); + self.front() + .and_then(|row| delegate.view_row(row)) + .and_then(|from| { + viewer::next_row(from, delta, visible.len(), |view| { + viewer::previewable(delegate, visible[view]).is_some() + }) + }) + .and_then(|view| delegate.row_ids().get(visible[view]).copied()) + }; + if let Some(id) = target { + self.pinned = Some(vec![id]); + self.sync(window, cx); + } + } + + /// Walk the stack of selected items, wrapping at both ends like the Details preview does. + fn step_stack(&mut self, forward: bool, window: &mut Window, cx: &mut Context) { + let count = self.rows.len(); + if count < 2 { + return; + } + let at = self.stack.min(count - 1); + self.stack = match forward { + true => (at + 1) % count, + false => (at + count - 1) % count, + }; + self.sync(window, cx); + } + + fn toggle_pin(&mut self, window: &mut Window, cx: &mut Context) { + self.pinned = match self.pinned { + Some(_) => None, + None => Some(self.ids(&self.rows, cx)), + }; + self.sync(window, cx); + } + + /// Stay pinned, but on whatever the grid has selected now. + fn jump(&mut self, window: &mut Window, cx: &mut Context) { + let Some(state) = self.table() else { + return; + }; + let rows = state.read(cx).delegate().selected_source_rows(); + self.pinned = Some(self.ids(&rows, cx)); + self.sync(window, cx); + } + + /// Switch the sidebar to Find, opening it if it was collapsed — only a document has text. + fn open_find(&mut self, window: &mut Window, cx: &mut Context) { + let Some(viewer) = self.viewer.clone().filter(|v| v.read(cx).document) else { + return; + }; + self.sidebar = true; + viewer.update(cx, |viewer, cx| viewer.open_find(window, cx)); + cx.notify(); + } + + /// Back to the Details tab — what Escape means here, and all it means. + fn show_details(&mut self, window: &mut Window, cx: &mut Context) { + if let Some(viewer) = self.viewer.clone().filter(|v| v.read(cx).find_open) { + viewer.update(cx, |viewer, cx| viewer.close_find(window, cx)); + } + } + + fn key_down(&mut self, ev: &KeyDownEvent, window: &mut Window, cx: &mut Context) { + let keys = &ev.keystroke; + // Arrows are a text box's caret wherever one has focus; they mean rows only on the page + // or the bare window. + let reading = self.focus_handle.is_focused(window) + || self + .viewer + .as_ref() + .is_some_and(|viewer| viewer.read(cx).focus_handle.is_focused(window)); + match keys.key.as_str() { + "f" if keys.modifiers.secondary() => self.open_find(window, cx), + "up" | "down" if reading && !keys.modifiers.alt => { + self.step(if keys.key == "up" { -1 } else { 1 }, window, cx) + } + "left" | "right" if reading && keys.modifiers.alt => { + self.step_stack(keys.key == "right", window, cx) + } + _ => {} + } + } + + /// The window's title, as the file part and the rest, which the title bar mutes. + fn title(&self, cx: &App) -> (String, String) { + let project = cx + .try_global::() + .map(|p| p.display_name()) + .unwrap_or_default(); + let file = match (self.rows.len(), self.front().zip(self.table())) { + (0, _) | (_, None) => String::new(), + (1, Some((row, state))) => { + let delegate = state.read(cx).delegate(); + delegate + .row_image(row) + .and_then(|path| path.file_name()) + .map(|name| name.to_string_lossy().into_owned()) + .or_else(|| { + table::file_links::missing_file(delegate, row, cx) + .map(|(_, name)| name.to_string()) + }) + .unwrap_or_default() + } + (count, _) => format!("{count} items"), + }; + let rest = match file.is_empty() { + true => format!("{project} — qrate"), + false => format!(" — {project} — qrate"), + }; + (file, rest) + } + + /// Where this window is in the grid's order, and — while pinned somewhere else — which row + /// the grid is on. + fn readouts(&self, cx: &App) -> (String, Option) { + let Some(state) = self.table() else { + return ("No selection".into(), None); + }; + let delegate = state.read(cx).delegate(); + let total = delegate.visible().len(); + let views: Vec = self + .rows + .iter() + .filter_map(|&row| delegate.view_row(row)) + .collect(); + let readout = match (self.rows.len(), views.iter().min(), views.iter().max()) { + (0, ..) => "No selection".to_string(), + (1, Some(view), _) => format!("Row {} of {total}", view + 1), + (1, None, _) => "Row hidden by the filter".to_string(), + (count, Some(low), Some(high)) => { + format!("Rows {}–{} · {count} selected", low + 1, high + 1) + } + (count, ..) => format!("{count} selected"), + }; + let table_row = self + .pinned + .as_ref() + .filter(|_| delegate.selected_source_rows() != self.rows) + .and_then(|_| match delegate.selection() { + Some(Selection::Cell { row, .. } | Selection::Row(row)) => delegate.view_row(row), + _ => None, + }) + .map(|view| view + 1); + (readout, table_row) + } + + fn title_bar(&self, cx: &mut Context) -> AnyElement { + let pinned = self.pinned.is_some(); + let (readout, table_row) = self.readouts(cx); + let (file, rest) = self.title(cx); + let dirty = settings::dirty::Dirty::has(settings::dirty::PROJECT_DATA, cx); + let theme = cx.theme(); + let (fg, muted, border, chip, link) = ( + theme.foreground, + theme.muted_foreground, + theme.border, + theme.muted, + theme.primary, + ); + + TitleBar::new() + .text_xs() + .text_color(fg) + .child( + h_flex().flex_1().min_w_0().child( + // Occluded: the title bar is a drag region, which Windows never delivers a + // click from. + h_flex() + .min_w_0() + .gap_1() + .occlude() + .child( + Button::new("pop-out-follow") + .icon(Icon::empty().path(match pinned { + true => "icons/pin-filled.svg", + false => "icons/pin.svg", + })) + .label(match pinned { + true => "Pinned", + false => "Following", + }) + .ghost() + .xsmall() + .selected(pinned) + .disabled(!pinned && self.rows.is_empty()) + .tooltip(match pinned { + true => "Follow the table's selection again", + false => "Keep this item here while the table moves on", + }) + .on_click( + cx.listener(|this, _, window, cx| this.toggle_pin(window, cx)), + ), + ) + .child(div().flex_none().w_px().h(px(14.)).mx_0p5().bg(border)) + .child( + Button::new("pop-out-previous-row") + .icon(IconName::ChevronUp) + .ghost() + .xsmall() + .tooltip("Previous row (↑)") + .on_click( + cx.listener(|this, _, window, cx| this.step(-1, window, cx)), + ), + ) + .child( + Button::new("pop-out-next-row") + .icon(IconName::ChevronDown) + .ghost() + .xsmall() + .tooltip("Next row (↓)") + .on_click( + cx.listener(|this, _, window, cx| this.step(1, window, cx)), + ), + ) + .child(div().min_w_0().truncate().text_color(muted).child(readout)) + .when_some(table_row, |bar, row| { + bar.child( + h_flex() + .flex_none() + .items_center() + .gap_1p5() + .h(px(20.)) + .px_2() + .ml_1() + .rounded_full() + .bg(chip) + .whitespace_nowrap() + .child(format!("Table is on row {row}")) + .child( + div() + .id("pop-out-jump") + .text_color(link) + .cursor_pointer() + .child("Jump") + .on_click(cx.listener(|this, _, window, cx| { + this.jump(window, cx) + })), + ), + ) + }), + ), + ) + // The file name gives way before the row controls do. + .child( + h_flex() + .min_w_0() + .max_w(relative(0.42)) + .gap_1p5() + .items_center() + .when(dirty, |title| { + title.child(div().flex_none().size(px(6.)).rounded_full().bg(fg)) + }) + .child(div().min_w_0().truncate().child(file)) + .child(div().min_w_0().truncate().text_color(muted).child(rest)), + ) + .child( + h_flex().flex_1().justify_end().pr_2().child( + div().occlude().child( + Button::new("pop-out-sidebar") + .icon(Icon::empty().path(match self.sidebar { + true => "icons/panel-right-filled.svg", + false => "icons/panel-right.svg", + })) + .ghost() + .small() + .tooltip(match self.sidebar { + true => "Hide details", + false => "Show details", + }) + .on_click(cx.listener(|this, _, _, cx| { + this.sidebar = !this.sidebar; + cx.notify(); + })), + ), + ), + ) + .into_any_element() + } + + /// The file, or what the stage says in its place, over the viewer's own backdrop. + fn stage(&self, cx: &mut Context) -> AnyElement { + let (fg, muted) = (rgb(STAGE_FG), rgb(STAGE_MUTED)); + let front = self.front().zip(self.table()).map(|(row, state)| { + let delegate = state.read(cx).delegate(); + ( + row, + delegate.row_image(row).map(|path| path.to_path_buf()), + table::file_links::missing_file(delegate, row, cx).map(|(_, name)| name), + ) + }); + + // What to say when there is nothing the viewer can draw. The one useful action goes with + // it, where there is one. + let message = self.viewer.is_none().then(|| match front { + None => v_flex() + .items_center() + .gap_1p5() + .max_w(px(360.)) + .p_6() + .text_center() + .child( + Icon::new(IconName::LayoutDashboard) + .size_8() + .text_color(muted), + ) + .child(div().mt_1().text_sm().text_color(fg).child("Nothing selected")) + .child( + div() + .text_size(px(13.)) + .text_color(muted) + .child("Select a row in the table to see its file here. This window follows the selection."), + ), + Some((row, None, Some(name))) => v_flex() + .items_center() + .gap_2() + .max_w(px(420.)) + .p_6() + .text_center() + .child( + Icon::empty() + .path("icons/file-x.svg") + .size_12() + .text_color(cx.theme().warning), + ) + .child(div().text_sm().text_color(fg).child("File not found")) + .child( + div() + .text_xs() + .font_family(cx.theme().mono_font_family.clone()) + .text_color(muted) + .child(name), + ) + .child( + Button::new("pop-out-locate-file") + .small() + .mt_1() + .label("Locate file…") + .on_click(move |_, window, cx| { + if let Some(table) = cx + .try_global::() + .and_then(|handle| handle.0.upgrade()) + { + table.update(cx, |table, cx| table.locate_file(row, window, cx)); + } + }), + ), + Some((_, file, _)) => { + let tag = file + .as_deref() + .and_then(|path| path.extension()) + .map(|ext| ext.to_string_lossy().to_uppercase()); + v_flex() + .items_center() + .gap_2p5() + .p_6() + .text_center() + .child( + div() + .relative() + .size_16() + .text_color(muted) + .child(Icon::new(IconName::File).size_16()) + .children(tag.clone().map(|tag| { + div() + .absolute() + .left_0() + .right_0() + .bottom(px(14.)) + .text_center() + .text_size(px(11.)) + .font_semibold() + .child(tag) + })), + ) + .child(div().text_size(px(13.)).text_color(muted).child( + match (&file, &tag) { + (Some(_), Some(tag)) => format!("No preview for {tag} files"), + (Some(_), None) => "No preview for this file".to_string(), + (None, _) => "No file is linked to this item".to_string(), + }, + )) + .children(file.map(|path| { + Button::new("pop-out-open-default") + .small() + .icon(IconName::ExternalLink) + .label("Open in the default app") + .on_click(move |_, _, _| { + if let Err(err) = settings::os_open::open_in_default_app(&path) { + log::error!("could not open {}: {err}", path.display()); + } + }) + })) + } + }); + + let count = self.rows.len(); + let at = self.stack.min(count.saturating_sub(1)); + // Above the viewer's own pill when it has one, so a stack of PDFs keeps its page controls. + let lift = match self + .viewer + .as_ref() + .is_some_and(|viewer| viewer.read(cx).has_controls()) + { + true => px(64.), + false => px(16.), + }; + let theme = cx.theme(); + let (background, popover, border, radius) = + (theme.background, theme.popover, theme.border, theme.radius); + + div() + .id("pop-out-stage") + .relative() + .size_full() + .overflow_hidden() + .bg(background) + .on_hover(cx.listener(|this, over: &bool, _, cx| { + this.stack_hover = *over; + cx.notify(); + })) + // Back from the sidebar, a click on the page gives the arrows and zoom keys back to it. + .on_mouse_down( + MouseButton::Left, + cx.listener(|this, _, window, cx| { + let focus = match &this.viewer { + Some(viewer) => viewer.read(cx).focus_handle.clone(), + None => this.focus_handle.clone(), + }; + focus.focus(window, cx); + }), + ) + .child( + // The viewer's backdrop, painted here so every state of the stage shares it. + div() + .absolute() + .top_0() + .left_0() + .size_full() + .bg(black().opacity(0.85)), + ) + .children(self.viewer.clone()) + .children(message.map(|message| { + div() + .absolute() + .top_0() + .left_0() + .size_full() + .flex() + .items_center() + .justify_center() + .child(message) + })) + .when(count > 1, |stage| { + stage + .when(self.stack_hover, |stage| { + stage.children([true, false].map(|left| { + div() + .absolute() + .top_0() + .bottom_0() + .map(|side| match left { + true => side.left_4(), + false => side.right_4(), + }) + .flex() + .items_center() + .child( + div() + .rounded_full() + .bg(popover.opacity(0.8)) + .border_1() + .border_color(border) + .occlude() + .child( + Button::new(match left { + true => "pop-out-stack-previous", + false => "pop-out-stack-next", + }) + .icon(match left { + true => IconName::ChevronLeft, + false => IconName::ChevronRight, + }) + .ghost() + .rounded_full() + .tooltip(match left { + true => "Previous selected item (Alt+←)", + false => "Next selected item (Alt+→)", + }) + .on_click(cx.listener(move |this, _, window, cx| { + this.step_stack(!left, window, cx) + })), + ), + ) + })) + }) + .child( + div() + .absolute() + .left_0() + .right_0() + .bottom(lift) + .flex() + .justify_center() + .child( + h_flex() + .items_center() + .gap_1() + .p_1() + .rounded(radius * 2.) + .bg(popover) + .border_1() + .border_color(border) + .shadow_lg() + .occlude() + .text_color(cx.theme().foreground) + .text_size(px(13.)) + .child( + Button::new("pop-out-stack-back") + .icon(IconName::ChevronLeft) + .ghost() + .small() + .tooltip("Previous selected item (Alt+←)") + .on_click(cx.listener(|this, _, window, cx| { + this.step_stack(false, window, cx) + })), + ) + .child( + div().px_1().child(format!("Item {} of {count}", at + 1)), + ) + // Past a dozen the dots stop telling anyone anything. + .when(count <= 12, |pill| { + pill.child(h_flex().gap_1().px_1().children( + (0..count).map(|ix| { + div().size(px(6.)).rounded_full().bg( + match ix == at { + true => cx.theme().primary, + false => cx.theme().muted, + }, + ) + }), + )) + }) + .child( + Button::new("pop-out-stack-forward") + .icon(IconName::ChevronRight) + .ghost() + .small() + .tooltip("Next selected item (Alt+→)") + .on_click(cx.listener(|this, _, window, cx| { + this.step_stack(true, window, cx) + })), + ), + ), + ) + }) + .into_any_element() + } + + /// Details, or — for a document — Details and Find as two tabs. A tab rather than a third + /// column, so the window still works at its smallest. + fn sidebar(&self, width: Pixels, cx: &mut Context) -> AnyElement { + let document = self + .viewer + .clone() + .filter(|viewer| viewer.read(cx).document); + let finding = document + .as_ref() + .is_some_and(|viewer| viewer.read(cx).find_open); + let theme = cx.theme(); + let (fg, muted, border, primary, strip, background) = ( + theme.foreground, + theme.muted_foreground, + theme.border, + theme.primary, + theme.tab_bar, + theme.background, + ); + let tab = |id: &'static str, label: &'static str, on: bool| { + div() + .id(id) + .flex() + .items_center() + .px_2() + .cursor_pointer() + .text_color(if on { fg } else { muted }) + .when(on, |tab| tab.border_b_2().border_color(primary)) + .child(label) + }; + let header = + h_flex() + .flex_none() + .h(HEADER_H) + .bg(strip) + .border_b_1() + .border_color(border) + .text_size(px(13.)) + .map(|header| match document.is_some() { + false => header.items_center().px_3().child("Details"), + true => header + .items_stretch() + .gap_0p5() + .px_2() + .child(tab("pop-out-tab-details", "Details", !finding).on_click( + cx.listener(|this, _, window, cx| this.show_details(window, cx)), + )) + .child(tab("pop-out-tab-find", "Find", finding).on_click( + cx.listener(|this, _, window, cx| this.open_find(window, cx)), + )), + }); + let body = match document.filter(|_| finding) { + Some(viewer) => viewer.update(cx, |viewer, cx| { + viewer::find::panel(&viewer.find, width, true, cx) + }), + None => self.details.clone().into_any_element(), + }; + v_flex() + .size_full() + .bg(background) + .border_l_1() + .border_color(border) + .child(header) + .child(div().flex_1().min_h_0().child(body)) + .into_any_element() + } +} + +impl Render for PopOut { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + let dialog_layer = Root::render_dialog_layer(window, cx); + // The live width, so the find rows re-trim as the sidebar is dragged. + let width = self + .split + .read(cx) + .sizes() + .get(1) + .copied() + .unwrap_or(SIDEBAR); + + v_flex() + .size_full() + .bg(cx.theme().background) + .text_color(cx.theme().foreground) + .key_context("PopOut") + .track_focus(&self.focus_handle) + .id("pop-out") + .role(Role::Group) + .aria_label("Pop-out viewer") + // Escape from Details would deselect the grid's rows; here it only ever means "back + // to Details", and past that, nothing. + .on_action( + cx.listener(|this, _: &table::Deselect, window, cx| this.show_details(window, cx)), + ) + .on_action( + cx.listener(|this, _: &gpui_component::input::Escape, window, cx| { + this.show_details(window, cx) + }), + ) + .on_key_down(cx.listener(Self::key_down)) + .child(self.title_bar(cx)) + .child( + div().flex_1().min_h_0().child( + h_resizable("pop-out-split") + .with_state(&self.split) + .child(resizable_panel().child(self.stage(cx))) + // `visible` rather than adding and removing the panel, so a collapse keeps + // the width the sidebar was dragged to. + .child( + resizable_panel() + .size(SIDEBAR) + .size_range(SIDEBAR_RANGE) + .visible(self.sidebar) + .child(self.sidebar(width, cx)), + ), + ), + ) + .children(dialog_layer) + } +} + +#[cfg(test)] +mod tests { + // Never `use super::*` here — the parent's `use gpui::*` would shadow `#[test]`. + use gpui::{Entity, TestAppContext, VisualTestContext}; + use gpui_component::table::TableState; + use table::{QrateTableDelegate, TableChanged}; + + use super::PopOut; + + /// Three rows in a real grid, and the pop-out window watching it. + fn window_over_a_table( + cx: &mut TestAppContext, + ) -> ( + Entity, + Entity>, + &mut VisualTestContext, + ) { + cx.update(|cx| { + gpui_component::init(cx); + let mut app = settings::AppSettings::default(); + app.values.insert( + settings::AUTOSAVE_KEY.into(), + settings::Val::Text("off".into()), + ); + cx.set_global(app); + cx.set_global(settings::SettingsPersistence::default()); + cx.set_global(settings::project::CurrentProject { + file: std::env::temp_dir().join("qrate-pop-out.qrate"), + data: settings::project::ProjectData { + name: "Aderman Collection".into(), + columns: Vec::new(), + headers: vec!["Identifier".into(), "Title".into()], + rows: vec![ + vec!["ADR-0042".into(), "Beacon Hill Park".into()], + vec!["ADR-0043".into(), "Sawmill crew".into()], + vec!["ADR-0044".into(), "Saanich mill".into()], + ], + row_ids: vec![11, 12, 13], + values: Default::default(), + }, + }); + }); + cx.add_window_view(table::TablePanel::new); + let state = cx.update(|cx| { + cx.global::() + .0 + .upgrade() + .expect("the table panel publishes its state handle") + }); + let (pop_out, cx) = cx.add_window_view(PopOut::new); + (pop_out, state, cx) + } + + fn select( + state: &Entity>, + rows: &[usize], + cx: &mut VisualTestContext, + ) { + state.update(cx, |state, cx| { + state.delegate_mut().select_only_row(rows[0]); + for &row in &rows[1..] { + state.delegate_mut().toggle_row(row); + } + cx.emit(TableChanged); + }); + cx.run_until_parked(); + } + + /// The window's reason to exist: it follows the grid until pinned, then holds its item while + /// the grid moves on — and says where the grid went, so Jump can catch up. + #[gpui::test] + fn a_pinned_window_holds_its_item_while_the_table_moves_on(cx: &mut TestAppContext) { + let (pop_out, state, cx) = window_over_a_table(cx); + + select(&state, &[0], cx); + pop_out.read_with(cx, |pop_out, _| assert_eq!(pop_out.rows, [0])); + + pop_out.update_in(cx, |pop_out, window, cx| pop_out.toggle_pin(window, cx)); + select(&state, &[1], cx); + pop_out.read_with(cx, |pop_out, cx| { + assert_eq!(pop_out.rows, [0], "pinned, the grid moving is not ours"); + assert_eq!( + pop_out.readouts(cx), + ("Row 1 of 3".to_string(), Some(2)), + "and the chip says where the grid is" + ); + }); + + pop_out.update_in(cx, |pop_out, window, cx| pop_out.jump(window, cx)); + pop_out.read_with(cx, |pop_out, cx| { + assert_eq!(pop_out.rows, [1], "Jump moves this window to the grid"); + assert!(pop_out.pinned.is_some(), "and leaves it pinned there"); + assert_eq!( + pop_out.readouts(cx).1, + None, + "so there is nowhere left to jump" + ); + }); + + pop_out.update_in(cx, |pop_out, window, cx| pop_out.toggle_pin(window, cx)); + select(&state, &[2], cx); + pop_out.read_with(cx, |pop_out, _| { + assert_eq!(pop_out.rows, [2], "unpinned, it follows again") + }); + } + + /// A pin is held by row id, so the item stays put when the grid's selection changes — and the + /// sidebar describes the pinned item, not the grid's. + #[gpui::test] + fn the_sidebar_describes_what_the_window_shows(cx: &mut TestAppContext) { + let (pop_out, state, cx) = window_over_a_table(cx); + + select(&state, &[1], cx); + pop_out.update_in(cx, |pop_out, window, cx| pop_out.toggle_pin(window, cx)); + pop_out.read_with(cx, |pop_out, _| { + assert_eq!(pop_out.pinned.as_deref(), Some(&[12][..])) + }); + select(&state, &[0, 2], cx); + + let details = pop_out.read_with(cx, |pop_out, _| pop_out.details.clone()); + details.read_with(cx, |details, cx| assert_eq!(details.picked(cx), [1])); + } + + /// Several rows: the stage steps through them, wrapping, while the sidebar keeps all of them. + #[gpui::test] + fn the_stack_steps_through_the_selection_and_wraps(cx: &mut TestAppContext) { + let (pop_out, state, cx) = window_over_a_table(cx); + select(&state, &[0, 2], cx); + + pop_out.update_in(cx, |pop_out, window, cx| { + assert_eq!(pop_out.front(), Some(0)); + pop_out.step_stack(true, window, cx); + assert_eq!(pop_out.front(), Some(2)); + pop_out.step_stack(true, window, cx); + assert_eq!(pop_out.front(), Some(0), "wraps past the end"); + pop_out.step_stack(false, window, cx); + assert_eq!(pop_out.front(), Some(2), "and past the start"); + assert_eq!(pop_out.rows, [0, 2], "without touching the selection"); + let (readout, _) = pop_out.readouts(cx); + assert_eq!(readout, "Rows 1–3 · 2 selected"); + assert_eq!(pop_out.title(cx).0, "2 items"); + }); + + // A new selection starts at its first item rather than wherever the last one was left. + select(&state, &[1, 2], cx); + pop_out.read_with(cx, |pop_out, _| assert_eq!(pop_out.front(), Some(1))); + } + + /// Nothing selected is a state of the stage, not an empty window. + #[gpui::test] + fn with_nothing_selected_the_window_says_so(cx: &mut TestAppContext) { + let (pop_out, _state, cx) = window_over_a_table(cx); + cx.update(|window, cx| window.draw(cx).clear(cx)); + pop_out.read_with(cx, |pop_out, cx| { + assert!(pop_out.rows.is_empty()); + assert!(pop_out.viewer.is_none()); + assert_eq!(pop_out.readouts(cx).0, "No selection"); + assert_eq!( + pop_out.title(cx), + (String::new(), "Aderman Collection — qrate".to_string()) + ); + }); + } +} diff --git a/crates/workspace/src/viewer/find.rs b/crates/workspace/src/viewer/find.rs index b1cf3d41..c40c3b33 100644 --- a/crates/workspace/src/viewer/find.rs +++ b/crates/workspace/src/viewer/find.rs @@ -82,7 +82,9 @@ impl Find { /// /// One row per hit rather than a box holding the whole document — a reader wants to see *where* a /// word turns up, and a wall of extracted text answers a question nobody asked. -pub fn panel(find: &Find, width: Pixels, cx: &mut Context) -> AnyElement { +/// +/// `docked` is the pop-out's sidebar tab, which has its own edge and no controls above it. +pub fn panel(find: &Find, width: Pixels, docked: bool, cx: &mut Context) -> AnyElement { let empty = find.hits.is_empty(); let rows: Vec<_> = find .hits @@ -96,11 +98,11 @@ pub fn panel(find: &Find, width: Pixels, cx: &mut Context) -> AnyElement .gap_2() .p_2() // Clears the control cluster pinned to the overlay's top-right corner. - .pt_12() + .when(!docked, |panel| { + panel.pt_12().border_l_1().border_color(cx.theme().border) + }) .occlude() .bg(cx.theme().background) - .border_l_1() - .border_color(cx.theme().border) .child( h_flex() .gap_1() diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 3e9f9fa5..829f41c7 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -10,7 +10,7 @@ //! The two neighbours hold the parts with rules in them: [`find`] owns the search state, and //! [`highlight`] turns a hit's position on the page into a position on the screen. -mod find; +pub(crate) mod find; mod highlight; pub(crate) mod transport; @@ -51,6 +51,9 @@ pub enum Scope { Workspace, /// Over the centre panel only, leaving the docked panels visible. Centre, + /// The pop-out window's stage. Never the global viewer: that window owns its viewer, and + /// follows the selection itself so that it can stop following while pinned. + PopOut, } /// The currently-open viewer and the focus to restore. Both mount slots observe this. @@ -71,24 +74,42 @@ pub fn viewer_in(scope: Scope, cx: &App) -> Option> { /// Opens `path` in the shared viewer overlay, replacing any viewer already open. pub fn open_viewer(path: PathBuf, scope: Scope, window: &mut Window, cx: &mut App) { - let document = preview::has_text(&path); - let video = preview::has_video(&path); - let details = preview::describe(&path); - let probe_path = path.clone(); let return_focus = cx .try_global::() .and_then(|active| active.return_focus.clone()) .or_else(|| window.focused(cx)); + let viewer = build(path, scope, window, cx); + cx.set_global(ActiveViewer { + viewer: Some(viewer), + return_focus, + }); +} + +/// A viewer for `path`, not yet mounted anywhere. +pub(crate) fn build( + path: PathBuf, + scope: Scope, + window: &mut Window, + cx: &mut App, +) -> Entity { + let document = preview::has_text(&path); + let video = preview::has_video(&path); + let details = preview::describe(&path); + let probe_path = path.clone(); let table = cx .try_global::() - .and_then(|handle| handle.0.upgrade()); + .and_then(|handle| handle.0.upgrade()) + .filter(|_| scope != Scope::PopOut); let needs = preview::missing(&path); let viewer = cx.new(|cx| Viewer { needs, _components: cx.observe_global_in::( window, |this: &mut Viewer, window, cx| { - if this.needs.is_some() && preview::missing(&this.path).is_none() { + if this.scope != Scope::PopOut + && this.needs.is_some() + && preview::missing(&this.path).is_none() + { open_viewer(this.path.clone(), this.scope, window, cx); } cx.notify(); @@ -149,16 +170,13 @@ pub fn open_viewer(path: PathBuf, scope: Scope, window: &mut Window, cx: &mut Ap }); }) .detach(); - cx.set_global(ActiveViewer { - viewer: Some(viewer), - return_focus, - }); + viewer } /// Select the next row, by `delta`, in the view's order that has something to preview. During a /// search the view is its hits, so this steps through the results. The open viewer follows the /// selection to that row's file. -fn step_row(delta: isize, cx: &mut App) { +pub(crate) fn step_row(delta: isize, cx: &mut App) { let Some(state) = cx .try_global::() .and_then(|handle| handle.0.upgrade()) @@ -189,7 +207,7 @@ fn step_row(delta: isize, cx: &mut App) { } /// The file `row` links to, if the viewer can show it. -fn previewable(delegate: &table::QrateTableDelegate, row: usize) -> Option { +pub(crate) fn previewable(delegate: &table::QrateTableDelegate, row: usize) -> Option { delegate .row_image(row) .filter(|file| preview::can_preview(file)) @@ -197,7 +215,7 @@ fn previewable(delegate: &table::QrateTableDelegate, row: usize) -> Option, scope: Scope, @@ -240,12 +258,12 @@ pub struct Viewer { pages: usize, /// Whether this file is a document at all, which is a different question from whether it has /// more than one page — a one-page PDF is still a document, and still says "1 / 1". - document: bool, + pub(crate) document: bool, /// Known from the extension immediately, before the duration probe finishes. video: bool, /// The playback transport, present exactly when the file is a recording. Gated on the format /// for the same reason `document` is: a silent tape is still audio and still gets a transport. - transport: Option, + pub(crate) transport: Option, /// The scrubber, present exactly when the file is a video ffmpeg could measure. /// scrubber: Option>, @@ -258,18 +276,18 @@ pub struct Viewer { offset: Point, /// Last pointer position while dragging; `None` when not panning. drag_from: Option>, - focus_handle: FocusHandle, + pub(crate) focus_handle: FocusHandle, /// Grabs focus on first render so Escape reaches [`Self`]; set once so we don't re-focus. focused: bool, - find: Find, - /// Whether the find panel is showing. - find_open: bool, + pub(crate) find: Find, + /// Whether the find panel is showing — in the pop-out, whether its sidebar is on Find. + pub(crate) find_open: bool, /// The split between the page and the find panel, owned by `gpui_component`'s resizable — it /// carries the drag handle, the sizing and the propagation rules, none of which are ours to /// reinvent. split: Entity, /// The optional part this file needs and does not have, which the viewer offers to install. - needs: Option, + pub(crate) needs: Option, /// Opens the file again once that part is installed, so its pages and timeline are read. _components: Subscription, /// Swaps in the selected row's file when the selection moves, from the find bar, the arrows or @@ -341,8 +359,20 @@ impl Viewer { self.offset = Point::default(); } + /// Whether the bottom pill has anything to hold: page controls, a transport or a scrubber. + pub(crate) fn has_controls(&self) -> bool { + self.document || self.pages > 1 || self.transport.is_some() || self.scrubber.is_some() + } + + /// Put the find panel away and hand the keys back to the page. + pub(crate) fn close_find(&mut self, window: &mut Window, cx: &mut Context) { + self.find_open = false; + window.focus(&self.focus_handle, cx); + cx.notify(); + } + /// Show the find panel, building its query box the first time. - fn open_find(&mut self, window: &mut Window, cx: &mut Context) { + pub(crate) fn open_find(&mut self, window: &mut Window, cx: &mut Context) { self.find_open = true; // Asked once, on first open: it opens the document, and the answer cannot change while @@ -485,6 +515,7 @@ impl Render for Viewer { let banner = self .needs .and_then(|id| crate::component_banner::banner(id, cx)); + let popped = self.scope == Scope::PopOut; div() .track_focus(&self.focus_handle) @@ -493,12 +524,11 @@ impl Render for Viewer { .role(Role::Group) .aria_label("File viewer") // The find panel is the inner layer, so Escape dismisses it before the viewer. + // The pop-out has no overlay to close: its window is what the viewer is. .on_action(cx.listener(|this, _: &CloseViewerLayer, window, cx| { if this.find_open { - this.find_open = false; - window.focus(&this.focus_handle, cx); - cx.notify(); - } else { + this.close_find(window, cx); + } else if this.scope != Scope::PopOut { close_viewer(window, cx); } })) @@ -516,23 +546,28 @@ impl Render for Viewer { .occlude() // Dim what's behind so the file reads as the focus. Not a theme colour: a light // theme's background is white, which hides nothing and lights the room around a photo. - .bg(black().opacity(0.85)) + // The pop-out's stage paints the same backdrop, whatever it is showing. + .when(!popped, |viewer| viewer.bg(black().opacity(0.85))) .on_key_down(cx.listener(|this, ev: &KeyDownEvent, window, cx| { // Paging keys are only ours while the viewer itself holds focus: with the query // box focused, left/right belong to its caret. let reading = this.focus_handle.is_focused(window); + // Alt+←/→ steps the pop-out's stack of selected items, not the pages. + let paging = reading && this.scrubber.is_none() && !ev.keystroke.modifiers.alt; + // The pop-out's window handles Find and the rows, since both reach past the page. + let overlay = this.scope != Scope::PopOut; match ev.keystroke.key.as_str() { - "f" if ev.keystroke.modifiers.secondary() && this.document => { + "f" if overlay && ev.keystroke.modifiers.secondary() && this.document => { this.open_find(window, cx); } // The keys anyone reading a document reaches for first. Harmless on a photo, // where there is only ever one page to move between — but kept off a video, // whose position is the scrubber's, and whose thumb would be left behind. - "left" | "pageup" if reading && this.scrubber.is_none() => { + "left" | "pageup" if paging => { this.turn_page(-1); cx.notify(); } - "right" | "pagedown" if reading && this.scrubber.is_none() => { + "right" | "pagedown" if paging => { this.turn_page(1); cx.notify(); } @@ -552,7 +587,7 @@ impl Render for Viewer { this.set_zoom(1.0); cx.notify(); } - "up" | "down" if reading => { + "up" | "down" if reading && overlay => { step_row(if ev.keystroke.key == "up" { -1 } else { 1 }, cx); } _ => {} @@ -638,8 +673,8 @@ impl Render for Viewer { resizable_panel() .size(PANEL) .size_range(PANEL_RANGE) - .visible(self.find_open) - .child(find::panel(&self.find, panel_width, cx)), + .visible(self.find_open && !popped) + .child(find::panel(&self.find, panel_width, false, cx)), ), ) .child( @@ -673,10 +708,7 @@ impl Render for Viewer { })) // Bottom pill: page controls (even for 1 page, or a TIFF stack), transport or scrubber. .when( - self.document - || self.pages > 1 - || self.transport.is_some() - || self.scrubber.is_some(), + self.has_controls(), |viewer| { viewer.child( div() @@ -790,8 +822,8 @@ impl Render for Viewer { }, ) // Rows, not pages: the neighbouring files in the view's order, which during a search are - // the neighbouring results. - .child( + // the neighbouring results. The pop-out keeps these in its title bar instead. + .children((!popped).then(|| { div() .absolute() .bottom_4() @@ -816,9 +848,10 @@ impl Render for Viewer { .small() .tooltip("Next row (↓)") .on_click(|_, _, cx| step_row(1, cx)), - ), - ) - .child( + ) + })) + // Zoom has its keys and the wheel there; find is a sidebar tab, and closing is the window's. + .children((!popped).then(|| { div() .absolute() .top_4() @@ -844,9 +877,7 @@ impl Render for Viewer { }) .on_click(cx.listener(|this, _, window, cx| { if this.find_open { - this.find_open = false; - window.focus(&this.focus_handle, cx); - cx.notify(); + this.close_find(window, cx); } else { this.open_find(window, cx); } @@ -882,8 +913,8 @@ impl Render for Viewer { .small() .tooltip("Close (Esc)") .on_click(cx.listener(|_, _, window, cx| close_viewer(window, cx))), - ), - ) + ) + })) } } @@ -895,7 +926,7 @@ mod tests { VisualTestContext, Window, div, }; - use crate::viewer::{Scope, close_viewer, next_row, open_viewer, viewer_in}; + use crate::viewer::{Scope, build, close_viewer, next_row, open_viewer, viewer_in}; #[test] fn stepping_rows_skips_what_cannot_be_previewed_and_stops_at_the_ends() { @@ -943,6 +974,35 @@ mod tests { }); } + /// The pop-out owns its viewer. Building one must not take over the overlay's slot, and + /// closing the overlay must leave it alone. + #[gpui::test] + fn a_pop_out_viewer_and_the_overlay_do_not_touch_each_other(cx: &mut TestAppContext) { + let cx = with_window(cx); + cx.update(|window, cx| { + open_viewer( + "/nonexistent/overlay.jpg".into(), + Scope::Workspace, + window, + cx, + ); + let popped = build("/nonexistent/popped.jpg".into(), Scope::PopOut, window, cx); + let overlay = viewer_in(Scope::Workspace, cx).expect("the overlay is still open"); + assert_ne!(overlay.entity_id(), popped.entity_id()); + assert!( + viewer_in(Scope::PopOut, cx).is_none(), + "never a global viewer" + ); + + close_viewer(window, cx); + assert!(viewer_in(Scope::Workspace, cx).is_none()); + assert_eq!( + popped.read(cx).path, + std::path::PathBuf::from("/nonexistent/popped.jpg") + ); + }); + } + #[gpui::test] fn closing_the_viewer_restores_its_callers_focus(cx: &mut TestAppContext) { let cx = with_window(cx); From 2c3b3ff95178d70933ca93073344cc29fbdaae2b Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:58:16 +0000 Subject: [PATCH 02/31] fix(workspace): pop-out playback, per-project placement and pin cost - Stop playback when the pop-out leaves or closes a file, and only when the shared player is playing that file, so the main window's recording is never silenced. - Keep the window's bounds per project through MainWindowBounds, the same mechanism and `.qrate` key pattern as the main window, instead of a new app-wide setting. - Re-find a pin only when rows were added, removed or moved (one id map per structural change), and compare the grid's cursor rather than its whole selection when drawing the "Table is on row N" chip. --- crates/settings/src/lib.rs | 5 +- crates/workspace/src/pop_out.rs | 187 +++++++++++++++++++++++++------- 2 files changed, 146 insertions(+), 46 deletions(-) diff --git a/crates/settings/src/lib.rs b/crates/settings/src/lib.rs index e47eb3b5..d6d19bae 100644 --- a/crates/settings/src/lib.rs +++ b/crates/settings/src/lib.rs @@ -35,9 +35,6 @@ use crate::path_picker::PathPickerApp; /// `AppSettings` value key for the Settings window's last size (a JSON [`MainWindowBounds`]). pub const SETTINGS_WINDOW_BOUNDS_KEY: &str = "settings_window_bounds"; -/// The same for the pop-out viewer, whose display is the point of it: it reopens on the monitor -/// the archivist moved it to. -pub const POP_OUT_WINDOW_BOUNDS_KEY: &str = "pop_out_window_bounds"; /// Setting key for autosave behavior: `"timed"` (buffered, the default), `"immediate"`, /// or `"off"`. Read by the table crate to decide when a committed cell edit reaches disk. @@ -542,7 +539,7 @@ impl AppSettings { } pub fn set_text(key: &'static str, val: SharedString, cx: &mut App) { - if key != SETTINGS_WINDOW_BOUNDS_KEY && key != POP_OUT_WINDOW_BOUNDS_KEY { + if key != SETTINGS_WINDOW_BOUNDS_KEY { log::debug!("settings: app text changed key={key}"); } Self::update(cx, |s| { diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index dac92476..ccde176e 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -20,8 +20,8 @@ use gpui_component::{ table::TableState, v_flex, }; +use settings::MainWindowBounds; use settings::project::{CurrentProject, RowId}; -use settings::{AppSettings, MainWindowBounds, POP_OUT_WINDOW_BOUNDS_KEY}; use table::{QrateTableDelegate, Selection, TableChanged, TablePanelHandle, TableStateHandle}; use crate::panels::DetailsPanel; @@ -36,16 +36,24 @@ const MIN_SIZE: Size = Size { width: px(800.), height: px(600.), }; -const DEFAULT_SIZE: Size = Size { - width: px(1120.), - height: px(700.), -}; /// Height of the sidebar's header strip, the Details title or the Details | Find tabs. const HEADER_H: Pixels = px(30.); /// The stage's own text colours. Not theme colours, for the reason the backdrop is not one. const STAGE_FG: u32 = 0xe8e8e8; const STAGE_MUTED: u32 = 0xa3a3a3; +/// `.qrate` setting key for the window's last size and display, as a JSON [`MainWindowBounds`] — +/// per project, the same way the main window keeps its own. +const BOUNDS_KEY: &str = "pop_out_window_bounds"; + +/// Silence `viewer`'s recording as it goes — only its own: the player is shared by the whole app, +/// and may be playing something the main window started. +fn stop_playing(viewer: &Entity, cx: &mut App) { + if preview::playback::playing(cx) == Some(viewer.read(cx).path.as_path()) { + preview::playback::stop(cx); + } +} + /// The open pop-out window. There is one per project, and one project open at a time. #[derive(Default)] pub(crate) struct PopOutWindow(Option); @@ -66,30 +74,14 @@ pub fn open(cx: &mut App) { { return; } - // Size and display only, like the other windows: the display is the point of this one. - let saved = AppSettings::get(cx) - .values - .get(POP_OUT_WINDOW_BOUNDS_KEY) - .map(|value| value.text()) + let saved = cx + .try_global::() + .and_then(|p| settings::project::read_setting(&p.file, BOUNDS_KEY).ok()) + .flatten() .and_then(|raw| serde_json::from_str::(&raw).ok()); - let display = saved.as_ref().and_then(|b| b.display_id).and_then(|raw| { - cx.displays() - .into_iter() - .find(|display| u64::from(display.id()) == raw) - .map(|display| display.id()) - }); - let win_size = saved - .filter(|b| { - b.width.is_finite() - && b.height.is_finite() - && px(b.width) >= MIN_SIZE.width - && px(b.height) >= MIN_SIZE.height - }) - .map_or(DEFAULT_SIZE, |b| size(px(b.width), px(b.height))); + let (bounds, display) = MainWindowBounds::startup_placement(saved.as_ref(), cx); let options = WindowOptions { - window_bounds: Some(WindowBounds::Windowed(Bounds::centered( - display, win_size, cx, - ))), + window_bounds: Some(WindowBounds::Windowed(bounds)), display_id: display, window_min_size: Some(MIN_SIZE), ..TitleBar::window_options() @@ -151,12 +143,8 @@ impl PopOut { let me = window.window_handle(); cx.on_release(move |this: &mut Self, cx| { // A recording playing here would otherwise go on with nothing on screen to stop it. - if this - .viewer - .as_ref() - .is_some_and(|viewer| viewer.read(cx).transport.is_some()) - { - preview::playback::stop(cx); + if let Some(viewer) = &this.viewer { + stop_playing(viewer, cx); } // Only this window's own entry: a new pop-out may already have replaced it. if cx @@ -189,10 +177,14 @@ impl PopOut { this.sync(window, cx); } }), - cx.observe_window_bounds(window, |_, window, cx| { + // Debounced by the writer: this fires on every pixel of a drag. + cx.observe_window_bounds(window, |this, window, cx| { + let Some(file) = &this.project else { + return; + }; let bounds = MainWindowBounds::capture_from_window(window, cx); if let Ok(json) = serde_json::to_string(&bounds) { - AppSettings::set_text(POP_OUT_WINDOW_BOUNDS_KEY, json.into(), cx); + settings::project::queue_write(file, BOUNDS_KEY, &json, cx); } }), ]; @@ -245,11 +237,25 @@ impl PopOut { fn sync(&mut self, window: &mut Window, cx: &mut Context) { let rows = self.table().map_or_else(Vec::new, |state| { let delegate = state.read(cx).delegate(); + let all = delegate.row_ids(); match &self.pinned { - Some(ids) => ids - .iter() - .filter_map(|id| delegate.row_ids().iter().position(|row| row == id)) - .collect(), + // Where the pin was last found, while every row is still there: a cell edit is the + // common change, and it moves nothing. + Some(ids) + if ids.len() == self.rows.len() + && ids + .iter() + .zip(&self.rows) + .all(|(id, &row)| all.get(row) == Some(id)) => + { + self.rows.clone() + } + // Rows were added, removed or moved: one pass to find the pin again. + Some(ids) => { + let at: std::collections::HashMap<_, _> = + all.iter().enumerate().map(|(row, id)| (*id, row)).collect(); + ids.iter().filter_map(|id| at.get(id).copied()).collect() + } None => delegate.selected_source_rows(), } }); @@ -270,6 +276,9 @@ impl PopOut { .zip(self.table()) .and_then(|(row, state)| viewer::previewable(state.read(cx).delegate(), row)); if self.viewer.as_ref().map(|viewer| &viewer.read(cx).path) != file.as_ref() { + if let Some(leaving) = &self.viewer { + stop_playing(leaving, cx); + } self.viewer = file.map(|file| viewer::build(file, Scope::PopOut, window, cx)); self._viewer_sub = self .viewer @@ -443,11 +452,13 @@ impl PopOut { let table_row = self .pinned .as_ref() - .filter(|_| delegate.selected_source_rows() != self.rows) .and_then(|_| match delegate.selection() { - Some(Selection::Cell { row, .. } | Selection::Row(row)) => delegate.view_row(row), + Some(Selection::Cell { row, .. } | Selection::Row(row)) => Some(row), _ => None, }) + // Only the grid's cursor, not its whole selection: this runs on every repaint. + .filter(|row| !self.rows.contains(row)) + .and_then(|row| delegate.view_row(row)) .map(|view| view + 1); (readout, table_row) } @@ -992,7 +1003,7 @@ mod tests { use gpui_component::table::TableState; use table::{QrateTableDelegate, TableChanged}; - use super::PopOut; + use super::{PopOut, stop_playing}; /// Three rows in a real grid, and the pop-out window watching it. fn window_over_a_table( @@ -1108,6 +1119,37 @@ mod tests { details.read_with(cx, |details, cx| assert_eq!(details.picked(cx), [1])); } + /// A pin is an item, not a position: a row added above it leaves the window on the same item. + #[gpui::test] + fn a_pin_stays_on_its_item_when_rows_are_added_above_it(cx: &mut TestAppContext) { + let (pop_out, state, cx) = window_over_a_table(cx); + select(&state, &[1], cx); + pop_out.update_in(cx, |pop_out, window, cx| pop_out.toggle_pin(window, cx)); + + state.update(cx, |state, cx| { + state.delegate_mut().set_data( + &["Identifier".into(), "Title".into()], + &[10, 11, 12, 13], + &[ + vec!["ADR-0041".into(), "Added above".into()], + vec!["ADR-0042".into(), "Beacon Hill Park".into()], + vec!["ADR-0043".into(), "Sawmill crew".into()], + vec!["ADR-0044".into(), "Saanich mill".into()], + ], + ); + cx.emit(TableChanged); + }); + cx.run_until_parked(); + + pop_out.read_with(cx, |pop_out, _| { + assert_eq!( + pop_out.rows, + [2], + "row id 12 moved down one, and the pin with it" + ) + }); + } + /// Several rows: the stage steps through them, wrapping, while the sidebar keeps all of them. #[gpui::test] fn the_stack_steps_through_the_selection_and_wraps(cx: &mut TestAppContext) { @@ -1133,6 +1175,67 @@ mod tests { pop_out.read_with(cx, |pop_out, _| assert_eq!(pop_out.front(), Some(1))); } + struct Blank; + + impl gpui::Render for Blank { + fn render( + &mut self, + _: &mut gpui::Window, + _: &mut gpui::Context, + ) -> impl gpui::IntoElement { + gpui::div() + } + } + + /// Replacing or closing the pop-out's viewer stops only its own recording. The player is the + /// app's, and what it is playing may be the main window's. + /// + /// Like the viewer's playback test, this is the real path on a machine with an output device + /// and holds trivially on one without. + #[gpui::test] + fn leaving_a_file_stops_only_its_own_recording(cx: &mut TestAppContext) { + let data = 8000usize * 2; + let mut wav = Vec::new(); + wav.extend(b"RIFF"); + wav.extend((36 + data as u32).to_le_bytes()); + wav.extend(b"WAVEfmt "); + wav.extend(16u32.to_le_bytes()); + wav.extend(1u16.to_le_bytes()); + wav.extend(1u16.to_le_bytes()); + wav.extend(8000u32.to_le_bytes()); + wav.extend(16000u32.to_le_bytes()); + wav.extend(2u16.to_le_bytes()); + wav.extend(16u16.to_le_bytes()); + wav.extend(b"data"); + wav.extend((data as u32).to_le_bytes()); + wav.extend(std::iter::repeat_n(0u8, data)); + let main = std::env::temp_dir().join("qrate-pop-out-main.wav"); + let shown = std::env::temp_dir().join("qrate-pop-out-shown.wav"); + std::fs::write(&main, &wav).unwrap(); + std::fs::write(&shown, &wav).unwrap(); + + let (_, cx) = cx.add_window_view(|_, _| Blank); + cx.update(|window, cx| { + let popped = + crate::viewer::build(shown.clone(), crate::viewer::Scope::PopOut, window, cx); + preview::playback::play(&main, cx); + let before = preview::playback::playing(cx).map(|path| path.to_path_buf()); + stop_playing(&popped, cx); + assert_eq!( + preview::playback::playing(cx).map(|path| path.to_path_buf()), + before, + "the main window's recording plays on" + ); + + preview::playback::play(&shown, cx); + stop_playing(&popped, cx); + assert!(preview::playback::playing(cx).is_none(), "its own stops"); + }); + + let _ = std::fs::remove_file(&main); + let _ = std::fs::remove_file(&shown); + } + /// Nothing selected is a state of the stage, not an empty window. #[gpui::test] fn with_nothing_selected_the_window_says_so(cx: &mut TestAppContext) { From 4c3072e67385e673e5e5a2b07d4ab664f733b6bc Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:58:16 +0000 Subject: [PATCH 03/31] fix(workspace): settle viewer probes and searches; aim Details edits by id - The viewer waits briefly before probing pages or duration, and holds the task, so stepping quickly through files starts one ffmpeg rather than one per file. - Find waits for typing to settle and replaces the previous search task, so a word typed quickly runs one PDFium search. - The Details editor captures row ids and resolves them, and its column, at commit, so a row added or removed mid-edit cannot redirect the write. --- crates/workspace/src/panels/details.rs | 98 ++++++++++++++++++++++---- crates/workspace/src/viewer/find.rs | 2 + crates/workspace/src/viewer/mod.rs | 45 ++++++++---- 3 files changed, 116 insertions(+), 29 deletions(-) diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 58e3121c..fe334ec8 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -100,10 +100,11 @@ pub struct DetailsPanel { /// The field editor, shared across whichever field is open — the same one-per-panel /// arrangement the grid uses for its cell editor. editor: Entity, - /// `(source_rows, data_col)` of the field being edited, in the grid's own coordinates so a - /// filter change between opening and committing can't redirect the write. Several rows when - /// the field belongs to a bundle: one edit box writing the same value down the selection. - editing: Option<(Vec, usize, SharedString)>, + /// `(row ids, data_col, header)` of the field being edited. Ids rather than source rows, so a + /// row added or removed before the commit — from either window — can't redirect the write. + /// Several rows when the field belongs to a bundle: one edit box writing the same value down + /// the selection. + editing: Option<(Vec, usize, SharedString)>, /// Which of the selected items the preview stack is showing, and whether the pointer is over /// it — the step arrows only exist while it is, so they never cover the photo at rest. stack: usize, @@ -381,14 +382,19 @@ impl DetailsPanel { ) { // Whatever was open loses focus rather than being silently dropped. self.commit(cx); - let rows = self.picked(cx); + let picked = self.picked(cx); let located = self .state .as_ref() .and_then(|w| w.upgrade()) - .and_then(|s| s.read(cx).delegate().data_col(header)) - .filter(|_| !rows.is_empty()); - let Some(col) = located else { + .filter(|_| !picked.is_empty()) + .and_then(|s| { + let delegate = s.read(cx).delegate(); + let ids = delegate.row_ids(); + let rows = picked.iter().filter_map(|&row| ids.get(row).copied()); + Some((rows.collect::>(), delegate.data_col(header)?)) + }); + let Some((rows, col)) = located else { log::warn!("details: no column named {header} to edit"); return; }; @@ -830,15 +836,38 @@ impl DetailsPanel { /// validation and undo stay single-sourced. Clearing `editing` first keeps the `TableChanged` /// this provokes from re-entering as a second commit. fn commit(&mut self, cx: &mut Context) { - let Some((rows, col, _)) = self.editing.take() else { + let Some((ids, _, header)) = self.editing.take() else { return; }; let value = self.editor.read(cx).value().clone(); + // Resolved now, not when the editor opened: rows and columns may have moved since. + let Some(cells) = self + .state + .as_ref() + .and_then(|w| w.upgrade()) + .and_then(|state| { + let delegate = state.read(cx).delegate(); + let col = delegate.data_col(&header)?; + let at: std::collections::HashMap<_, _> = delegate + .row_ids() + .iter() + .enumerate() + .map(|(row, id)| (*id, row)) + .collect(); + let rows = ids.iter().filter_map(|id| at.get(id).copied()); + Some( + rows.map(|row| (row, col, value.clone())) + .collect::>(), + ) + }) + else { + log::warn!("details: the field being edited is gone, so the edit was dropped"); + return; + }; // One batch, so setting a field across a bundle is a single undo step — and `apply_edit` // drops the rows whose text this didn't change, so committing an untouched shared field // costs nothing. - let cells = rows.into_iter().map(|row| (row, col, value.clone())); - table::write_cells(cells.collect(), settings::history::Origin::Details, cx); + table::write_cells(cells, settings::history::Origin::Details, cx); cx.notify(); } @@ -2034,7 +2063,7 @@ mod tests { panel.edit_field(&"Title".into(), &"".into(), window, cx); assert_eq!( panel.editing, - Some((vec![0, 2], 1, "Title".into())), + Some((vec![1, 3], 1, "Title".into())), "the write is aimed at both selected items" ); panel @@ -2057,6 +2086,47 @@ mod tests { assert_eq!(titles(cx), vec!["one", "two", "three"]); } + /// A row added above the item while its field is open must not redirect the write onto + /// whichever row slid into the old position — the other window can do this mid-edit. + #[gpui::test] + fn an_edit_lands_on_its_item_after_a_row_is_added_above_it(cx: &mut TestAppContext) { + project_with_table(cx); + let state = cx.update(|cx| { + cx.try_global::() + .and_then(|h| h.0.upgrade()) + .expect("the table panel publishes its state handle") + }); + let (panel, cx) = cx.add_window_view(DetailsPanel::new); + state.update(cx, |state, cx| state.set_selected_cell(1, 2, cx)); + + panel.update_in(cx, |panel, window, cx| { + panel.edit_field(&"Title".into(), &"two".into(), window, cx); + panel.editor.update(cx, |editor, cx| { + editor.set_value("two, revised", window, cx) + }); + }); + state.update(cx, |state, _| { + state.delegate_mut().set_data( + &["Medium".into(), "Title".into()], + &[9, 1, 2, 3], + &[ + vec!["Photo".into(), "new".into()], + vec!["Film".into(), "one".into()], + vec!["Video".into(), "two".into()], + vec!["Film".into(), "three".into()], + ], + ) + }); + panel.update(cx, |panel, cx| panel.commit(cx)); + + let titles = state.read_with(cx, |s, _| { + (0..4) + .map(|row| s.delegate().cell(row, 1).cloned().unwrap_or_default()) + .collect::>() + }); + assert_eq!(titles, vec!["new", "one", "two, revised", "three"]); + } + /// The DoD: a field edited in the panel lands in the grid, and undo — the grid's own history, /// which the panel must not have bypassed — puts it back. Also pins the name→column lookup: /// each field must write its own column, not the one at its position in the list. @@ -2076,7 +2146,7 @@ mod tests { panel.edit_field(&"Title".into(), &"two".into(), window, cx); assert_eq!( panel.editing, - Some((vec![1], 1, "Title".into())), + Some((vec![2], 1, "Title".into())), "Title is data column 1" ); panel.editor.update(cx, |editor, cx| { @@ -2087,7 +2157,7 @@ mod tests { panel.edit_field(&"Medium".into(), &"Video".into(), window, cx); assert_eq!( panel.editing, - Some((vec![1], 0, "Medium".into())), + Some((vec![2], 0, "Medium".into())), "Medium is data column 0" ); panel.editing = None; diff --git a/crates/workspace/src/viewer/find.rs b/crates/workspace/src/viewer/find.rs index c40c3b33..a0841bc3 100644 --- a/crates/workspace/src/viewer/find.rs +++ b/crates/workspace/src/viewer/find.rs @@ -34,6 +34,8 @@ pub struct Find { /// Whether the document has any text to search. `None` until checked. A scan that was never /// OCR'd finds nothing for every query, and "No matches" would blame the query for it. pub layered: Option, + /// The search for `query`. Replaced by the next one, which drops it if it has not begun. + pub task: Option>, } impl Find { diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 829f41c7..c3bb5cdf 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -42,6 +42,12 @@ pub const VIEWER_CONTEXT: &str = "Viewer"; const PANEL: Pixels = px(384.); const PANEL_RANGE: std::ops::Range = px(240.)..px(720.); +/// How long a file has to stay open before its pages or duration are probed, and a query has to +/// stay typed before it is searched. Stepping through videos or typing a word then starts one +/// ffmpeg or one PDFium search, not one per file or per keystroke — neither can be stopped once +/// running. +const SETTLE: Duration = Duration::from_millis(150); + /// Which slot mounts the viewer. Two, because they answer different asks: the Details panel's /// button means "show me this as big as the window allows", while a gallery card means "show me /// this instead of the thumbnails" — the side panels stay readable beside it. @@ -151,25 +157,29 @@ pub(crate) fn build( find: Find::default(), find_open: false, split: cx.new(|_| ResizableState::default()), - }); - let probe = cx.background_executor().spawn(async move { - let seconds = preview::video_duration(&probe_path); - let pages = seconds.map_or_else( - || preview::page_count(&probe_path), - |seconds| seconds.max(1) as usize, - ); - (seconds, pages) + _probe: None, }); let weak = viewer.downgrade(); - cx.spawn(async move |cx| { - let (seconds, pages) = probe.await; + let probe = cx.spawn(async move |cx| { + cx.background_executor().timer(SETTLE).await; + let (seconds, pages) = cx + .background_executor() + .spawn(async move { + let seconds = preview::video_duration(&probe_path); + let pages = seconds.map_or_else( + || preview::page_count(&probe_path), + |seconds| seconds.max(1) as usize, + ); + (seconds, pages) + }) + .await; cx.update(|cx| { if let Some(viewer) = weak.upgrade() { viewer.update(cx, |viewer, cx| viewer.install_timeline(seconds, pages, cx)); } }); - }) - .detach(); + }); + viewer.update(cx, |viewer, _| viewer._probe = Some(probe)); viewer } @@ -293,6 +303,9 @@ pub struct Viewer { /// Swaps in the selected row's file when the selection moves, from the find bar, the arrows or /// anywhere else. _follow: Option, + /// Reads the page count or duration. Held so that a viewer replaced before [`SETTLE`] is up + /// never starts it. + _probe: Option>, } impl Viewer { @@ -442,7 +455,9 @@ impl Viewer { self.find.searching = true; let path = self.path.clone(); - cx.spawn(async move |this, cx| { + // Replacing the task drops the previous query's search if it has not started yet. + self.find.task = Some(cx.spawn(async move |this, cx| { + cx.background_executor().timer(SETTLE).await; let hits = cx .background_executor() .spawn({ @@ -463,8 +478,7 @@ impl Viewer { cx.notify(); }) .ok(); - }) - .detach(); + })); cx.notify(); } @@ -1208,6 +1222,7 @@ mod tests { open_viewer(path.clone(), Scope::Workspace, window, cx); viewer_in(Scope::Workspace, cx).expect("just opened") }); + cx.executor().advance_clock(super::SETTLE); cx.run_until_parked(); let scrubber = cx.update(|_, cx| { let scrubber = viewer From 52b215647cb0b7d166a92d17e8beb2d105ab266e Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Mon, 28 Sep 2026 21:03:12 +0000 Subject: [PATCH 04/31] fix(workspace): pop-out playback ownership, fresh bounds and pin pruning - Playback records the view whose transport started it; the pop-out stops only playback its own viewer started, so the main window playing the same file keeps going. - Keep the latest pop-out bounds in memory per project and prefer them over the debounced .qrate write, so a quick reopen gets the current size and display. - Drop deleted items from a pin when it is re-found, so later cell edits take the cheap path instead of rebuilding the id map each time. - Open at 1120x700 when nothing is saved and never below the 800x600 minimum, rather than MainWindowBounds' 600x800 main-window fallback. --- crates/preview/src/playback.rs | 19 ++++- crates/workspace/src/pop_out.rs | 102 +++++++++++++++++++---- crates/workspace/src/viewer/mod.rs | 3 +- crates/workspace/src/viewer/transport.rs | 2 +- 4 files changed, 103 insertions(+), 23 deletions(-) diff --git a/crates/preview/src/playback.rs b/crates/preview/src/playback.rs index 91612d28..3acecf90 100644 --- a/crates/preview/src/playback.rs +++ b/crates/preview/src/playback.rs @@ -14,7 +14,7 @@ use std::io::BufReader; use std::path::{Path, PathBuf}; use std::time::Duration; -use gpui::{App, Global}; +use gpui::{App, EntityId, Global}; use rodio::{Decoder, DeviceSinkBuilder, MixerDeviceSink, Player}; pub use crate::audio::duration; @@ -26,6 +26,9 @@ struct Playback { /// What was last handed to the player. There is one device and one recording, but more than /// one transport can be on screen — each has to know whether the position is even its own. playing: Option, + /// The view whose transport started it. Two windows can show the same recording, and closing + /// one must not silence the other's. + owner: Option, } impl Global for Playback {} @@ -36,9 +39,9 @@ fn player(cx: &App) -> Option<&Player> { Some(&cx.try_global::()?.player) } -/// Start `path` from the beginning, replacing whatever was playing. Opens the output device on -/// first use, and stays quiet on a machine that has none. -pub fn play(path: &Path, cx: &mut App) { +/// Start `path` from the beginning for `owner`, replacing whatever was playing. Opens the output +/// device on first use, and stays quiet on a machine that has none. +pub fn play(path: &Path, owner: EntityId, cx: &mut App) { let opened = File::open(path) .map_err(|err| err.to_string()) .and_then(|file| Decoder::new(BufReader::new(file)).map_err(|err| err.to_string())); @@ -58,11 +61,13 @@ pub fn play(path: &Path, cx: &mut App) { _device: device, player, playing: None, + owner: None, }); } let playback = cx.global_mut::(); playback.playing = Some(path.to_path_buf()); + playback.owner = Some(owner); playback.player.clear(); playback.player.append(source); playback.player.play(); @@ -74,6 +79,11 @@ pub fn playing(cx: &App) -> Option<&Path> { cx.try_global::()?.playing.as_deref() } +/// The view that started what is loaded. +pub fn owner(cx: &App) -> Option { + cx.try_global::()?.owner +} + /// Pause if playing, resume if paused. Does nothing before anything is loaded. pub fn toggle(cx: &App) { let Some(player) = player(cx) else { @@ -108,6 +118,7 @@ pub fn stop(cx: &mut App) { let playback = cx.global_mut::(); playback.player.clear(); playback.playing = None; + playback.owner = None; } } diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index ccde176e..f0dcb5f0 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -36,6 +36,10 @@ const MIN_SIZE: Size = Size { width: px(800.), height: px(600.), }; +const DEFAULT_SIZE: Size = Size { + width: px(1120.), + height: px(700.), +}; /// Height of the sidebar's header strip, the Details title or the Details | Find tabs. const HEADER_H: Pixels = px(30.); /// The stage's own text colours. Not theme colours, for the reason the backdrop is not one. @@ -46,14 +50,21 @@ const STAGE_MUTED: u32 = 0xa3a3a3; /// per project, the same way the main window keeps its own. const BOUNDS_KEY: &str = "pop_out_window_bounds"; -/// Silence `viewer`'s recording as it goes — only its own: the player is shared by the whole app, -/// and may be playing something the main window started. +/// Silence `viewer`'s recording as it goes — only if it started it: the player is shared by the +/// whole app, and the main window may be playing the same file. fn stop_playing(viewer: &Entity, cx: &mut App) { - if preview::playback::playing(cx) == Some(viewer.read(cx).path.as_path()) { + if preview::playback::owner(cx) == Some(viewer.entity_id()) { preview::playback::stop(cx); } } +/// The bounds last seen, for the project they belong to. The `.qrate` write is debounced, so a +/// window closed and reopened inside that interval would otherwise read the size it had before. +#[derive(Default)] +struct LastBounds(Option<(PathBuf, MainWindowBounds)>); + +impl Global for LastBounds {} + /// The open pop-out window. There is one per project, and one project open at a time. #[derive(Default)] pub(crate) struct PopOutWindow(Option); @@ -74,12 +85,27 @@ pub fn open(cx: &mut App) { { return; } - let saved = cx - .try_global::() - .and_then(|p| settings::project::read_setting(&p.file, BOUNDS_KEY).ok()) - .flatten() - .and_then(|raw| serde_json::from_str::(&raw).ok()); - let (bounds, display) = MainWindowBounds::startup_placement(saved.as_ref(), cx); + let file = cx.try_global::().map(|p| p.file.clone()); + let saved = file.as_ref().and_then(|file| { + let remembered = cx + .try_global::() + .and_then(|last| last.0.as_ref()) + .filter(|(of, _)| of == file) + .map(|(_, bounds)| bounds.clone()); + remembered.or_else(|| { + settings::project::read_setting(file, BOUNDS_KEY) + .ok() + .flatten() + .and_then(|raw| serde_json::from_str::(&raw).ok()) + }) + }); + // The shared placement's fallback is the main window's portrait default, narrower than this + // window's minimum; nothing saved means this window's own default. + let (bounds, display) = match saved { + Some(saved) => MainWindowBounds::startup_placement(Some(&saved), cx), + None => (Bounds::centered(None, DEFAULT_SIZE, cx), None), + }; + let bounds = Bounds::centered(display, bounds.size.max(&MIN_SIZE), cx); let options = WindowOptions { window_bounds: Some(WindowBounds::Windowed(bounds)), display_id: display, @@ -186,6 +212,7 @@ impl PopOut { if let Ok(json) = serde_json::to_string(&bounds) { settings::project::queue_write(file, BOUNDS_KEY, &json, cx); } + cx.set_global(LastBounds(Some((file.clone(), bounds)))); }), ]; @@ -235,6 +262,7 @@ impl PopOut { /// Bring the rows, the stage and the sidebar up to date with the grid. fn sync(&mut self, window: &mut Window, cx: &mut Context) { + let mut kept = None; let rows = self.table().map_or_else(Vec::new, |state| { let delegate = state.read(cx).delegate(); let all = delegate.row_ids(); @@ -250,15 +278,24 @@ impl PopOut { { self.rows.clone() } - // Rows were added, removed or moved: one pass to find the pin again. + // Rows were added, removed or moved: one pass to find the pin again, dropping the + // items that are gone so the pass above matches again from the next change. Some(ids) => { let at: std::collections::HashMap<_, _> = all.iter().enumerate().map(|(row, id)| (*id, row)).collect(); - ids.iter().filter_map(|id| at.get(id).copied()).collect() + let (ids, rows): (Vec<_>, Vec<_>) = ids + .iter() + .filter_map(|id| Some((*id, *at.get(id)?))) + .unzip(); + kept = Some(ids); + rows } None => delegate.selected_source_rows(), } }); + if kept.is_some() { + self.pinned = kept; + } // Every pinned item deleted: there is nothing left to hold, so follow again. if self.pinned.is_some() && rows.is_empty() { self.pinned = None; @@ -1150,6 +1187,37 @@ mod tests { }); } + /// An item deleted out of a pin of several leaves the pin holding only what is left, so the + /// next cell edit takes the cheap path rather than re-finding every row. + #[gpui::test] + fn a_deleted_item_leaves_the_pin(cx: &mut TestAppContext) { + let (pop_out, state, cx) = window_over_a_table(cx); + select(&state, &[0, 2], cx); + pop_out.update_in(cx, |pop_out, window, cx| pop_out.toggle_pin(window, cx)); + + state.update(cx, |state, cx| { + state.delegate_mut().set_data( + &["Identifier".into(), "Title".into()], + &[12, 13], + &[ + vec!["ADR-0043".into(), "Sawmill crew".into()], + vec!["ADR-0044".into(), "Saanich mill".into()], + ], + ); + cx.emit(TableChanged); + }); + cx.run_until_parked(); + + pop_out.read_with(cx, |pop_out, _| { + assert_eq!( + pop_out.pinned.as_deref(), + Some(&[13][..]), + "11 is gone from the pin" + ); + assert_eq!(pop_out.rows, [1]); + }); + } + /// Several rows: the stage steps through them, wrapping, while the sidebar keeps all of them. #[gpui::test] fn the_stack_steps_through_the_selection_and_wraps(cx: &mut TestAppContext) { @@ -1209,30 +1277,30 @@ mod tests { wav.extend(b"data"); wav.extend((data as u32).to_le_bytes()); wav.extend(std::iter::repeat_n(0u8, data)); - let main = std::env::temp_dir().join("qrate-pop-out-main.wav"); let shown = std::env::temp_dir().join("qrate-pop-out-shown.wav"); - std::fs::write(&main, &wav).unwrap(); std::fs::write(&shown, &wav).unwrap(); let (_, cx) = cx.add_window_view(|_, _| Blank); cx.update(|window, cx| { let popped = crate::viewer::build(shown.clone(), crate::viewer::Scope::PopOut, window, cx); - preview::playback::play(&main, cx); + // The same recording, open in the main window too. + let main = + crate::viewer::build(shown.clone(), crate::viewer::Scope::Workspace, window, cx); + preview::playback::play(&shown, main.entity_id(), cx); let before = preview::playback::playing(cx).map(|path| path.to_path_buf()); stop_playing(&popped, cx); assert_eq!( preview::playback::playing(cx).map(|path| path.to_path_buf()), before, - "the main window's recording plays on" + "the main window's playback of the same file plays on" ); - preview::playback::play(&shown, cx); + preview::playback::play(&shown, popped.entity_id(), cx); stop_playing(&popped, cx); assert!(preview::playback::playing(cx).is_none(), "its own stops"); }); - let _ = std::fs::remove_file(&main); let _ = std::fs::remove_file(&shown); } diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index ea9eb68c..127225b7 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -1228,7 +1228,8 @@ mod tests { cx.update(|window, cx| { open_viewer(path.clone(), Scope::Workspace, window, cx); - preview::playback::play(&path, cx); + let viewer = viewer_in(Scope::Workspace, cx).expect("just opened"); + preview::playback::play(&path, viewer.entity_id(), cx); close_viewer(window, cx); assert!( !preview::playback::position(cx).is_some_and(|(_, playing)| playing), diff --git a/crates/workspace/src/viewer/transport.rs b/crates/workspace/src/viewer/transport.rs index 9b15d54e..8f02e463 100644 --- a/crates/workspace/src/viewer/transport.rs +++ b/crates/workspace/src/viewer/transport.rs @@ -154,7 +154,7 @@ pub fn toggle(this: &mut V, window: &mut Window, cx: &mut Context) { if transport.is_current(cx) { preview::playback::toggle(cx); } else { - preview::playback::play(&path, cx); + preview::playback::play(&path, cx.entity_id(), cx); } if needs_tick { From ac1770ee6027feb66f1d4d573ca3438a93909460 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:50:39 -0700 Subject: [PATCH 05/31] feat(workspace): animate GIFs in the viewer, rotate, and actual-size zoom The viewer's image had no element id, so gpui kept no frame clock and a GIF stood on its first frame. Only the fullscreen viewer animates now; the Details pane and gallery get the first frame through the cached ladder. - Thumbnails honour EXIF orientation, as gpui already did for the viewer; the thumbnail cache key is salted so sideways entries are rebuilt - Captions carry pixel dimensions, read from the header - Viewer: quarter-turn rotation (R / Shift+R), a zoom readout that toggles fit and 1:1, double-click to zoom, pan clamped to the image, and open/closed hand cursors while it can be panned --- crates/preview/src/cache.rs | 5 + crates/preview/src/lib.rs | 168 +++++++++++++++++++------ crates/workspace/src/viewer/mod.rs | 192 ++++++++++++++++++++++++++++- 3 files changed, 325 insertions(+), 40 deletions(-) diff --git a/crates/preview/src/cache.rs b/crates/preview/src/cache.rs index 2ac9ba0e..4b083b55 100644 --- a/crates/preview/src/cache.rs +++ b/crates/preview/src/cache.rs @@ -52,6 +52,10 @@ pub fn dir() -> Option { .clone() } +/// Bumped when a decode changes what an unchanged file looks like, so every old entry misses. +/// 1: raster thumbnails turned upright by their EXIF orientation. +const FORMAT: u32 = 1; + /// Identity of one cached rendering. The file's length and mtime are in the hash, so editing or /// replacing a source file misses rather than serving the old picture — which is why nothing here /// needs an invalidation pass. @@ -61,6 +65,7 @@ pub fn dir() -> Option { pub fn key(path: &Path, max_edge: u32, page: usize) -> Option { let meta = fs::metadata(path).ok()?; let mut hasher = DefaultHasher::new(); + FORMAT.hash(&mut hasher); path.hash(&mut hasher); meta.len().hash(&mut hasher); meta.modified().ok()?.hash(&mut hasher); diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index 608e13fa..63b4d79b 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -135,8 +135,9 @@ pub fn placeholder_icon(path: Option<&Path>) -> IconName { pub struct Preview; /// File, size cap, where in it (the page for a document, whole seconds in for a video, zero for -/// everything else, which has only one thing to show), and the [`generation`] it was drawn at. -type Key = (PathBuf, u32, usize, u64); +/// everything else, which has only one thing to show), the [`generation`] it was drawn at, and how +/// many quarter turns clockwise the viewer has rotated it. +type Key = (PathBuf, u32, usize, u64, u8); /// For the files PDFium and ffmpeg draw, the component generation, so installing either gives /// them new keys and a card that fell back to an icon is drawn again. Zero for everything else, @@ -154,13 +155,13 @@ impl Asset for Preview { type Output = Option>; fn load( - (path, max_edge, page, _): Self::Source, + (path, max_edge, page, _, turns): Self::Source, cx: &mut App, ) -> impl Future + Send + 'static { let executor = cx.background_executor().clone(); async move { executor - .spawn(async move { render(&path, max_edge, page) }) + .spawn(async move { render(&path, max_edge, page, turns) }) .await } } @@ -284,13 +285,41 @@ fn tiff_page(path: &Path, page: usize) -> Option { /// One stat per call, so call it when the selection changes rather than per frame. pub fn describe(path: &Path) -> Option { let kind = extension(path).map(|extension| extension.to_uppercase()); + let pixels = dimensions(path).map(|(width, height)| format!("{width} × {height}")); let size = std::fs::metadata(path) .ok() .map(|metadata| file_size(metadata.len())); - let parts: Vec = [kind, size].into_iter().flatten().collect(); + let parts: Vec = [kind, pixels, size].into_iter().flatten().collect(); (!parts.is_empty()).then(|| parts.join(" · ")) } +/// Width and height in pixels, upright, read from the header alone. `None` for anything that is not +/// one of the raster formats `image` reads, which have no pixel size of their own to report. +pub fn dimensions(path: &Path) -> Option<(u32, u32)> { + use image::ImageDecoder as _; + use image::metadata::Orientation::{Rotate90, Rotate90FlipH, Rotate270, Rotate270FlipH}; + + if !extension(path).is_some_and(|extension| is_raster(&extension)) { + return None; + } + let mut decoder = image::ImageReader::open(path) + .ok()? + .with_guessed_format() + .ok()? + .into_decoder() + .ok()?; + let (width, height) = decoder.dimensions(); + let sideways = matches!( + decoder.orientation(), + Ok(Rotate90 | Rotate270 | Rotate90FlipH | Rotate270FlipH) + ); + Some(if sideways { + (height, width) + } else { + (width, height) + }) +} + /// `2.4 MB`. Powers of 1024 with the unit names every file manager on the three platforms shows, /// and whole bytes below a kilobyte — "0.3 KB" reads as a rounding of something, not as a stub. pub fn file_size(bytes: u64) -> String { @@ -411,10 +440,17 @@ pub fn thumbnail_png(path: &Path, page: usize) -> Option> { Some(encoded.into_inner()) } -/// Decode `path`, shrink it to fit `max_edge`, and hand back something gpui can draw. Runs on a -/// background thread; `None` for anything that won't decode, which the caller turns into the icon. -fn render(path: &Path, max_edge: u32, page: usize) -> Option> { - let mut bgra = thumbnail_pixels(path, max_edge, page)?; +/// Decode `path`, shrink it to fit `max_edge`, turn it `turns` quarter turns clockwise, and hand +/// back something gpui can draw. Runs on a background thread; `None` for anything that won't +/// decode, which the caller turns into the icon. +fn render(path: &Path, max_edge: u32, page: usize, turns: u8) -> Option> { + let upright = thumbnail_pixels(path, max_edge, page)?; + let mut bgra = match turns % 4 { + 1 => image::imageops::rotate90(&upright), + 2 => image::imageops::rotate180(&upright), + 3 => image::imageops::rotate270(&upright), + _ => upright, + }; // `RenderImage` is documented as BGRA and gpui only swaps inside its own decode path, so an // image built by hand has to arrive already swapped or every preview draws blue-for-red. for px in bgra.pixels_mut() { @@ -520,13 +556,24 @@ fn decode(path: &Path, max_edge: u32, page: usize) -> Option Option { + use image::ImageDecoder as _; + image::ImageReader::open(path) .ok()? .with_guessed_format() .ok()? - .decode() + .into_decoder() + .and_then(|mut decoder| { + let orientation = decoder + .orientation() + .unwrap_or(image::metadata::Orientation::NoTransforms); + let mut image = image::DynamicImage::from_decoder(decoder)?; + image.apply_orientation(orientation); + Ok(image) + }) .map_err(|err| log::warn!("could not decode {}: {err}", path.display())) .ok() } @@ -567,8 +614,8 @@ struct Live { impl Global for Live {} fn cost(image: &RenderImage) -> usize { - // ponytail: frame 0 only, so an animated GIF is undercounted. Costs accuracy on a format the - // budget already tolerates; revisit if animations become common in collections. + // Frame 0 is the whole cost: the ladder builds one frame, and the animated original goes + // through gpui's loader instead. image.as_bytes(0).map_or(0, <[u8]>::len) } @@ -665,7 +712,7 @@ pub fn forget(path: &Path, cx: &mut App) { } } let at = extension(path).map_or(0, |extension| generation(&extension)); - let unheld = [CARD, PANE, FULL].map(|edge| (path.to_path_buf(), edge, 0, at)); + let unheld = [CARD, PANE, FULL].map(|edge| (path.to_path_buf(), edge, 0, at, 0)); for key in held.iter().chain(&unheld) { cx.remove_asset::(key); } @@ -719,7 +766,7 @@ pub fn thumb(path: Option<&Path>, max_edge: u32, fit: ObjectFit, cx: &App) -> An // For contain, keep the image's intrinsic ratio under `max_w/h_full` so it can // letterbox. Cover gives the image the frame's full size so GPUI crops it. Some(path) => { - let image = img(source(path, max_edge, 0)) + let image = img(source(path, max_edge, 0, 0)) .object_fit(fit) .with_fallback(placeholder); frame.child(if cover { @@ -739,13 +786,15 @@ pub fn thumb(path: Option<&Path>, max_edge: u32, fit: ObjectFit, cx: &App) -> An /// /// gpui's own loader gets the file whenever it can read it and nothing has to be shrunk, so what /// the fullscreen viewer draws is the original: animation intact, no round trip through our decode -/// and BGRA swap, no second interpretation of a file gpui already understands. +/// and BGRA swap, no second interpretation of a file gpui already understands. It is also the only +/// place a GIF moves: a card or the details pane gets its first frame, shrunk and cached. /// /// - **SVG at every size.** It is the one format [`can_preview`] accepts that the `image` crate /// cannot decode — gpui rasterises it through the `resvg` it already vendors — and vector files /// are small enough that neither the downscale nor the disk cache would earn its keep. -/// - **Raster at [`FULL`] only**, i.e. the viewer. A card or a details pane wants the capped, -/// cached copy; there is nothing to cap here. +/// - **Raster at [`FULL`] only**, i.e. the viewer, while it is upright. A card or a details pane +/// wants the capped, cached copy; there is nothing to cap here. A turned image has to be turned +/// by us, so a rotated GIF stands still. /// /// Everything else goes through [`Preview`] as a custom source — the formats gpui cannot read at /// all (PDF, RAW, video, audio artwork, whatever only the OS can thumbnail), and every capped @@ -757,18 +806,21 @@ pub fn thumb(path: Option<&Path>, max_edge: u32, fit: ObjectFit, cx: &App) -> An /// so a session spent opening one large scan after another keeps every one of them. Acceptable /// while the viewer shows one at a time; give the viewer an explicit `drop_image` on close if it /// ever shows up in a memory profile. -pub fn source(path: &Path, max_edge: u32, page: usize) -> ImageSource { +pub fn source(path: &Path, max_edge: u32, page: usize, turns: u8) -> ImageSource { let extension = extension(path).unwrap_or_default(); - // ponytail: a GIF goes to gpui whole at every size, because only gpui's own decode keeps the - // frames that animate it. It skips the thumbnail cache and the memory budget, so a gallery of - // large GIFs holds them all; downscale every frame here if that shows up. - if extension == "svg" - || extension == "gif" - || (max_edge == FULL && page == 0 && is_raster(&extension)) + let turns = turns % 4; + if turns == 0 + && (extension == "svg" || (max_edge == FULL && page == 0 && is_raster(&extension))) { return ImageSource::Resource(path.to_path_buf().into()); } - let key = (path.to_path_buf(), max_edge, page, generation(&extension)); + let key = ( + path.to_path_buf(), + max_edge, + page, + generation(&extension), + turns, + ); ImageSource::Custom(Arc::new(move |window: &mut Window, cx: &mut App| { // `None` while the decode is still running, which leaves the frame empty rather than // flashing the icon; gpui re-renders the view when the task lands. @@ -866,11 +918,11 @@ mod tests { fn the_viewer_gets_the_original_file_and_everything_else_gets_the_ladder() { use gpui::ImageSource; - use crate::{CARD, FULL, source}; + use crate::{CARD, FULL, PANE, source}; let native = |p: &str, max_edge, page| { matches!( - source(Path::new(p), max_edge, page), + source(Path::new(p), max_edge, page, 0), ImageSource::Resource(_) ) }; @@ -884,9 +936,15 @@ mod tests { assert!(native("/f/logo.svg", CARD, 0)); assert!(native("/f/logo.svg", FULL, 0)); + // Only the viewer animates; the Details pane and a card get the cached first frame. + assert!(!native("/f/anim.gif", PANE, 0)); + assert!(!native("/f/anim.gif", CARD, 0)); assert!( - native("/f/anim.gif", CARD, 0), - "the Details pane animates too" + !matches!( + source(Path::new("/f/anim.gif"), FULL, 0, 1), + ImageSource::Resource(_) + ), + "a turned image is turned by the ladder" ); // Capped sizes stay on the ladder — a card wants the shrunk, disk-cached copy. assert!(!native("/f/scan.jpg", CARD, 0)); @@ -1128,7 +1186,7 @@ mod tests { .save(&red) .unwrap(); - let rendered = crate::render(&red, crate::CARD, 0).expect("a 4x4 png decodes"); + let rendered = crate::render(&red, crate::CARD, 0, 0).expect("a 4x4 png decodes"); let bytes = rendered.as_bytes(0).expect("one frame"); assert_eq!( &bytes[..4], @@ -1147,7 +1205,7 @@ mod tests { image::RgbaImage::from_pixel(900, 300, image::Rgba([1, 2, 3, 255])) .save(&big) .unwrap(); - let rendered = crate::render(&big, 256, 0).expect("decodes"); + let rendered = crate::render(&big, 256, 0, 0).expect("decodes"); let size = rendered.size(0); assert_eq!(i32::from(size.width), 256, "longest edge is capped"); assert_eq!(i32::from(size.height), 85, "aspect ratio preserved"); @@ -1156,7 +1214,7 @@ mod tests { image::RgbaImage::from_pixel(40, 20, image::Rgba([1, 2, 3, 255])) .save(&small) .unwrap(); - let rendered = crate::render(&small, 512, 0).expect("decodes"); + let rendered = crate::render(&small, 512, 0, 0).expect("decodes"); assert_eq!(i32::from(rendered.size(0).width), 40, "never enlarged"); let _ = std::fs::remove_file(&big); @@ -1175,7 +1233,7 @@ mod tests { let entry = crate::cache::dir().expect("cache dir").join(&key); let _ = std::fs::remove_file(&entry); - crate::render(&path, 128, 0).expect("decodes"); + crate::render(&path, 128, 0, 0).expect("decodes"); assert!(entry.is_file(), "the first decode leaves an entry behind"); assert!(crate::cache::read(&key).is_some(), "and it reads back"); @@ -1212,7 +1270,7 @@ mod tests { cx.update(|window, cx| { for n in 0..8 { - let key = (PathBuf::from(format!("/f/{n}.png")), crate::CARD, 0, 0); + let key = (PathBuf::from(format!("/f/{n}.png")), crate::CARD, 0, 0, 0); crate::retain(&key, &image(), budget, window, cx); } assert_eq!( @@ -1261,12 +1319,48 @@ mod tests { }); } + /// A phone photo stored sideways with an EXIF turn has to come out upright in every thumbnail, + /// as it does in the viewer, where gpui applies the tag itself. The viewer's own rotation is + /// applied on top of that, never instead of it. + #[test] + fn exif_orientation_and_the_viewers_turn_both_reach_the_pixels() { + use image::ImageEncoder as _; + + let path = std::env::temp_dir().join("qrate-exif-orientation-probe.png"); + // A little-endian TIFF header with one IFD entry: Orientation (0x0112) = 6, "rotate 90°". + let exif = vec![ + 0x49, 0x49, 0x2A, 0, 8, 0, 0, 0, 1, 0, 0x12, 0x01, 3, 0, 1, 0, 0, 0, 6, 0, 0, 0, 0, 0, + 0, 0, + ]; + let mut encoder = + image::codecs::png::PngEncoder::new(std::fs::File::create(&path).unwrap()); + encoder.set_exif_metadata(exif).unwrap(); + encoder + .write_image(&[0u8; 4 * 2 * 4], 4, 2, image::ExtendedColorType::Rgba8) + .unwrap(); + + assert_eq!(crate::dimensions(&path), Some((2, 4)), "reported upright"); + let upright = crate::render(&path, crate::CARD, 0, 0).expect("decodes"); + assert_eq!(i32::from(upright.size(0).width), 2, "drawn upright"); + let turned = crate::render(&path, crate::CARD, 0, 1).expect("decodes"); + assert_eq!( + i32::from(turned.size(0).width), + 4, + "then turned by the viewer" + ); + + if let Some(key) = crate::cache::key(&path, crate::CARD, 0) { + let _ = std::fs::remove_file(crate::cache::dir().unwrap().join(key)); + } + let _ = std::fs::remove_file(&path); + } + #[test] fn undecodable_files_render_nothing_rather_than_panicking() { let junk = std::env::temp_dir().join("qrate-junk-probe.jpg"); std::fs::write(&junk, b"this is not an image").unwrap(); - assert!(crate::render(&junk, crate::CARD, 0).is_none()); - assert!(crate::render(Path::new("/nonexistent/x.png"), crate::CARD, 0).is_none()); + assert!(crate::render(&junk, crate::CARD, 0, 0).is_none()); + assert!(crate::render(Path::new("/nonexistent/x.png"), crate::CARD, 0, 0).is_none()); let _ = std::fs::remove_file(&junk); } } diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 127225b7..6faa666a 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -103,6 +103,7 @@ pub(crate) fn build( let document = preview::has_text(&path); let video = preview::has_video(&path); let details = preview::describe(&path); + let pixels = preview::dimensions(&path); let probe_path = path.clone(); let table = cx .try_global::() @@ -154,6 +155,9 @@ pub(crate) fn build( zoom: 1.0, offset: Point::default(), drag_from: None, + turns: 0, + pixels, + scale: 1.0, frame: Rc::default(), focus_handle: cx.focus_handle(), focused: false, @@ -289,6 +293,12 @@ pub struct Viewer { offset: Point, /// Last pointer position while dragging; `None` when not panning. drag_from: Option>, + /// Quarter turns clockwise, for a scan that was fed in sideways. A view, never saved. + turns: u8, + /// Upright pixel size, where the header says one — what "actual size" is measured against. + pixels: Option<(u32, u32)>, + /// The window's scale factor at the last render, so 1:1 means one image pixel per device pixel. + scale: f32, /// Window-space rect of the content box, from `canvas` prepaint — where scroll-zoom's anchor /// is measured from. frame: Rc>>, @@ -350,13 +360,80 @@ impl Viewer { /// `anchor` (relative to the frame's centre) still, and recenter once the image is no bigger /// than its frame, where there's nothing to pan to. fn set_zoom(&mut self, zoom: f32, anchor: Point) { - let zoom = zoom.clamp(0.1, 8.0); + let most = self.actual_size().map_or(8.0, |actual| actual.max(8.0)); + let zoom = zoom.clamp(0.1, most); let scale = zoom / self.zoom; self.offset = anchor - (anchor - self.offset) * scale; self.zoom = zoom; if self.zoom <= 1.0 { self.offset = Point::default(); } + self.clamp_pan(); + } + + /// The picture's pixel size as turned, and the scale that fits it to the frame. `None` where + /// the file has no pixel size of its own, or before the frame has been laid out. + fn fit(&self) -> Option<(Size, f32)> { + let (width, height) = self.pixels?; + let image = match self.turns % 2 { + 0 => size(width as f32, height as f32), + _ => size(height as f32, width as f32), + }; + let frame = self.frame.get().size; + let fit = + (f32::from(frame.width) / image.width).min(f32::from(frame.height) / image.height); + (fit > 0.0).then_some((image, fit)) + } + + /// The zoom at which one pixel of the image is one pixel of the screen. + fn actual_size(&self) -> Option { + self.fit().map(|(_, fit)| 1.0 / (fit * self.scale)) + } + + /// How far the picture overhangs its frame on each side — the most it may be panned. Measured + /// on the image itself where its size is known, so the letterbox bars are not pannable. + fn slack(&self) -> Point { + let frame = self.frame.get().size; + let shown = self.fit().map_or(frame, |(image, fit)| { + size(px(image.width * fit), px(image.height * fit)) + }); + point( + ((shown.width * self.zoom - frame.width) / 2.).max(px(0.)), + ((shown.height * self.zoom - frame.height) / 2.).max(px(0.)), + ) + } + + /// Keep an edge of the picture on its frame's edge, so a drag cannot lose it off screen. Left + /// alone before the frame has a size, which is only ever the case before the first paint. + fn clamp_pan(&mut self) { + if self.frame.get().size.width <= px(0.) { + return; + } + let slack = self.slack(); + self.offset.x = self.offset.x.clamp(-slack.x, slack.x); + self.offset.y = self.offset.y.clamp(-slack.y, slack.y); + } + + /// A quarter turn clockwise, or back with `-1`. Starts from fit: the old zoom and pan were aimed + /// at a picture of a different shape. + fn rotate(&mut self, delta: i8) { + self.turns = (self.turns as i8 + delta).rem_euclid(4) as u8; + self.zoom = 1.0; + self.offset = Point::default(); + } + + /// Fit when zoomed, actual size when fitted; 2× for a file with no pixel size of its own, or + /// whose actual size is the fit. + fn toggle_zoom(&mut self, anchor: Point) { + if (self.zoom - 1.0).abs() > 0.01 { + self.set_zoom(1.0, anchor); + return; + } + let target = self + .actual_size() + .filter(|actual| (actual - 1.0).abs() > 0.05) + .unwrap_or(2.0); + self.set_zoom(target, anchor); } /// Move `delta` pages, stopping at either end rather than wrapping — a document has a first @@ -512,6 +589,7 @@ impl Render for Viewer { window.focus(&self.focus_handle, cx); self.focused = true; } + self.scale = window.scale_factor(); let (zoom, offset, page, pages) = (self.zoom, self.offset, self.page, self.pages); let name: SharedString = self .path @@ -532,7 +610,20 @@ impl Render for Viewer { }; let pill = cx.theme().background.opacity(0.8); let accent = cx.theme().primary.opacity(0.55); - let marks: Vec = self.find.on_page(page).cloned().collect(); + // A hit's box is measured on the upright page, so a turned page shows none. + let marks: Vec = match self.turns { + 0 => self.find.on_page(page).cloned().collect(), + _ => Vec::new(), + }; + let cursor = match (self.drag_from.is_some(), self.slack()) { + (true, _) => CursorStyle::ClosedHand, + (false, slack) if slack.x > px(0.) || slack.y > px(0.) => CursorStyle::OpenHand, + _ => CursorStyle::Arrow, + }; + let readout = match self.actual_size() { + Some(actual) => format!("{:.0}%", zoom / actual * 100.), + None => format!("{:.0}%", zoom * 100.), + }; // The panel's *live* width, straight off the resizable's state, so the rows re-trim as it // is dragged. Empty until the group has laid out once. let panel_width = self.split.read(cx).sizes().get(1).copied().unwrap_or(PANEL); @@ -611,6 +702,16 @@ impl Render for Viewer { this.set_zoom(1.0, Point::default()); cx.notify(); } + "1" if reading => { + if let Some(actual) = this.actual_size() { + this.set_zoom(actual, Point::default()); + cx.notify(); + } + } + "r" if reading => { + this.rotate(if ev.keystroke.modifiers.shift { -1 } else { 1 }); + cx.notify(); + } "up" | "down" if reading && overlay => { step_row(if ev.keystroke.key == "up" { -1 } else { 1 }, cx); } @@ -639,9 +740,14 @@ impl Render for Viewer { cx.notify(); }, )) + .cursor(cursor) .on_mouse_down( MouseButton::Left, cx.listener(|this, ev: &MouseDownEvent, _, cx| { + if ev.click_count == 2 { + let anchor = ev.position - this.frame.get().center(); + this.toggle_zoom(anchor); + } this.drag_from = Some(ev.position); cx.notify(); }), @@ -652,6 +758,7 @@ impl Render for Viewer { }; this.offset.x += ev.position.x - last.x; this.offset.y += ev.position.y - last.y; + this.clamp_pan(); this.drag_from = Some(ev.position); cx.notify(); })) @@ -662,6 +769,14 @@ impl Render for Viewer { cx.notify(); }), ) + // Released over a panel or outside the window, the drag still ends. + .on_mouse_up_out( + MouseButton::Left, + cx.listener(|this, _: &MouseUpEvent, _, cx| { + this.drag_from = None; + cx.notify(); + }), + ) .child( // The positioned content box. The overlay measures *this* // element, so the highlight maths never has to know about the @@ -682,7 +797,11 @@ impl Render for Viewer { }) .children((!bare).then(|| { // `flex_shrink_0` keeps `relative(zoom)` past 1. - img(preview::source(&self.path, cap, page)) + // The id is what lets gpui keep a GIF's frame clock. + img(preview::source( + &self.path, cap, page, self.turns, + )) + .id("viewer-image") .flex_shrink_0() .relative() .w(relative(zoom)) @@ -917,6 +1036,19 @@ impl Render for Viewer { })), ) }) + .when(!bare, |group| { + group.child( + Button::new("rotate") + .icon(IconName::RotateCw) + .ghost() + .small() + .tooltip("Rotate (R, Shift+R back)") + .on_click(cx.listener(|this, _, _, cx| { + this.rotate(1); + cx.notify(); + })), + ) + }) .child( Button::new("zoom-out") .icon(IconName::Minus) @@ -928,6 +1060,21 @@ impl Render for Viewer { cx.notify(); })), ) + // Percent of actual size where the file has one, else of the fit. + .child( + Button::new("zoom-readout") + .label(readout) + .ghost() + .small() + .tooltip(match self.actual_size() { + Some(_) => "Toggle fit (0) and actual size (1)", + None => "Toggle fit (0) and 2×", + }) + .on_click(cx.listener(|this, _, _, cx| { + this.toggle_zoom(Point::default()); + cx.notify(); + })), + ) .child( Button::new("zoom-in") .icon(IconName::Plus) @@ -1115,6 +1262,45 @@ mod tests { }); } + /// A drag cannot lose the picture off screen, "actual size" follows the turn, and a turn starts + /// again from fit. + #[gpui::test] + fn panning_stops_at_the_edge_and_a_turn_swaps_the_actual_size(cx: &mut TestAppContext) { + let cx = with_window(cx); + let path = std::path::PathBuf::from("/nonexistent/qrate-pan-test.png"); + cx.update(|window, cx| { + open_viewer(path, Scope::Workspace, window, cx); + let viewer = viewer_in(Scope::Workspace, cx).expect("just opened"); + viewer.update(cx, |viewer, _| { + // A 2000×1000 picture in a 1000×500 frame: fitted at half scale. + viewer.pixels = Some((2000, 1000)); + viewer.frame.set(gpui::Bounds::new( + gpui::Point::default(), + gpui::size(gpui::px(1000.), gpui::px(500.)), + )); + assert_eq!(viewer.actual_size(), Some(2.0)); + + viewer.set_zoom(2.0, gpui::Point::default()); + viewer.offset = gpui::point(gpui::px(5000.), gpui::px(-5000.)); + viewer.clamp_pan(); + assert_eq!( + viewer.offset, + gpui::point(gpui::px(500.), gpui::px(-250.)), + "no further than the overhang on either side" + ); + + viewer.rotate(1); + assert_eq!(viewer.zoom, 1.0); + assert_eq!(viewer.offset, gpui::Point::default()); + // Now 1000×2000 in the same frame, fitted at a quarter. + assert_eq!(viewer.actual_size(), Some(4.0)); + viewer.rotate(-2); + assert_eq!(viewer.turns, 3, "a turn back from upright wraps"); + }); + close_viewer(window, cx); + }); + } + /// Paging has to stop at both ends. Wrapping past the last page loses the reader's place, and /// an underflow on page zero would panic on a `usize` subtraction. #[gpui::test] From 9772ea9c3ec23dbb1fdb73a71eb4b34f7f386e87 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:54:06 -0700 Subject: [PATCH 06/31] fix(preview): round thumbnail images so corners stay inside their frame overflow_hidden clips to a rectangle in gpui, so a Cover image's square corners showed past the launcher's rounded border. --- crates/preview/src/lib.rs | 1 + crates/project-wizard/src/launcher.rs | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index 63b4d79b..613fb662 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -768,6 +768,7 @@ pub fn thumb(path: Option<&Path>, max_edge: u32, fit: ObjectFit, cx: &App) -> An Some(path) => { let image = img(source(path, max_edge, 0, 0)) .object_fit(fit) + .rounded(cx.theme().radius) .with_fallback(placeholder); frame.child(if cover { image.size_full() diff --git a/crates/project-wizard/src/launcher.rs b/crates/project-wizard/src/launcher.rs index 67b15ca9..56dc21db 100644 --- a/crates/project-wizard/src/launcher.rs +++ b/crates/project-wizard/src/launcher.rs @@ -652,7 +652,7 @@ fn project_thumbnail(image: Thumbnail<'_>, size: Pixels, cx: &App) -> AnyElement let frame = div() .size(size) .flex_none() - .rounded_md() + .rounded(cx.theme().radius) .border_1() .border_color(cx.theme().border) .bg(cx.theme().tiles) @@ -665,7 +665,8 @@ fn project_thumbnail(image: Thumbnail<'_>, size: Pixels, cx: &App) -> AnyElement example::THUMBNAIL.to_vec(), ))) .size_full() - .object_fit(ObjectFit::Cover), + .object_fit(ObjectFit::Cover) + .rounded(cx.theme().radius), ) .into_any_element(), Thumbnail::Recent(Some(path)) => frame From 4d1800467fa82ca7609238302e94e15d464f4515 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:02:20 -0700 Subject: [PATCH 07/31] fix(workspace): tint the pop-out button when open and truncate the caption The pop-out button now lights its glyph like the status bar instead of filling a box. The caption chip caps its width and ellipsizes, so a narrow pane cannot run it under the action buttons. --- crates/workspace/src/panels/details.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 42810ad4..af422540 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -7,7 +7,7 @@ use std::time::Duration; use gpui::prelude::FluentBuilder as _; use gpui::*; use gpui_component::{ - ActiveTheme, Icon, IconName, Selectable as _, Sizable, StyledExt as _, + ActiveTheme, Icon, IconName, Sizable, StyledExt as _, button::{Button, ButtonVariants}, dock::{BasePanel, DockPlacement, Panel, PanelEvent}, h_flex, @@ -1080,10 +1080,13 @@ fn render_image_frame( .child({ let open = crate::pop_out::is_open(cx); Button::new("pop-out") - .icon(Icon::empty().path("icons/app-window.svg")) + .icon( + Icon::empty() + .path("icons/app-window.svg") + .when(open, |icon| icon.text_color(cx.theme().primary)), + ) .ghost() .small() - .selected(open) .tooltip(match open { true => "Show pop-out window", false => "Open in new window", @@ -1115,12 +1118,17 @@ fn render_image_frame( .absolute() .top_1() .left_1() + // The action chip opposite takes the rest; a narrow pane truncates rather than overlaps. + .max_w(relative(0.4)) .px_1p5() .py_0p5() .rounded(cx.theme().radius) .bg(cx.theme().background) .text_xs() .text_color(cx.theme().foreground) + .whitespace_nowrap() + .overflow_hidden() + .text_ellipsis() .child(caption) })) // Along the bottom of the frame, over the cover art rather than beside it: the pane is a From 05979869b907cc14c0b29afa658b18ab4f5e9d59 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:02:25 -0700 Subject: [PATCH 08/31] fix(workspace): keep the picture up while a turn decodes; pan when zoomed out A turn re-decodes the file, since gpui cannot rotate an image sprite, so the stage went blank until it landed. The viewer now draws the last finished turn until the new one is ready. Panning is allowed again below fit, bounded by the frame. Windows has no grab cursors in gpui, so the hand stands in there. Turns and preview decodes are logged at debug level with their timings. The details caption drops the pixel dimensions and shows type and size only. --- crates/preview/src/lib.rs | 26 ++++++++++++++-- crates/workspace/src/viewer/mod.rs | 48 +++++++++++++++++++++++------- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index 613fb662..9f0ad417 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -161,7 +161,16 @@ impl Asset for Preview { let executor = cx.background_executor().clone(); async move { executor - .spawn(async move { render(&path, max_edge, page, turns) }) + .spawn(async move { + let started = std::time::Instant::now(); + let image = render(&path, max_edge, page, turns); + log::debug!( + "preview: {} page {page} turned {turns} at {max_edge}px took {:?}", + path.display(), + started.elapsed() + ); + image + }) .await } } @@ -285,11 +294,10 @@ fn tiff_page(path: &Path, page: usize) -> Option { /// One stat per call, so call it when the selection changes rather than per frame. pub fn describe(path: &Path) -> Option { let kind = extension(path).map(|extension| extension.to_uppercase()); - let pixels = dimensions(path).map(|(width, height)| format!("{width} × {height}")); let size = std::fs::metadata(path) .ok() .map(|metadata| file_size(metadata.len())); - let parts: Vec = [kind, pixels, size].into_iter().flatten().collect(); + let parts: Vec = [kind, size].into_iter().flatten().collect(); (!parts.is_empty()).then(|| parts.join(" · ")) } @@ -839,6 +847,18 @@ pub fn source(path: &Path, max_edge: u32, page: usize, turns: u8) -> ImageSource })) } +/// Whether `source` has finished decoding, starting it if not. gpui re-renders the asking view +/// when it lands. +/// +/// ponytail: a path handed to gpui counts as ready — it is only ever the upright picture the +/// viewer opened with, already loaded. +pub fn ready(source: &ImageSource, window: &mut Window, cx: &mut App) -> bool { + match source { + ImageSource::Custom(load) => load(window, cx).is_some(), + _ => true, + } +} + #[cfg(test)] mod tests { // No `use super::*`: chain-globbing `gpui::*` shadows the built-in `#[test]` and recurses (see CLAUDE.md). diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 6faa666a..c9c9ac0d 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -156,6 +156,7 @@ pub(crate) fn build( offset: Point::default(), drag_from: None, turns: 0, + shown: 0, pixels, scale: 1.0, frame: Rc::default(), @@ -295,6 +296,9 @@ pub struct Viewer { drag_from: Option>, /// Quarter turns clockwise, for a scan that was fed in sideways. A view, never saved. turns: u8, + /// The turn on screen: the last one decoded, kept up while `turns` decodes so a turn never + /// blanks the stage. + shown: u8, /// Upright pixel size, where the header says one — what "actual size" is measured against. pixels: Option<(u32, u32)>, /// The window's scale factor at the last render, so 1:1 means one image pixel per device pixel. @@ -390,20 +394,20 @@ impl Viewer { self.fit().map(|(_, fit)| 1.0 / (fit * self.scale)) } - /// How far the picture overhangs its frame on each side — the most it may be panned. Measured - /// on the image itself where its size is known, so the letterbox bars are not pannable. + /// The most the picture may be panned each way: to where its edge meets the frame's, from + /// outside when it overhangs and from inside when it is smaller. fn slack(&self) -> Point { let frame = self.frame.get().size; let shown = self.fit().map_or(frame, |(image, fit)| { size(px(image.width * fit), px(image.height * fit)) }); point( - ((shown.width * self.zoom - frame.width) / 2.).max(px(0.)), - ((shown.height * self.zoom - frame.height) / 2.).max(px(0.)), + ((shown.width * self.zoom - frame.width) / 2.).abs(), + ((shown.height * self.zoom - frame.height) / 2.).abs(), ) } - /// Keep an edge of the picture on its frame's edge, so a drag cannot lose it off screen. Left + /// Keep the picture's edges against the frame's, so a drag cannot lose it off screen. Left /// alone before the frame has a size, which is only ever the case before the first paint. fn clamp_pan(&mut self) { if self.frame.get().size.width <= px(0.) { @@ -420,6 +424,11 @@ impl Viewer { self.turns = (self.turns as i8 + delta).rem_euclid(4) as u8; self.zoom = 1.0; self.offset = Point::default(); + log::debug!( + "viewer: {} turned to {}°", + self.path.display(), + self.turns as u16 * 90 + ); } /// Fit when zoomed, actual size when fitted; 2× for a file with no pixel size of its own, or @@ -615,10 +624,20 @@ impl Render for Viewer { 0 => self.find.on_page(page).cloned().collect(), _ => Vec::new(), }; - let cursor = match (self.drag_from.is_some(), self.slack()) { + let wanted = preview::source(&self.path, cap, page, self.turns); + if preview::ready(&wanted, window, cx) { + self.shown = self.turns; + } + let picture = match self.shown == self.turns { + true => wanted, + false => preview::source(&self.path, cap, page, self.shown), + }; + // gpui on Windows has no grab cursors and falls back to the arrow; the hand is its nearest. + let cursor = match (self.drag_from.is_some(), self.slack() != Point::default()) { + (false, false) => CursorStyle::Arrow, + _ if cfg!(windows) => CursorStyle::PointingHand, (true, _) => CursorStyle::ClosedHand, - (false, slack) if slack.x > px(0.) || slack.y > px(0.) => CursorStyle::OpenHand, - _ => CursorStyle::Arrow, + (false, true) => CursorStyle::OpenHand, }; let readout = match self.actual_size() { Some(actual) => format!("{:.0}%", zoom / actual * 100.), @@ -798,9 +817,7 @@ impl Render for Viewer { .children((!bare).then(|| { // `flex_shrink_0` keeps `relative(zoom)` past 1. // The id is what lets gpui keep a GIF's frame clock. - img(preview::source( - &self.path, cap, page, self.turns, - )) + img(picture) .id("viewer-image") .flex_shrink_0() .relative() @@ -1289,6 +1306,15 @@ mod tests { "no further than the overhang on either side" ); + viewer.set_zoom(0.5, gpui::Point::default()); + viewer.offset = gpui::point(gpui::px(5000.), gpui::px(5000.)); + viewer.clamp_pan(); + assert_eq!( + viewer.offset, + gpui::point(gpui::px(250.), gpui::px(125.)), + "zoomed out, it still drags, as far as the frame's edge" + ); + viewer.rotate(1); assert_eq!(viewer.zoom, 1.0); assert_eq!(viewer.offset, gpui::Point::default()); From 0ca6ff40239cb98c8d73a5fde45948a2eebb9343 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:16:06 -0700 Subject: [PATCH 09/31] feat(app): show preview decoding in the status bar; redraw after clearing the cache Clear cache only deleted the files on disk, so every thumbnail already decoded stayed in memory and nothing changed on screen. It now drops the decoded pictures too, and they are drawn again from the files. While previews decode, the right of the status bar shows a spinner and how many are in flight, so a gallery filling in reads as work under way. --- crates/app/src/app_settings/mod.rs | 2 +- crates/app/src/status_items/mod.rs | 5 ++ crates/app/src/status_items/previews_busy.rs | 48 ++++++++++++++++++++ crates/preview/src/lib.rs | 24 ++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 crates/app/src/status_items/previews_busy.rs diff --git a/crates/app/src/app_settings/mod.rs b/crates/app/src/app_settings/mod.rs index ba531aeb..e953b936 100644 --- a/crates/app/src/app_settings/mod.rs +++ b/crates/app/src/app_settings/mod.rs @@ -791,7 +791,7 @@ fn previews_group(cx: &App) -> SettingGroup { }; cx.update(|cx| { cx.set_global(CacheCleared(outcome.into())); - cx.refresh_windows(); + preview::forget_all(cx); }); }) .detach(); diff --git a/crates/app/src/status_items/mod.rs b/crates/app/src/status_items/mod.rs index 9826330e..0685332a 100644 --- a/crates/app/src/status_items/mod.rs +++ b/crates/app/src/status_items/mod.rs @@ -3,6 +3,7 @@ pub mod markup; mod new_files; mod panel_buttons; mod plugin_bar; +mod previews_busy; use cell_location::CellLocation; use gpui::*; @@ -11,6 +12,7 @@ use new_files::NewFilesButton; use panel_buttons::PanelButtons; use plugin_api::{Bar, BarContributions, Side}; pub use plugin_bar::PluginBar; +use previews_busy::PreviewsBusy; use window_wrapper::{BarRegistry, status_bar::StatusBarRegistry}; use workspace::{BarSide, DockToggleButton, PANELS}; @@ -64,6 +66,9 @@ pub fn build_status_bar_registry(cx: &mut App, dock: WeakEntity) -> St !BarContributions::at(Bar::Status, Side::Right, cx).is_empty() }); + let previews_busy = cx.new(PreviewsBusy::new); + registry.items_mut().add_right(previews_busy); + // Text readout of the table's selected cell. let cell_location = cx.new(CellLocation::new); registry.items_mut().add_right(cell_location); diff --git a/crates/app/src/status_items/previews_busy.rs b/crates/app/src/status_items/previews_busy.rs new file mode 100644 index 00000000..0d69bd17 --- /dev/null +++ b/crates/app/src/status_items/previews_busy.rs @@ -0,0 +1,48 @@ +//! Right-side status-bar readout: a spinner and a count while previews are decoding, so a gallery +//! filling in reads as work under way rather than a slow app. + +use std::time::Duration; + +use gpui::prelude::FluentBuilder as _; +use gpui::*; +use gpui_component::{Sizable as _, h_flex, spinner::Spinner}; + +pub struct PreviewsBusy { + shown: usize, + _poll: Task<()>, +} + +impl PreviewsBusy { + /// ponytail: polls, since decodes finish on a background thread that cannot notify a view. + /// Four cheap loads a second, and a re-render only when the count moves. + pub fn new(cx: &mut Context) -> Self { + let _poll = cx.spawn(async move |this, cx| { + loop { + cx.background_executor() + .timer(Duration::from_millis(250)) + .await; + let now = preview::decoding(); + let alive = this.update(cx, |this, cx| { + if this.shown != now { + this.shown = now; + cx.notify(); + } + }); + if alive.is_err() { + break; + } + } + }); + Self { shown: 0, _poll } + } +} + +impl Render for PreviewsBusy { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + h_flex().when(self.shown > 0, |row| { + row.gap_1() + .child(Spinner::new().xsmall()) + .child(format!("Loading previews ({})", self.shown)) + }) + } +} diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index 9f0ad417..9942110b 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -19,6 +19,7 @@ pub mod playback; use std::collections::{BTreeMap, HashMap}; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use components::ComponentId; @@ -159,11 +160,13 @@ impl Asset for Preview { cx: &mut App, ) -> impl Future + Send + 'static { let executor = cx.background_executor().clone(); + DECODING.fetch_add(1, Ordering::Relaxed); async move { executor .spawn(async move { let started = std::time::Instant::now(); let image = render(&path, max_edge, page, turns); + DECODING.fetch_sub(1, Ordering::Relaxed); log::debug!( "preview: {} page {page} turned {turns} at {max_edge}px took {:?}", path.display(), @@ -176,6 +179,13 @@ impl Asset for Preview { } } +static DECODING: AtomicUsize = AtomicUsize::new(0); + +/// How many previews are being decoded right now, for a busy readout. +pub fn decoding() -> usize { + DECODING.load(Ordering::Relaxed) +} + /// Page counts learned by whoever last opened each file: the thumbnail loader, from the disk cache /// or a fresh count, and the viewer. Read by the gallery per card per frame, so it touches no disk. /// @@ -730,6 +740,20 @@ pub fn forget(path: &Path, cx: &mut App) { cx.refresh_windows(); } +/// Drop every decoded picture and page count, so a cleared disk cache is drawn again from the +/// files rather than from memory. Call it outside a frame, as [`release`] explains. +pub fn forget_all(cx: &mut App) { + if let Ok(mut known) = PAGES.lock() { + known.clear(); + } + let live = std::mem::take(cx.default_global::()); + for (key, image) in live.entries.into_values() { + cx.remove_asset::(&key); + cx.drop_image(image, None); + } + cx.refresh_windows(); +} + /// The file's contents fit to whatever box the caller gives it, or a type icon when there is /// nothing to draw — no path, an undecodable one, or a decode that fails at paint time. `fit` /// lets a small icon fill its frame while gallery and details views show the entire file. From 46b5802f85b1d0a8a6520f819a0ce58233e585de Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:29:41 -0700 Subject: [PATCH 10/31] fix(app): drop the previews readout from the status bar when idle It rendered empty but still counted as occupied, so the bar kept a divider and a gap for it. --- crates/app/src/status_items/mod.rs | 6 ++++- crates/app/src/status_items/previews_busy.rs | 23 +++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/app/src/status_items/mod.rs b/crates/app/src/status_items/mod.rs index 0685332a..711a9f2f 100644 --- a/crates/app/src/status_items/mod.rs +++ b/crates/app/src/status_items/mod.rs @@ -67,7 +67,11 @@ pub fn build_status_bar_registry(cx: &mut App, dock: WeakEntity) -> St }); let previews_busy = cx.new(PreviewsBusy::new); - registry.items_mut().add_right(previews_busy); + registry + .items_mut() + .add_right_if(previews_busy.clone(), move |cx| { + previews_busy.read(cx).shown > 0 + }); // Text readout of the table's selected cell. let cell_location = cx.new(CellLocation::new); diff --git a/crates/app/src/status_items/previews_busy.rs b/crates/app/src/status_items/previews_busy.rs index 0d69bd17..1509db50 100644 --- a/crates/app/src/status_items/previews_busy.rs +++ b/crates/app/src/status_items/previews_busy.rs @@ -3,12 +3,11 @@ use std::time::Duration; -use gpui::prelude::FluentBuilder as _; use gpui::*; use gpui_component::{Sizable as _, h_flex, spinner::Spinner}; pub struct PreviewsBusy { - shown: usize, + pub shown: usize, _poll: Task<()>, } @@ -23,10 +22,15 @@ impl PreviewsBusy { .await; let now = preview::decoding(); let alive = this.update(cx, |this, cx| { - if this.shown != now { - this.shown = now; - cx.notify(); + if this.shown == now { + return; } + // Appearing or leaving changes the bar's dividers, which only the bar redraws. + if (this.shown == 0) != (now == 0) { + cx.refresh_windows(); + } + this.shown = now; + cx.notify(); }); if alive.is_err() { break; @@ -39,10 +43,9 @@ impl PreviewsBusy { impl Render for PreviewsBusy { fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { - h_flex().when(self.shown > 0, |row| { - row.gap_1() - .child(Spinner::new().xsmall()) - .child(format!("Loading previews ({})", self.shown)) - }) + h_flex() + .gap_1() + .child(Spinner::new().xsmall()) + .child(format!("Loading previews ({})", self.shown)) } } From 589c1c7d0449b1223e545c3a4f5ce3e9c36925be Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:31:49 -0700 Subject: [PATCH 11/31] style(workspace): match the viewer's page pill to its other controls It was the one opaque, bordered, shadowed pill with outline buttons and bold text. It now uses the translucent pill and small ghost buttons the toolbar and row stepper use. The video scrubber shares it. --- crates/workspace/src/viewer/mod.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index c9c9ac0d..159a1fe0 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -22,7 +22,7 @@ use std::time::Duration; use gpui::prelude::FluentBuilder as _; use gpui::*; use gpui_component::{ - ActiveTheme, Disableable as _, IconName, Selectable as _, Sizable, StyledExt as _, + ActiveTheme, Disableable as _, IconName, Selectable as _, Sizable, button::{Button, ButtonVariants}, input::{InputEvent, InputState}, resizable::{ResizableState, h_resizable, resizable_panel}, @@ -893,20 +893,15 @@ impl Render for Viewer { false => slot.bottom_4(), }) .child( - // Loud on purpose. These are the only controls a reader reaches for - // constantly, and over a dimmed page a translucent pill of small - // ghost buttons reads as decoration. + // Same pill as the toolbar and row stepper. div() .flex() .items_center() - .gap_2() - .px_2() - .py_1() + .gap_1() + .p_1() .rounded(cx.theme().radius) - .bg(cx.theme().background) - .border_1() - .border_color(cx.theme().border) - .shadow_lg() + .bg(pill) + .text_sm() .occlude() .map(|pill| match (&self.transport, &self.scrubber) { (Some(transport), _) => { @@ -914,7 +909,7 @@ impl Render for Viewer { } (_, Some(scrubber)) => pill .child( - div().px_1().font_semibold().child(transport::clock( + div().px_1().child(transport::clock( Duration::from_secs(self.scrub as u64), )), ) @@ -936,7 +931,8 @@ impl Render for Viewer { .child( Button::new("play-in-default-app") .icon(IconName::ExternalLink) - .outline() + .ghost() + .small() .tooltip("Play in the default app") .on_click({ let path = self.path.clone(); @@ -958,7 +954,8 @@ impl Render for Viewer { .child( Button::new("previous-page") .icon(IconName::ChevronLeft) - .outline() + .ghost() + .small() .disabled(page == 0) .tooltip("Previous page") .on_click(cx.listener(|this, _, _, cx| { @@ -971,13 +968,13 @@ impl Render for Viewer { .child( div() .px_1() - .font_semibold() .child(format!("Page {} of {pages}", page + 1)), ) .child( Button::new("next-page") .icon(IconName::ChevronRight) - .outline() + .ghost() + .small() .disabled(page + 1 >= pages) .tooltip("Next page") .on_click(cx.listener(|this, _, _, cx| { From efdd335240efbb98d05b2740393bfc66a0f94920 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:51:14 -0700 Subject: [PATCH 12/31] feat(workspace): go to a page by typing its number The page readout in the viewer's pill is now a box: Enter jumps to the typed page, clamped to the document, and Escape puts the current page back. Works in the overlay and the pop-out, which share the pill. Part of #144. --- crates/workspace/src/viewer/mod.rs | 85 ++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 159a1fe0..dc9974e0 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -24,7 +24,7 @@ use gpui::*; use gpui_component::{ ActiveTheme, Disableable as _, IconName, Selectable as _, Sizable, button::{Button, ButtonVariants}, - input::{InputEvent, InputState}, + input::{Input, InputEvent, InputState}, resizable::{ResizableState, h_resizable, resizable_panel}, slider::{Slider, SliderEvent, SliderState, SliderValue}, }; @@ -164,6 +164,7 @@ pub(crate) fn build( focused: false, find: Find::default(), find_open: false, + page_input: None, split: cx.new(|_| ResizableState::default()), _probe: None, }); @@ -191,6 +192,13 @@ pub(crate) fn build( viewer } +/// The page a typed number lands on: 1-based as the reader types it, clamped to the document. +/// `None` for anything that is not a number, which leaves the page where it is. +fn typed_page(text: &str, pages: usize) -> Option { + let number: usize = text.trim().parse().ok()?; + Some(number.clamp(1, pages.max(1)) - 1) +} + /// Select the next row, by `delta`, in the view's order that has something to preview. During a /// search the view is its hits, so this steps through the results. The open viewer follows the /// selection to that row's file. @@ -312,6 +320,8 @@ pub struct Viewer { pub(crate) find: Find, /// Whether the find panel is showing — in the pop-out, whether its sidebar is on Find. pub(crate) find_open: bool, + /// The go-to-page box in the bottom pill, built the first time the pill draws. + page_input: Option>, /// The split between the page and the find panel, owned by `gpui_component`'s resizable — it /// carries the drag handle, the sizing and the propagation rules, none of which are ours to /// reinvent. @@ -468,6 +478,38 @@ impl Viewer { self.offset = Point::default(); } + /// The page box, built on first use. It shows the current page whenever it is not being typed + /// in, and Enter goes to what was typed. + fn page_input(&mut self, window: &mut Window, cx: &mut Context) -> Entity { + let input = match self.page_input.clone() { + Some(input) => input, + None => { + let input = cx.new(|cx| InputState::new(window, cx)); + cx.subscribe_in( + &input, + window, + |this, input, event: &InputEvent, window, cx| { + if let InputEvent::PressEnter { .. } = event { + if let Some(page) = typed_page(&input.read(cx).value(), this.pages) { + this.show_page(page); + } + window.focus(&this.focus_handle, cx); + cx.notify(); + } + }, + ) + .detach(); + self.page_input = Some(input.clone()); + input + } + }; + let current = (self.page + 1).to_string(); + if !input.focus_handle(cx).is_focused(window) && input.read(cx).value() != current { + input.update(cx, |input, cx| input.set_value(current, window, cx)); + } + input + } + /// Whether the bottom pill has anything to hold: page controls, a transport or a scrubber. pub(crate) fn has_controls(&self) -> bool { self.document || self.pages > 1 || self.transport.is_some() || self.scrubber.is_some() @@ -650,6 +692,7 @@ impl Render for Viewer { .needs .and_then(|id| crate::component_banner::banner(id, cx)); let popped = self.scope == Scope::PopOut; + let page_input = self.page_input(window, cx); div() .track_focus(&self.focus_handle) @@ -965,11 +1008,19 @@ impl Render for Viewer { ) // Numbered from one: the page count a reader sees has to // match the one printed on the document. + .child(div().pl_1().child("Page")) .child( div() - .px_1() - .child(format!("Page {} of {pages}", page + 1)), + .w(px(52.)) + .on_action(cx.listener( + |this, _: &gpui_component::input::Escape, window, cx| { + window.focus(&this.focus_handle, cx); + cx.notify(); + }, + )) + .child(Input::new(&page_input).small()), ) + .child(div().pr_1().child(format!("of {pages}"))) .child( Button::new("next-page") .icon(IconName::ChevronRight) @@ -1324,6 +1375,34 @@ mod tests { }); } + /// A typed page is 1-based, lands inside the document however far off it is, and anything + /// that is not a number leaves the page alone. + #[test] + fn a_typed_page_is_one_based_and_clamped() { + use super::typed_page; + + assert_eq!(typed_page("1", 300), Some(0)); + assert_eq!(typed_page(" 42 ", 300), Some(41)); + assert_eq!( + typed_page("999", 300), + Some(299), + "past the end is the last page" + ); + assert_eq!( + typed_page("0", 300), + Some(0), + "before the start is the first page" + ); + assert_eq!(typed_page("", 300), None); + assert_eq!(typed_page("x", 300), None); + assert_eq!(typed_page("-3", 300), None); + assert_eq!( + typed_page("5", 0), + Some(0), + "an uncounted file still has page one" + ); + } + /// Paging has to stop at both ends. Wrapping past the last page loses the reader's place, and /// an underflow on page zero would panic on a `usize` subtraction. #[gpui::test] From e85b523b3b4b3e1d8d8a9c5df8b5f780401dd8d4 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:54:24 -0700 Subject: [PATCH 13/31] feat(workspace): add a page strip beside multi-page documents A column of page thumbnails on the left of the viewer's stage, for PDFs and TIFF stacks. It is a virtualised list, so only the pages on screen are rendered, through the same cached thumbnail path as the gallery. The current page is outlined and kept in view, and a click goes to it. The page pill has a button to hide it. It lives in the stage rather than the pop-out's sidebar, so the overlay and the pop-out get the same strip. Part of #144. --- crates/workspace/src/viewer/mod.rs | 134 ++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index dc9974e0..cd289334 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -22,8 +22,9 @@ use std::time::Duration; use gpui::prelude::FluentBuilder as _; use gpui::*; use gpui_component::{ - ActiveTheme, Disableable as _, IconName, Selectable as _, Sizable, + ActiveTheme, Disableable as _, Icon, IconName, Selectable as _, Sizable, button::{Button, ButtonVariants}, + h_flex, input::{Input, InputEvent, InputState}, resizable::{ResizableState, h_resizable, resizable_panel}, slider::{Slider, SliderEvent, SliderState, SliderValue}, @@ -44,6 +45,10 @@ pub const VIEWER_CONTEXT: &str = "Viewer"; const PANEL: Pixels = px(384.); const PANEL_RANGE: std::ops::Range = px(240.)..px(720.); +/// The page strip's width, and the height of one page in it: a thumbnail and its number. +const STRIP: Pixels = px(128.); +const STRIP_ROW: Pixels = px(168.); + /// How long a file has to stay open before its pages or duration are probed, and a query has to /// stay typed before it is searched. Stepping through videos or typing a word then starts one /// ffmpeg or one PDFium search, not one per file or per keystroke — neither can be stopped once @@ -165,6 +170,8 @@ pub(crate) fn build( find: Find::default(), find_open: false, page_input: None, + strip_open: true, + strip: UniformListScrollHandle::new(), split: cx.new(|_| ResizableState::default()), _probe: None, }); @@ -322,6 +329,9 @@ pub struct Viewer { pub(crate) find_open: bool, /// The go-to-page box in the bottom pill, built the first time the pill draws. page_input: Option>, + /// Whether the page strip is showing, for a file with pages to list. + strip_open: bool, + strip: UniformListScrollHandle, /// The split between the page and the find panel, owned by `gpui_component`'s resizable — it /// carries the drag handle, the sizing and the propagation rules, none of which are ours to /// reinvent. @@ -476,6 +486,12 @@ impl Viewer { self.page = page; self.zoom = 1.0; self.offset = Point::default(); + self.strip.scroll_to_item(page, ScrollStrategy::Nearest); + } + + /// Whether the file has pages to list: a document or image stack, not a video's seconds. + fn paged(&self) -> bool { + self.pages > 1 && self.scrubber.is_none() && !self.video } /// The page box, built on first use. It shows the current page whenever it is not being typed @@ -693,6 +709,84 @@ impl Render for Viewer { .and_then(|id| crate::component_banner::banner(id, cx)); let popped = self.scope == Scope::PopOut; let page_input = self.page_input(window, cx); + let paged = self.paged(); + let strip_width = match paged && self.strip_open { + true => STRIP, + false => px(0.), + }; + // Only the rows on screen are built, so a 300-page scan asks for a handful of thumbnails. + // ponytail: they share PDFium's one lock with the page itself, so a jump can wait behind a + // screenful of thumbnails; render the page first if that shows up. + let strip = (strip_width > px(0.)).then(|| { + let (primary, muted, radius) = ( + cx.theme().primary, + cx.theme().muted_foreground, + cx.theme().radius, + ); + div() + .w(STRIP) + .h_full() + .flex_none() + .bg(pill) + .occlude() + .child( + uniform_list( + "viewer-pages", + pages, + cx.processor(move |this, range: std::ops::Range, _, cx| { + range + .map(|index| { + let on = index == this.page; + div() + .id(("viewer-page", index)) + .h(STRIP_ROW) + .p_2() + .flex() + .flex_col() + .items_center() + .gap_1() + .cursor_pointer() + .child( + div() + .w_full() + .flex_1() + .min_h_0() + .rounded(radius) + .border_2() + .border_color(match on { + true => primary, + false => transparent_black(), + }) + .overflow_hidden() + .child( + img(preview::source( + &this.path, + preview::CARD, + index, + 0, + )) + .size_full() + .object_fit(ObjectFit::Contain), + ), + ) + .child( + div() + .text_xs() + .when(!on, |label| label.text_color(muted)) + .child((index + 1).to_string()), + ) + .on_click(cx.listener(move |this, _, _, cx| { + this.show_page(index); + cx.notify(); + })) + }) + .collect() + }), + ) + .track_scroll(&self.strip) + .size_full(), + ) + }); div() .track_focus(&self.focus_handle) @@ -781,6 +875,8 @@ impl Render for Viewer { } })) .child( + h_flex().size_full().children(strip).child( + div().flex_1().min_w_0().h_full().child( h_resizable("viewer-split") .with_state(&self.split) .child( @@ -888,12 +984,13 @@ impl Render for Viewer { .visible(self.find_open && !popped) .child(find::panel(&self.find, panel_width, false, cx)), ), + )), ) .child( div() .absolute() .top_4() - .left_4() + .left(px(16.) + strip_width) .px_2() .py_1() .rounded(cx.theme().radius) @@ -994,6 +1091,29 @@ impl Render for Viewer { }), ), _ => pill + .when(paged, |pill| { + pill.child( + Button::new("toggle-pages") + .icon(Icon::new(IconName::PanelLeft).when( + self.strip_open, + |icon| icon.text_color(cx.theme().primary), + )) + .ghost() + .small() + .tooltip(match self.strip_open { + true => "Hide pages", + false => "Show pages", + }) + .on_click(cx.listener(|this, _, _, cx| { + this.strip_open = !this.strip_open; + this.strip.scroll_to_item( + this.page, + ScrollStrategy::Center, + ); + cx.notify(); + })), + ) + }) .child( Button::new("previous-page") .icon(IconName::ChevronLeft) @@ -1415,7 +1535,9 @@ mod tests { viewer.update(cx, |viewer, _| { // A missing file reports one page, so give it a document to page through. + assert!(!viewer.paged(), "one page has nothing to list"); viewer.pages = 3; + assert!(viewer.paged(), "a document with pages gets the strip"); viewer.turn_page(-1); assert_eq!(viewer.page, 0, "cannot go back from the first page"); @@ -1481,6 +1603,14 @@ mod tests { let document = viewer_in(Scope::Workspace, cx).expect("just opened"); document.update(cx, |viewer, _| assert!(viewer.transport.is_none())); + // A video's positions are seconds, which are the scrubber's and not a page strip's. + open_viewer("/nonexistent/clip.mp4".into(), Scope::Workspace, window, cx); + let video = viewer_in(Scope::Workspace, cx).expect("just opened"); + video.update(cx, |viewer, _| { + viewer.pages = 6; + assert!(!viewer.paged()); + }); + close_viewer(window, cx); }); } From 481846ada5fde2791ff5ebfcb216646aaa5ee850 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:11:53 -0700 Subject: [PATCH 14/31] feat(workspace): fit a page to the viewer's width A button in the page pill, and W, zoom a page until it spans the frame and start at its top. The page's shape comes from the picture that was decoded, so a PDF or TIFF page works without asking PDFium for a size, and actual size (1) now works on documents too. Part of #144. --- assets/icons/move-horizontal.svg | 1 + crates/app/src/assets.rs | 8 +++- crates/preview/src/lib.rs | 21 ++++++--- crates/workspace/src/viewer/mod.rs | 76 +++++++++++++++++++++++++++++- 4 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 assets/icons/move-horizontal.svg diff --git a/assets/icons/move-horizontal.svg b/assets/icons/move-horizontal.svg new file mode 100644 index 00000000..87176ae5 --- /dev/null +++ b/assets/icons/move-horizontal.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/crates/app/src/assets.rs b/crates/app/src/assets.rs index 67614d35..ef91e7b6 100644 --- a/crates/app/src/assets.rs +++ b/crates/app/src/assets.rs @@ -1,7 +1,7 @@ //! Our own icons in front of `gpui_component_assets`, which is otherwise the only asset source. //! The bundled icon set has no filled panel glyphs (the title bar's "this dock is open" state) and //! no picture glyph (visual search), and none of the pop-out viewer's window, pin or missing-file -//! glyphs, so those SVGs are ours, copied from Lucide. +//! glyphs, nor the viewer's fit-to-width glyph, so those SVGs are ours, copied from Lucide. use std::borrow::Cow; @@ -9,7 +9,7 @@ use gpui::{AssetSource, Result, SharedString}; pub struct Assets; -const OWN: [(&str, &str); 9] = [ +const OWN: [(&str, &str); 10] = [ ( "icons/history.svg", include_str!("../../../assets/icons/history.svg"), @@ -46,6 +46,10 @@ const OWN: [(&str, &str); 9] = [ "icons/file-x.svg", include_str!("../../../assets/icons/file-x.svg"), ), + ( + "icons/move-horizontal.svg", + include_str!("../../../assets/icons/move-horizontal.svg"), + ), ]; impl AssetSource for Assets { diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index 9942110b..e224c0e6 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -871,15 +871,22 @@ pub fn source(path: &Path, max_edge: u32, page: usize, turns: u8) -> ImageSource })) } -/// Whether `source` has finished decoding, starting it if not. gpui re-renders the asking view -/// when it lands. +/// `None` while `source` decodes, starting it if it has not started; gpui re-renders the asking +/// view when it lands. Once done, the pixel size of what was decoded, if we decoded it. /// -/// ponytail: a path handed to gpui counts as ready — it is only ever the upright picture the -/// viewer opened with, already loaded. -pub fn ready(source: &ImageSource, window: &mut Window, cx: &mut App) -> bool { +/// ponytail: a path handed to gpui counts as done with no size — it is only ever the upright +/// picture the viewer opened with, already loaded, whose size the header gave. +pub fn decoded( + source: &ImageSource, + window: &mut Window, + cx: &mut App, +) -> Option> { match source { - ImageSource::Custom(load) => load(window, cx).is_some(), - _ => true, + ImageSource::Custom(load) => Some(load(window, cx)?.ok().map(|image| { + let size = image.size(0); + (size.width.0 as u32, size.height.0 as u32) + })), + _ => Some(None), } } diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index cd289334..3da6aae6 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -489,6 +489,19 @@ impl Viewer { self.strip.scroll_to_item(page, ScrollStrategy::Nearest); } + /// Zoom until the page spans the frame's width, starting at its top. A page already that wide + /// at fit stays at fit. + fn fit_width(&mut self) { + let Some((image, fit)) = self.fit() else { + return; + }; + let zoom = f32::from(self.frame.get().size.width) / (image.width * fit); + self.set_zoom(zoom, Point::default()); + if self.zoom > 1.0 { + self.offset.y = self.slack().y; + } + } + /// Whether the file has pages to list: a document or image stack, not a video's seconds. fn paged(&self) -> bool { self.pages > 1 && self.scrubber.is_none() && !self.video @@ -683,8 +696,15 @@ impl Render for Viewer { _ => Vec::new(), }; let wanted = preview::source(&self.path, cap, page, self.turns); - if preview::ready(&wanted, window, cx) { + if let Some(size) = preview::decoded(&wanted, window, cx) { self.shown = self.turns; + // A PDF or TIFF page has no header size, and pages differ; what was drawn does. + if let Some((width, height)) = size { + self.pixels = Some(match self.turns % 2 { + 0 => (width, height), + _ => (height, width), + }); + } } let picture = match self.shown == self.turns { true => wanted, @@ -864,6 +884,10 @@ impl Render for Viewer { cx.notify(); } } + "w" if reading => { + this.fit_width(); + cx.notify(); + } "r" if reading => { this.rotate(if ev.keystroke.modifiers.shift { -1 } else { 1 }); cx.notify(); @@ -1152,6 +1176,20 @@ impl Render for Viewer { this.turn_page(1); cx.notify(); })), + ) + .child( + Button::new("fit-width") + .icon( + Icon::empty() + .path("icons/move-horizontal.svg"), + ) + .ghost() + .small() + .tooltip("Fit to width (W)") + .on_click(cx.listener(|this, _, _, cx| { + this.fit_width(); + cx.notify(); + })), ), }), ), @@ -1495,6 +1533,42 @@ mod tests { }); } + /// Fit to width fills the frame's width from the top of a tall page, and leaves a page that + /// is already as wide as its frame at fit. + #[gpui::test] + fn fit_width_fills_the_width_from_the_top(cx: &mut TestAppContext) { + let cx = with_window(cx); + let path = std::path::PathBuf::from("/nonexistent/qrate-fit-width.pdf"); + cx.update(|window, cx| { + open_viewer(path, Scope::Workspace, window, cx); + let viewer = viewer_in(Scope::Workspace, cx).expect("just opened"); + viewer.update(cx, |viewer, _| { + viewer.frame.set(gpui::Bounds::new( + gpui::Point::default(), + gpui::size(gpui::px(1000.), gpui::px(500.)), + )); + + // Portrait, fitted at a quarter: four times over to span the width. + viewer.pixels = Some((1000, 2000)); + viewer.fit_width(); + assert_eq!(viewer.zoom, 4.0); + assert_eq!( + viewer.offset, + gpui::point(gpui::px(0.), gpui::px(750.)), + "the page's top edge on the frame's" + ); + + // Landscape already spans the width at fit. + viewer.show_page(0); + viewer.pixels = Some((2000, 1000)); + viewer.fit_width(); + assert_eq!(viewer.zoom, 1.0); + assert_eq!(viewer.offset, gpui::Point::default()); + }); + close_viewer(window, cx); + }); + } + /// A typed page is 1-based, lands inside the document however far off it is, and anything /// that is not a number leaves the page alone. #[test] From 4feaa8791344e95b214d9ae381dd6c4844174802 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:21:20 -0700 Subject: [PATCH 15/31] fix(workspace): start the page strip closed The page pill's button opens it. --- crates/workspace/src/viewer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 3da6aae6..8bdc3b0e 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -170,7 +170,7 @@ pub(crate) fn build( find: Find::default(), find_open: false, page_input: None, - strip_open: true, + strip_open: false, strip: UniformListScrollHandle::new(), split: cx.new(|_| ResizableState::default()), _probe: None, From 2872725eabed7f45a23f0dd22a1650db0a1d9597 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:22:19 -0700 Subject: [PATCH 16/31] fix(workspace): keep each page's shape in the page strip The outline and rounding were on a fixed box around the thumbnail, so every page showed as the same shape. They are on the picture now, which keeps its own aspect ratio inside the row. --- crates/workspace/src/viewer/mod.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 8bdc3b0e..cda6ea22 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -766,18 +766,15 @@ impl Render for Viewer { .items_center() .gap_1() .cursor_pointer() + // The outline is on the picture, so it takes the page's shape. .child( div() .w_full() .flex_1() .min_h_0() - .rounded(radius) - .border_2() - .border_color(match on { - true => primary, - false => transparent_black(), - }) - .overflow_hidden() + .flex() + .items_center() + .justify_center() .child( img(preview::source( &this.path, @@ -785,8 +782,14 @@ impl Render for Viewer { index, 0, )) - .size_full() - .object_fit(ObjectFit::Contain), + .max_w_full() + .max_h_full() + .rounded(radius) + .border_2() + .border_color(match on { + true => primary, + false => transparent_black(), + }), ), ) .child( From 2636fe50754e160d89a65d506b37c766d51c002c Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:30:40 -0700 Subject: [PATCH 17/31] fix(workspace): size page-strip thumbnails from the decoded page Capping the picture with max_w_full/max_h_full inside a flexed row left it at its intrinsic 512px: it spilled past the strip's edge and the outlined page came up empty. Each thumbnail is now given an explicit size, its decoded shape fitted to the row, and a portrait placeholder until it lands. --- crates/workspace/src/viewer/mod.rs | 91 +++++++++++++++++++++--------- 1 file changed, 64 insertions(+), 27 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index cda6ea22..0ee5f6b9 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -48,6 +48,8 @@ const PANEL_RANGE: std::ops::Range = px(240.)..px(720.); /// The page strip's width, and the height of one page in it: a thumbnail and its number. const STRIP: Pixels = px(128.); const STRIP_ROW: Pixels = px(168.); +/// The most a page thumbnail may take inside a row, after its padding and number. +const STRIP_THUMB: Size = size(px(112.), px(128.)); /// How long a file has to stay open before its pages or duration are probed, and a query has to /// stay typed before it is searched. Stepping through videos or typing a word then starts one @@ -199,6 +201,16 @@ pub(crate) fn build( viewer } +/// The size a page thumbnail takes in the strip: its own shape, as large as fits the row. A page +/// that has not decoded yet is drawn portrait, the shape most documents are. +fn strip_frame(pixels: Option<(u32, u32)>) -> Size { + let (width, height) = pixels + .filter(|(width, height)| *width > 0 && *height > 0) + .map_or((3.0, 4.0), |(width, height)| (width as f32, height as f32)); + let scale = (f32::from(STRIP_THUMB.width) / width).min(f32::from(STRIP_THUMB.height) / height); + size(px(width * scale), px(height * scale)) +} + /// The page a typed number lands on: 1-based as the reader types it, clamped to the document. /// `None` for anything that is not a number, which leaves the page where it is. fn typed_page(text: &str, pages: usize) -> Option { @@ -738,10 +750,11 @@ impl Render for Viewer { // ponytail: they share PDFium's one lock with the page itself, so a jump can wait behind a // screenful of thumbnails; render the page first if that shows up. let strip = (strip_width > px(0.)).then(|| { - let (primary, muted, radius) = ( + let (primary, muted, radius, tile) = ( cx.theme().primary, cx.theme().muted_foreground, cx.theme().radius, + cx.theme().muted, ); div() .w(STRIP) @@ -753,7 +766,7 @@ impl Render for Viewer { uniform_list( "viewer-pages", pages, - cx.processor(move |this, range: std::ops::Range, _, cx| { + cx.processor(move |this, range: std::ops::Range, window, cx| { range .map(|index| { let on = index == this.page; @@ -766,32 +779,30 @@ impl Render for Viewer { .items_center() .gap_1() .cursor_pointer() - // The outline is on the picture, so it takes the page's shape. - .child( + .child({ + let source = preview::source( + &this.path, + preview::CARD, + index, + 0, + ); + let shape = + preview::decoded(&source, window, cx).flatten(); + let frame = strip_frame(shape); div() - .w_full() - .flex_1() - .min_h_0() - .flex() - .items_center() - .justify_center() - .child( - img(preview::source( - &this.path, - preview::CARD, - index, - 0, - )) - .max_w_full() - .max_h_full() - .rounded(radius) - .border_2() - .border_color(match on { - true => primary, - false => transparent_black(), - }), - ), - ) + .w(frame.width) + .h(frame.height) + .flex_none() + .rounded(radius) + .border_2() + .border_color(match on { + true => primary, + false => transparent_black(), + }) + .overflow_hidden() + .bg(tile) + .child(img(source).size_full().rounded(radius)) + }) .child( div() .text_xs() @@ -1572,6 +1583,32 @@ mod tests { }); } + /// A page in the strip keeps its own shape and never leaves the row's box. + #[test] + fn a_strip_thumbnail_keeps_the_pages_shape_inside_the_row() { + use super::{STRIP_THUMB, strip_frame}; + use gpui::{px, size}; + + assert_eq!( + strip_frame(Some((1000, 2000))), + size(px(64.), px(128.)), + "tall" + ); + assert_eq!( + strip_frame(Some((2000, 1000))), + size(px(112.), px(56.)), + "wide" + ); + let pending = strip_frame(None); + assert!(pending.height > pending.width, "portrait until it decodes"); + assert!(pending.width <= STRIP_THUMB.width && pending.height <= STRIP_THUMB.height); + assert_eq!( + strip_frame(Some((0, 10))), + pending, + "a degenerate size is not divided by" + ); + } + /// A typed page is 1-based, lands inside the document however far off it is, and anything /// that is not a number leaves the page alone. #[test] From 6e4e3355d8af38b2df38079dc9e70ad7971b02c2 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:02:23 -0700 Subject: [PATCH 18/31] fix(workspace): measure a file drawn by gpui from its header again Page one of a TIFF stack is handed to gpui, which reports no size, so paging back to it kept the previous page's shape for fit, pan and fit to width. The viewer now falls back to the header's size for it. --- crates/preview/src/lib.rs | 4 ++-- crates/workspace/src/viewer/mod.rs | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index e224c0e6..d6685086 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -874,8 +874,8 @@ pub fn source(path: &Path, max_edge: u32, page: usize, turns: u8) -> ImageSource /// `None` while `source` decodes, starting it if it has not started; gpui re-renders the asking /// view when it lands. Once done, the pixel size of what was decoded, if we decoded it. /// -/// ponytail: a path handed to gpui counts as done with no size — it is only ever the upright -/// picture the viewer opened with, already loaded, whose size the header gave. +/// A path handed to gpui counts as done with no size: it is the file itself, and [`dimensions`] +/// already answers for it. pub fn decoded( source: &ImageSource, window: &mut Window, diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 0ee5f6b9..71a52c90 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -165,6 +165,7 @@ pub(crate) fn build( turns: 0, shown: 0, pixels, + header: pixels, scale: 1.0, frame: Rc::default(), focus_handle: cx.focus_handle(), @@ -326,8 +327,10 @@ pub struct Viewer { /// The turn on screen: the last one decoded, kept up while `turns` decodes so a turn never /// blanks the stage. shown: u8, - /// Upright pixel size, where the header says one — what "actual size" is measured against. + /// Upright pixel size of what is on screen — what "actual size" is measured against. pixels: Option<(u32, u32)>, + /// The file header's size, for the picture gpui draws from the file itself and never reports. + header: Option<(u32, u32)>, /// The window's scale factor at the last render, so 1:1 means one image pixel per device pixel. scale: f32, /// Window-space rect of the content box, from `canvas` prepaint — where scroll-zoom's anchor @@ -711,12 +714,13 @@ impl Render for Viewer { if let Some(size) = preview::decoded(&wanted, window, cx) { self.shown = self.turns; // A PDF or TIFF page has no header size, and pages differ; what was drawn does. - if let Some((width, height)) = size { - self.pixels = Some(match self.turns % 2 { + self.pixels = match size { + Some((width, height)) => Some(match self.turns % 2 { 0 => (width, height), _ => (height, width), - }); - } + }), + None => self.header, + }; } let picture = match self.shown == self.turns { true => wanted, From 227a7a6b784dc5602fee2154dcb96f92202d1594 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:03:52 -0700 Subject: [PATCH 19/31] perf(preview): cap a turned full-size picture at 4096px A turn is decoded by us rather than gpui and held under the preview budget; a 60 MP scan held whole was larger than the budget on its own, so it and the gallery evicted each other. The shrink happens after the decode, so nothing extra is written to the disk cache. --- crates/preview/src/lib.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/preview/src/lib.rs b/crates/preview/src/lib.rs index d6685086..b951a442 100644 --- a/crates/preview/src/lib.rs +++ b/crates/preview/src/lib.rs @@ -53,6 +53,10 @@ pub const FULL: u32 = 0; /// natural pixel size, so "no cap" still needs a number; this is generous enough to zoom into. const FULL_FALLBACK: u32 = 2048; +/// The most a turned [`FULL`] picture keeps. gpui draws the upright one from the file; a turn is +/// ours to hold, and a 60 MP scan held whole would outgrow [`BUDGET`] on its own. +const TURNED: u32 = 4096; + /// How much decoded image data may stay resident. gpui's asset cache never evicts on its own, so /// without a ceiling a scroll through a large collection retains every thumbnail it passes. /// @@ -462,7 +466,13 @@ pub fn thumbnail_png(path: &Path, page: usize) -> Option> { /// back something gpui can draw. Runs on a background thread; `None` for anything that won't /// decode, which the caller turns into the icon. fn render(path: &Path, max_edge: u32, page: usize, turns: u8) -> Option> { - let upright = thumbnail_pixels(path, max_edge, page)?; + let mut upright = thumbnail_pixels(path, max_edge, page)?; + if !turns.is_multiple_of(4) + && max_edge == FULL + && upright.width().max(upright.height()) > TURNED + { + upright = downscale(upright.into(), TURNED); + } let mut bgra = match turns % 4 { 1 => image::imageops::rotate90(&upright), 2 => image::imageops::rotate180(&upright), @@ -1273,6 +1283,20 @@ mod tests { let _ = std::fs::remove_file(&small); } + /// A turned full-size picture is ours to hold, so it is capped; an upright one is not. + #[test] + fn a_turned_full_size_picture_is_capped() { + let wide = std::env::temp_dir().join("qrate-turned-cap-probe.png"); + image::RgbaImage::from_pixel(5000, 10, image::Rgba([1, 2, 3, 255])) + .save(&wide) + .unwrap(); + let turned = crate::render(&wide, crate::FULL, 0, 1).expect("decodes"); + assert_eq!(i32::from(turned.size(0).height), crate::TURNED as i32); + let upright = crate::render(&wide, crate::FULL, 0, 0).expect("decodes"); + assert_eq!(i32::from(upright.size(0).width), 5000); + let _ = std::fs::remove_file(&wide); + } + /// A second look at the same file must come off disk rather than decoding again — the reason /// the cache exists. Asserted through the artefact, since the decode itself is not observable. #[test] From b3540c38d49661184e83431d9ea20d4e5405c37b Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:05:36 -0700 Subject: [PATCH 20/31] perf(workspace): stop the pop-out redrawing on every pan and zoom It re-rendered its title bar, stage and sidebar each time the viewer notified, which is every mouse move of a drag. It now redraws only when the pill, the document tabs or the find results change. The stage skips the missing-file lookup while a file is showing, and the OS title is set only when it changes. --- crates/workspace/src/pop_out.rs | 47 ++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index f0dcb5f0..12389dbe 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -156,6 +156,8 @@ pub struct PopOut { _table_sub: Option, /// Repaints when the viewer's pages, controls or find tab change under it. _viewer_sub: Option, + /// What the OS title bar was last told, so a table edit does not set it again unchanged. + window_title: String, _subs: Vec, } @@ -232,6 +234,7 @@ impl PopOut { split: cx.new(|_| ResizableState::default()), _table_sub: None, _viewer_sub: None, + window_title: String::new(), _subs, }; this.bind(window, cx); @@ -317,13 +320,26 @@ impl PopOut { stop_playing(leaving, cx); } self.viewer = file.map(|file| viewer::build(file, Scope::PopOut, window, cx)); - self._viewer_sub = self - .viewer - .as_ref() - .map(|viewer| cx.observe(viewer, |_, _, cx| cx.notify())); + self._viewer_sub = self.viewer.as_ref().map(|viewer| { + // The viewer repaints itself on every pan and zoom; this window only draws + // its controls, tabs and find results. + let mut seen = None; + cx.observe(viewer, move |_, viewer, cx| { + let viewer = viewer.read(cx); + let now = (viewer.has_controls(), viewer.document, viewer.find_open); + if viewer.find_open || seen != Some(now) { + seen = Some(now); + cx.notify(); + } + }) + }); } let (file, rest) = self.title(cx); - window.set_window_title(&format!("{file}{rest}")); + let title = format!("{file}{rest}"); + if title != self.window_title { + window.set_window_title(&title); + self.window_title = title; + } cx.notify(); } @@ -636,14 +652,19 @@ impl PopOut { /// The file, or what the stage says in its place, over the viewer's own backdrop. fn stage(&self, cx: &mut Context) -> AnyElement { let (fg, muted) = (rgb(STAGE_FG), rgb(STAGE_MUTED)); - let front = self.front().zip(self.table()).map(|(row, state)| { - let delegate = state.read(cx).delegate(); - ( - row, - delegate.row_image(row).map(|path| path.to_path_buf()), - table::file_links::missing_file(delegate, row, cx).map(|(_, name)| name), - ) - }); + // Only the messages read it, and they show only when there is no viewer. + let front = self + .front() + .filter(|_| self.viewer.is_none()) + .zip(self.table()) + .map(|(row, state)| { + let delegate = state.read(cx).delegate(); + ( + row, + delegate.row_image(row).map(|path| path.to_path_buf()), + table::file_links::missing_file(delegate, row, cx).map(|(_, name)| name), + ) + }); // What to say when there is nothing the viewer can draw. The one useful action goes with // it, where there is one. From edcef95fcdffe1372449834f7f7e8b56a8679946 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:06:31 -0700 Subject: [PATCH 21/31] fix(workspace): let zoom reach actual size on tiny images The floor was a flat 10%, so an icon fitted far past 10x could not get down to one pixel per pixel and 1 showed 312%. The floor now widens to the actual size, as the ceiling already did. --- crates/workspace/src/viewer/mod.rs | 36 +++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 71a52c90..5940578a 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -395,12 +395,14 @@ impl Viewer { cx.notify(); } - /// Clamp zoom to [0.1, 8] — below 1 zooms out past the initial fit — keeping the point at - /// `anchor` (relative to the frame's centre) still, and recenter once the image is no bigger - /// than its frame, where there's nothing to pan to. + /// Clamp zoom to [0.1, 8], widened to reach actual size either way — below 1 zooms out past + /// the initial fit — keeping the point at `anchor` (relative to the frame's centre) still, and + /// recenter once the image is no bigger than its frame, where there's nothing to pan to. fn set_zoom(&mut self, zoom: f32, anchor: Point) { - let most = self.actual_size().map_or(8.0, |actual| actual.max(8.0)); - let zoom = zoom.clamp(0.1, most); + let actual = self.actual_size(); + let most = actual.map_or(8.0, |actual| actual.max(8.0)); + let least = actual.map_or(0.1, |actual| actual.min(0.1)); + let zoom = zoom.clamp(least, most); let scale = zoom / self.zoom; self.offset = anchor - (anchor - self.offset) * scale; self.zoom = zoom; @@ -1587,6 +1589,30 @@ mod tests { }); } + /// Actual size is reachable however small the file: an icon fitted far past 10× still gets + /// down to one pixel per pixel. + #[gpui::test] + fn actual_size_is_reachable_for_a_tiny_image(cx: &mut TestAppContext) { + let cx = with_window(cx); + let path = std::path::PathBuf::from("/nonexistent/qrate-tiny.png"); + cx.update(|window, cx| { + open_viewer(path, Scope::Workspace, window, cx); + let viewer = viewer_in(Scope::Workspace, cx).expect("just opened"); + viewer.update(cx, |viewer, _| { + viewer.pixels = Some((32, 32)); + viewer.frame.set(gpui::Bounds::new( + gpui::Point::default(), + gpui::size(gpui::px(1000.), gpui::px(500.)), + )); + let actual = viewer.actual_size().expect("has a size"); + assert!(actual < 0.1); + viewer.set_zoom(actual, gpui::Point::default()); + assert_eq!(viewer.zoom, actual); + }); + close_viewer(window, cx); + }); + } + /// A page in the strip keeps its own shape and never leaves the row's box. #[test] fn a_strip_thumbnail_keeps_the_pages_shape_inside_the_row() { From 8902fca282248ccf5d8e97456c7b50fe27a3ef5b Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:07:19 -0700 Subject: [PATCH 22/31] perf(workspace): build the page box only for files with a page pill Every viewer built the input and its subscription on first render, including photos, recordings and each row stepped through in the pop-out. --- crates/workspace/src/viewer/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 5940578a..0e6ca4e4 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -746,7 +746,10 @@ impl Render for Viewer { .needs .and_then(|id| crate::component_banner::banner(id, cx)); let popped = self.scope == Scope::PopOut; - let page_input = self.page_input(window, cx); + // Only the page pill has one, so a photo, recording or video never builds it. + let page_input = + (self.has_controls() && self.transport.is_none() && self.scrubber.is_none()) + .then(|| self.page_input(window, cx)); let paged = self.paged(); let strip_width = match paged && self.strip_open { true => STRIP, @@ -1182,7 +1185,7 @@ impl Render for Viewer { cx.notify(); }, )) - .child(Input::new(&page_input).small()), + .children(page_input.map(|input| Input::new(&input).small())), ) .child(div().pr_1().child(format!("of {pages}"))) .child( From adb1ce73c550ef0c83d48c5ca2d7c0090d926444 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:08:23 -0700 Subject: [PATCH 23/31] fix(workspace): measure zoom and pan on the turn that is on screen While a turn decoded, fit and pan limits already used the new shape though the old picture was still shown. They now follow the turn on screen. A page change drops the old turn too, since the new page has none decoded and the stage would otherwise wait on it blank. --- crates/workspace/src/viewer/mod.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 0e6ca4e4..1f90754b 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -412,11 +412,11 @@ impl Viewer { self.clamp_pan(); } - /// The picture's pixel size as turned, and the scale that fits it to the frame. `None` where - /// the file has no pixel size of its own, or before the frame has been laid out. + /// The picture on screen's pixel size as turned, and the scale that fits it to the frame. `None` + /// where the file has no pixel size of its own, or before the frame has been laid out. fn fit(&self) -> Option<(Size, f32)> { let (width, height) = self.pixels?; - let image = match self.turns % 2 { + let image = match self.shown % 2 { 0 => size(width as f32, height as f32), _ => size(height as f32, width as f32), }; @@ -503,6 +503,8 @@ impl Viewer { self.page = page; self.zoom = 1.0; self.offset = Point::default(); + // A new page has no old turn decoded to keep up. + self.shown = self.turns; self.strip.scroll_to_item(page, ScrollStrategy::Nearest); } @@ -1547,7 +1549,13 @@ mod tests { viewer.rotate(1); assert_eq!(viewer.zoom, 1.0); assert_eq!(viewer.offset, gpui::Point::default()); - // Now 1000×2000 in the same frame, fitted at a quarter. + assert_eq!( + viewer.actual_size(), + Some(2.0), + "measured on the upright picture still on screen" + ); + // The turn lands: now 1000×2000 in the same frame, fitted at a quarter. + viewer.shown = viewer.turns; assert_eq!(viewer.actual_size(), Some(4.0)); viewer.rotate(-2); assert_eq!(viewer.turns, 3, "a turn back from upright wraps"); From 898fa54ac211012ff12b7e4cd1e838c2ca416c8a Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:10:20 -0700 Subject: [PATCH 24/31] fix(workspace): keep the pan when zooming below fit Zooming at or below fit snapped the picture back to the centre, so a picture dragged inside its frame jumped on the next wheel tick. Zoom now keeps the pan, clamped to the frame. Fit (0, or the readout) still recentres, through one fit_view shared with page changes and turns. --- crates/workspace/src/viewer/mod.rs | 31 +++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 1f90754b..5d737376 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -396,8 +396,8 @@ impl Viewer { } /// Clamp zoom to [0.1, 8], widened to reach actual size either way — below 1 zooms out past - /// the initial fit — keeping the point at `anchor` (relative to the frame's centre) still, and - /// recenter once the image is no bigger than its frame, where there's nothing to pan to. + /// the initial fit — keeping the point at `anchor` (relative to the frame's centre) still. The + /// pan stays where the reader put it, inside the frame; only [`Self::fit_view`] recentres. fn set_zoom(&mut self, zoom: f32, anchor: Point) { let actual = self.actual_size(); let most = actual.map_or(8.0, |actual| actual.max(8.0)); @@ -406,12 +406,15 @@ impl Viewer { let scale = zoom / self.zoom; self.offset = anchor - (anchor - self.offset) * scale; self.zoom = zoom; - if self.zoom <= 1.0 { - self.offset = Point::default(); - } self.clamp_pan(); } + /// Back to fit, centred: the one view the pointer cannot land on exactly. + fn fit_view(&mut self) { + self.zoom = 1.0; + self.offset = Point::default(); + } + /// The picture on screen's pixel size as turned, and the scale that fits it to the frame. `None` /// where the file has no pixel size of its own, or before the frame has been laid out. fn fit(&self) -> Option<(Size, f32)> { @@ -459,8 +462,7 @@ impl Viewer { /// at a picture of a different shape. fn rotate(&mut self, delta: i8) { self.turns = (self.turns as i8 + delta).rem_euclid(4) as u8; - self.zoom = 1.0; - self.offset = Point::default(); + self.fit_view(); log::debug!( "viewer: {} turned to {}°", self.path.display(), @@ -472,7 +474,7 @@ impl Viewer { /// whose actual size is the fit. fn toggle_zoom(&mut self, anchor: Point) { if (self.zoom - 1.0).abs() > 0.01 { - self.set_zoom(1.0, anchor); + self.fit_view(); return; } let target = self @@ -501,8 +503,7 @@ impl Viewer { /// lands on a page directly rather than by stepping to it, and must reset the same things. fn show_page(&mut self, page: usize) { self.page = page; - self.zoom = 1.0; - self.offset = Point::default(); + self.fit_view(); // A new page has no old turn decoded to keep up. self.shown = self.turns; self.strip.scroll_to_item(page, ScrollStrategy::Nearest); @@ -900,7 +901,7 @@ impl Render for Viewer { } // Back to fit, the one zoom the pointer cannot land on exactly. "0" if reading => { - this.set_zoom(1.0, Point::default()); + this.fit_view(); cx.notify(); } "1" if reading => { @@ -1545,6 +1546,14 @@ mod tests { gpui::point(gpui::px(250.), gpui::px(125.)), "zoomed out, it still drags, as far as the frame's edge" ); + viewer.set_zoom(0.6, gpui::Point::default()); + assert_ne!( + viewer.offset, + gpui::Point::default(), + "zooming keeps the pan" + ); + viewer.fit_view(); + assert_eq!(viewer.offset, gpui::Point::default(), "fit recentres"); viewer.rotate(1); assert_eq!(viewer.zoom, 1.0); From ab87e557a30b79fb961348b3e8ae6099634e3c5d Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:15:40 -0700 Subject: [PATCH 25/31] fix(workspace): stop only the recording a view started Closing the full-screen viewer, or moving the Details selection, stopped whatever was playing, including a pinned pop-out's recording. The owner check the pop-out had is now playback::stop itself, so every caller gets it; the viewer also stops its own recording when the next row's replaces it. --- crates/preview/src/playback.rs | 9 +++++---- crates/workspace/src/panels/details.rs | 2 +- crates/workspace/src/pop_out.rs | 27 +++++++++++++------------- crates/workspace/src/viewer/mod.rs | 13 ++++++++++++- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/crates/preview/src/playback.rs b/crates/preview/src/playback.rs index 3acecf90..ffde94c9 100644 --- a/crates/preview/src/playback.rs +++ b/crates/preview/src/playback.rs @@ -111,10 +111,11 @@ pub fn position(cx: &App) -> Option<(Duration, bool)> { Some((player.get_pos(), !player.is_paused() && !player.empty())) } -/// Silence. The viewer calls this as it closes — without it the recording plays on over an empty -/// screen, with nothing left on the page to stop it. -pub fn stop(cx: &mut App) { - if cx.has_global::() { +/// Silence, if `owner` started what is playing. A view calls this as it closes or moves on — +/// without it the recording plays on over an empty screen — and must not silence another +/// window's recording on the way out. +pub fn stop(owner: EntityId, cx: &mut App) { + if self::owner(cx) == Some(owner) { let playback = cx.global_mut::(); playback.player.clear(); playback.playing = None; diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index af422540..47b44566 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -301,7 +301,7 @@ impl DetailsPanel { // Whatever was playing belonged to the row being left. Leaving it running would narrate // one item while the panel details another. if self.transport.is_some() { - preview::playback::stop(cx); + preview::playback::stop(cx.entity_id(), cx); } self.transport = path.and_then(|path| Transport::new(path, cx)); } diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index 12389dbe..57eac22e 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -50,14 +50,6 @@ const STAGE_MUTED: u32 = 0xa3a3a3; /// per project, the same way the main window keeps its own. const BOUNDS_KEY: &str = "pop_out_window_bounds"; -/// Silence `viewer`'s recording as it goes — only if it started it: the player is shared by the -/// whole app, and the main window may be playing the same file. -fn stop_playing(viewer: &Entity, cx: &mut App) { - if preview::playback::owner(cx) == Some(viewer.entity_id()) { - preview::playback::stop(cx); - } -} - /// The bounds last seen, for the project they belong to. The `.qrate` write is debounced, so a /// window closed and reopened inside that interval would otherwise read the size it had before. #[derive(Default)] @@ -172,7 +164,7 @@ impl PopOut { cx.on_release(move |this: &mut Self, cx| { // A recording playing here would otherwise go on with nothing on screen to stop it. if let Some(viewer) = &this.viewer { - stop_playing(viewer, cx); + preview::playback::stop(viewer.entity_id(), cx); } // Only this window's own entry: a new pop-out may already have replaced it. if cx @@ -317,7 +309,7 @@ impl PopOut { .and_then(|(row, state)| viewer::previewable(state.read(cx).delegate(), row)); if self.viewer.as_ref().map(|viewer| &viewer.read(cx).path) != file.as_ref() { if let Some(leaving) = &self.viewer { - stop_playing(leaving, cx); + preview::playback::stop(leaving.entity_id(), cx); } self.viewer = file.map(|file| viewer::build(file, Scope::PopOut, window, cx)); self._viewer_sub = self.viewer.as_ref().map(|viewer| { @@ -1061,7 +1053,7 @@ mod tests { use gpui_component::table::TableState; use table::{QrateTableDelegate, TableChanged}; - use super::{PopOut, stop_playing}; + use super::PopOut; /// Three rows in a real grid, and the pop-out window watching it. fn window_over_a_table( @@ -1310,7 +1302,7 @@ mod tests { crate::viewer::build(shown.clone(), crate::viewer::Scope::Workspace, window, cx); preview::playback::play(&shown, main.entity_id(), cx); let before = preview::playback::playing(cx).map(|path| path.to_path_buf()); - stop_playing(&popped, cx); + preview::playback::stop(popped.entity_id(), cx); assert_eq!( preview::playback::playing(cx).map(|path| path.to_path_buf()), before, @@ -1318,7 +1310,16 @@ mod tests { ); preview::playback::play(&shown, popped.entity_id(), cx); - stop_playing(&popped, cx); + let before = preview::playback::playing(cx).map(|path| path.to_path_buf()); + crate::viewer::open_viewer(shown.clone(), crate::viewer::Scope::Workspace, window, cx); + crate::viewer::close_viewer(window, cx); + assert_eq!( + preview::playback::playing(cx).map(|path| path.to_path_buf()), + before, + "closing the main window's viewer leaves the pop-out playing" + ); + + preview::playback::stop(popped.entity_id(), cx); assert!(preview::playback::playing(cx).is_none(), "its own stops"); }); diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 5d737376..8e47e29b 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -93,6 +93,7 @@ pub fn open_viewer(path: PathBuf, scope: Scope, window: &mut Window, cx: &mut Ap .try_global::() .and_then(|active| active.return_focus.clone()) .or_else(|| window.focused(cx)); + stop_active(cx); let viewer = build(path, scope, window, cx); cx.set_global(ActiveViewer { viewer: Some(viewer), @@ -280,9 +281,19 @@ pub(crate) fn next_row( } } +/// Stop the recording the open viewer started, as it is closed or replaced by the next row's. +fn stop_active(cx: &mut App) { + if let Some(viewer) = cx + .try_global::() + .and_then(|active| active.viewer.clone()) + { + preview::playback::stop(viewer.entity_id(), cx); + } +} + pub fn close_viewer(window: &mut Window, cx: &mut App) { // Without this the recording plays on over an empty screen, with nothing left to stop it. - preview::playback::stop(cx); + stop_active(cx); let return_focus = cx .try_global::() .and_then(|active| active.return_focus.clone()); From e82b27368441d496b951d31a7994b09915216511 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:17:26 -0700 Subject: [PATCH 26/31] refactor(table): name the row the grid's cursor is on The Cell-or-Row match was written out in five places across the grid, the viewer, the pop-out and the view switcher. It is now QrateTableDelegate::cursor_row. --- crates/table/src/delegate.rs | 13 +++++++++---- crates/workspace/src/pop_out.rs | 7 ++----- crates/workspace/src/viewer/mod.rs | 16 ++++------------ crates/workspace/src/views/mod.rs | 7 +------ 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/crates/table/src/delegate.rs b/crates/table/src/delegate.rs index 9cb6cde2..0366e7b0 100644 --- a/crates/table/src/delegate.rs +++ b/crates/table/src/delegate.rs @@ -2058,6 +2058,14 @@ impl QrateTableDelegate { self.selection } + /// The source row the cursor is on: a cell's row or a whole row, and nothing for a column. + pub fn cursor_row(&self) -> Option { + match self.selection? { + Selection::Cell { row, .. } | Selection::Row(row) => Some(row), + Selection::Column(_) => None, + } + } + /// Every selected item as source rows, in view order: the ⌘-clicked set unioned with whatever /// row the cursor is on. This is what Details, the gallery, the selection menu and the status /// bar all count — one answer to "what is selected", so none of them can disagree. @@ -2065,10 +2073,7 @@ impl QrateTableDelegate { /// Filtered-away rows stay in the set but drop out here, so an action reaches what the /// archivist can actually see and clearing the filter brings the rest back. pub fn selected_source_rows(&self) -> Vec { - let cursor = match self.selection { - Some(Selection::Cell { row, .. } | Selection::Row(row)) => Some(row), - Some(Selection::Column(_)) | None => None, - }; + let cursor = self.cursor_row(); self.visible_rows .iter() .copied() diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index 57eac22e..9d423387 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -22,7 +22,7 @@ use gpui_component::{ }; use settings::MainWindowBounds; use settings::project::{CurrentProject, RowId}; -use table::{QrateTableDelegate, Selection, TableChanged, TablePanelHandle, TableStateHandle}; +use table::{QrateTableDelegate, TableChanged, TablePanelHandle, TableStateHandle}; use crate::panels::DetailsPanel; use crate::viewer::{self, Scope, Viewer}; @@ -497,10 +497,7 @@ impl PopOut { let table_row = self .pinned .as_ref() - .and_then(|_| match delegate.selection() { - Some(Selection::Cell { row, .. } | Selection::Row(row)) => Some(row), - _ => None, - }) + .and_then(|_| delegate.cursor_row()) // Only the grid's cursor, not its whole selection: this runs on every repaint. .filter(|row| !self.rows.contains(row)) .and_then(|row| delegate.view_row(row)) diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 8e47e29b..5547eeb2 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -138,12 +138,9 @@ pub(crate) fn build( window, |this: &mut Viewer, table, _: &table::TableChanged, window, cx| { let delegate = table.read(cx).delegate(); - let file = match delegate.selection() { - Some(table::Selection::Cell { row, .. } | table::Selection::Row(row)) => { - previewable(delegate, row) - } - _ => None, - }; + let file = delegate + .cursor_row() + .and_then(|row| previewable(delegate, row)); if let Some(file) = file.filter(|file| *file != this.path) { open_viewer(file, this.scope, window, cx); } @@ -233,12 +230,7 @@ pub(crate) fn step_row(delta: isize, cx: &mut App) { let target = { let delegate = state.read(cx).delegate(); let visible = delegate.visible(); - let from = match delegate.selection() { - Some(table::Selection::Cell { row, .. } | table::Selection::Row(row)) => { - delegate.view_row(row) - } - _ => None, - }; + let from = delegate.cursor_row().and_then(|row| delegate.view_row(row)); from.and_then(|from| { next_row(from, delta, visible.len(), |view| { previewable(delegate, visible[view]).is_some() diff --git a/crates/workspace/src/views/mod.rs b/crates/workspace/src/views/mod.rs index 0f871e50..0ea9db36 100644 --- a/crates/workspace/src/views/mod.rs +++ b/crates/workspace/src/views/mod.rs @@ -237,12 +237,7 @@ impl ViewsPanel { .and_then(WeakEntity::upgrade) .and_then(|state| { let delegate = state.read(cx).delegate(); - match delegate.selection()? { - table::Selection::Cell { row, .. } | table::Selection::Row(row) => { - delegate.view_row(row) - } - table::Selection::Column(_) => None, - } + delegate.view_row(delegate.cursor_row()?) }); if cursor == self.gallery_followed { return; From 49c62a956b52ec8e76841c62e2d044d0ddec8f89 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:18:24 -0700 Subject: [PATCH 27/31] refactor(workspace): share the step to the next previewable row The viewer's row arrows and the pinned pop-out's each looked up the view row, stepped and tested for a previewable file. Both now call viewer::next_previewable. --- crates/workspace/src/pop_out.rs | 10 ++-------- crates/workspace/src/viewer/mod.rs | 26 ++++++++++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index 9d423387..422c3e0f 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -357,15 +357,9 @@ impl PopOut { }; let target = { let delegate = state.read(cx).delegate(); - let visible = delegate.visible(); self.front() - .and_then(|row| delegate.view_row(row)) - .and_then(|from| { - viewer::next_row(from, delta, visible.len(), |view| { - viewer::previewable(delegate, visible[view]).is_some() - }) - }) - .and_then(|view| delegate.row_ids().get(visible[view]).copied()) + .and_then(|row| viewer::next_previewable(delegate, row, delta)) + .and_then(|view| delegate.row_ids().get(delegate.visible()[view]).copied()) }; if let Some(id) = target { self.pinned = Some(vec![id]); diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index 5547eeb2..bfee2553 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -229,13 +229,9 @@ pub(crate) fn step_row(delta: isize, cx: &mut App) { }; let target = { let delegate = state.read(cx).delegate(); - let visible = delegate.visible(); - let from = delegate.cursor_row().and_then(|row| delegate.view_row(row)); - from.and_then(|from| { - next_row(from, delta, visible.len(), |view| { - previewable(delegate, visible[view]).is_some() - }) - }) + delegate + .cursor_row() + .and_then(|row| next_previewable(delegate, row, delta)) }; if let Some(view) = target { state.update(cx, |state, cx| { @@ -245,6 +241,20 @@ pub(crate) fn step_row(delta: isize, cx: &mut App) { } } +/// The view index of the nearest row past source `row`, by `delta`, whose file the viewer can +/// show. `None` at either end, or when `row` is filtered out of the view. +pub(crate) fn next_previewable( + delegate: &table::QrateTableDelegate, + row: usize, + delta: isize, +) -> Option { + let visible = delegate.visible(); + let from = delegate.view_row(row)?; + next_row(from, delta, visible.len(), |view| { + previewable(delegate, visible[view]).is_some() + }) +} + /// The file `row` links to, if the viewer can show it. pub(crate) fn previewable(delegate: &table::QrateTableDelegate, row: usize) -> Option { delegate @@ -254,7 +264,7 @@ pub(crate) fn previewable(delegate: &table::QrateTableDelegate, row: usize) -> O } /// The nearest view index past `from` in `delta`'s direction that `viewable` accepts, if any. -pub(crate) fn next_row( +fn next_row( from: usize, delta: isize, len: usize, From 448b7efb2727f4effe3cbe88a73bc5f1f305e6fc Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:20:10 -0700 Subject: [PATCH 28/31] refactor(workspace): share the selection stack's front and wrap The pop-out's stack copied Details' clamp-to-last and wrap-around maths verbatim. Both now use details::stack_front and stack_step. --- crates/workspace/src/panels/details.rs | 57 ++++++++++++++++++-------- crates/workspace/src/pop_out.rs | 13 ++---- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 47b44566..70c09ed8 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -256,21 +256,13 @@ impl DetailsPanel { .unwrap_or_default() } - /// The item the preview is showing: the stack's front card. Clamped rather than remembered, so - /// stepping to the fifth of five and then selecting two doesn't leave the preview blank. - fn front(&self, picked: &[usize]) -> Option { - picked - .get(self.stack.min(picked.len().checked_sub(1)?)) - .copied() - } - /// Point the transport at whatever is selected now. A no-op while the selection stays on the /// same file — this runs on every table change, and rebuilding would re-probe the file and /// throw away the position on every keystroke in the grid. fn retarget(&mut self, cx: &mut Context) { self.fields = None; let picked = self.picked(cx); - let front = self.front(&picked); + let front = stack_front(&picked, self.stack); self.load_row_history(front, cx); // The pop-out's stage has the file, its caption and its transport. if self.rows.is_some() { @@ -822,12 +814,7 @@ impl DetailsPanel { /// Move the preview stack one item along, wrapping at both ends so a bundle can be walked in /// either direction without hunting for the end of it. fn step_stack(&mut self, forward: bool, cx: &mut Context) { - let count = self.picked(cx).len().max(1); - let at = self.stack.min(count - 1); - self.stack = match forward { - true => (at + 1) % count, - false => (at + count - 1) % count, - }; + self.stack = stack_step(self.stack, self.picked(cx).len(), forward); self.retarget(cx); cx.notify(); } @@ -1152,6 +1139,22 @@ fn render_image_frame( /// One of the preview stack's step arrows, pinned to the edge its chevron points at and centred /// down the card. Full-height flex rather than a top offset: the pane is a height the user drags, /// so there is no fixed centre to hardcode. +/// The item a stack of selected `rows` shows: the front card. Clamped rather than remembered, so +/// stepping to the fifth of five and then selecting two doesn't leave the preview blank. +pub(crate) fn stack_front(rows: &[usize], stack: usize) -> Option { + rows.get(stack.min(rows.len().checked_sub(1)?)).copied() +} + +/// The front card after one step through `count` cards, wrapping at either end. +pub(crate) fn stack_step(stack: usize, count: usize, forward: bool) -> usize { + let count = count.max(1); + let at = stack.min(count - 1); + match forward { + true => (at + 1) % count, + false => (at + count - 1) % count, + } +} + fn step( id: &'static str, left: bool, @@ -1230,7 +1233,7 @@ fn shared_fields(delegate: &QrateTableDelegate, picked: &[usize]) -> Vec) -> impl IntoElement { let picked = self.picked(cx); - let front = self.front(&picked); + let front = stack_front(&picked, self.stack); let count = picked.len(); let selection = self.state.as_ref().and_then(|w| w.upgrade()).map(|s| { let delegate = s.read(cx).delegate(); @@ -1816,6 +1819,28 @@ mod tests { use super::{DetailsPanel, render_image_frame}; + /// The stack wraps at both ends, and a remembered position past a smaller selection lands + /// on its last card rather than on nothing. + #[test] + fn the_stack_wraps_and_clamps_to_the_selection() { + use super::{stack_front, stack_step}; + + assert_eq!( + stack_step(2, 3, true), + 0, + "past the last wraps to the first" + ); + assert_eq!( + stack_step(0, 3, false), + 2, + "before the first wraps to the last" + ); + assert_eq!(stack_step(4, 2, true), 0, "a stale position clamps first"); + assert_eq!(stack_step(0, 0, true), 0, "nothing selected goes nowhere"); + assert_eq!(stack_front(&[7, 8], 5), Some(8)); + assert_eq!(stack_front(&[], 0), None); + } + /// Wraps `render_image_frame` in a root `Render` view so a test can actually draw it — /// `Img`'s real load/fallback logic runs during layout/paint, not at element construction, /// so building the element tree alone (without a window draw) wouldn't exercise it. diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index 422c3e0f..92b2c59b 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -250,9 +250,7 @@ impl PopOut { /// The item the stage shows: the stack's front card, clamped so a smaller selection never /// leaves it pointing past the end. fn front(&self) -> Option { - self.rows - .get(self.stack.min(self.rows.len().checked_sub(1)?)) - .copied() + crate::panels::details::stack_front(&self.rows, self.stack) } /// Bring the rows, the stage and the sidebar up to date with the grid. @@ -369,15 +367,10 @@ impl PopOut { /// Walk the stack of selected items, wrapping at both ends like the Details preview does. fn step_stack(&mut self, forward: bool, window: &mut Window, cx: &mut Context) { - let count = self.rows.len(); - if count < 2 { + if self.rows.len() < 2 { return; } - let at = self.stack.min(count - 1); - self.stack = match forward { - true => (at + 1) % count, - false => (at + count - 1) % count, - }; + self.stack = crate::panels::details::stack_step(self.stack, self.rows.len(), forward); self.sync(window, cx); } From cf257838b253b82505a0b42405b6d7bdfbf850ee Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:22:06 -0700 Subject: [PATCH 29/31] refactor(table): convert between row ids and rows in one place The id-to-row map was built by hand in four places and scanned for in two more, and rows were turned into ids through row_ids().get() in three. The delegate now has row_positions, row_of and a public row_id. --- crates/table/src/delegate.rs | 16 +++++++++++++++- crates/table/src/lib.rs | 18 +++--------------- crates/workspace/src/panels/details.rs | 12 +++--------- crates/workspace/src/panels/history.rs | 7 +------ crates/workspace/src/pop_out.rs | 12 +++++------- 5 files changed, 27 insertions(+), 38 deletions(-) diff --git a/crates/table/src/delegate.rs b/crates/table/src/delegate.rs index 0366e7b0..dad47873 100644 --- a/crates/table/src/delegate.rs +++ b/crates/table/src/delegate.rs @@ -1825,10 +1825,24 @@ impl QrateTableDelegate { ); } - pub(crate) fn row_id(&self, source: usize) -> Option { + pub fn row_id(&self, source: usize) -> Option { self.row_ids.get(source).copied() } + /// The source row `id` sits at now. A scan; for a batch of ids, [`Self::row_positions`]. + pub fn row_of(&self, id: settings::project::RowId) -> Option { + self.row_ids.iter().position(|row| *row == id) + } + + /// Where every row id sits now, for finding a batch of ids again after rows moved. + pub fn row_positions(&self) -> std::collections::HashMap { + self.row_ids + .iter() + .enumerate() + .map(|(row, id)| (*id, row)) + .collect() + } + pub fn row_ids(&self) -> &[settings::project::RowId] { &self.row_ids } diff --git a/crates/table/src/lib.rs b/crates/table/src/lib.rs index 95b81816..29644fd7 100644 --- a/crates/table/src/lib.rs +++ b/crates/table/src/lib.rs @@ -401,12 +401,7 @@ pub(crate) fn file_rows( /// The rows `changes` put new text in or back into, as they now sit; `None` when a column moved, /// which can change what every row resolves to. fn changed_rows(delegate: &QrateTableDelegate, changes: &[Change]) -> Option> { - let position: std::collections::HashMap<_, _> = delegate - .row_ids() - .iter() - .enumerate() - .map(|(at, id)| (*id, at)) - .collect(); + let position = delegate.row_positions(); changes .iter() .filter_map(|change| match change { @@ -533,10 +528,7 @@ pub fn restore_to(to: EntryId, cx: &mut App) { else { continue; }; - let position = row.and_then(|id| { - let delegate = state.read(cx).delegate(); - delegate.row_ids().iter().position(|r| *r == id) - }); + let position = row.and_then(|id| state.read(cx).delegate().row_of(id)); if row.is_some() && position.is_none() { continue; } @@ -571,11 +563,7 @@ pub fn restore_value( }; let target = { let delegate = state.read(cx).delegate(); - delegate - .row_ids() - .iter() - .position(|id| *id == row) - .zip(delegate.data_col(column)) + delegate.row_of(row).zip(delegate.data_col(column)) }; if let Some((row, col)) = target { write_cell(row, col, text, Origin::Restore(from), cx); diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 70c09ed8..281cdefb 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -313,7 +313,7 @@ impl DetailsPanel { fn load_row_history(&mut self, front: Option, cx: &mut Context) { let row = front.and_then(|row| { let state = self.state.as_ref()?.upgrade()?; - state.read(cx).delegate().row_ids().get(row).copied() + state.read(cx).delegate().row_id(row) }); let file = cx .try_global::() @@ -382,8 +382,7 @@ impl DetailsPanel { .filter(|_| !picked.is_empty()) .and_then(|s| { let delegate = s.read(cx).delegate(); - let ids = delegate.row_ids(); - let rows = picked.iter().filter_map(|&row| ids.get(row).copied()); + let rows = picked.iter().filter_map(|&row| delegate.row_id(row)); Some((rows.collect::>(), delegate.data_col(header)?)) }); let Some((rows, col)) = located else { @@ -835,12 +834,7 @@ impl DetailsPanel { .and_then(|state| { let delegate = state.read(cx).delegate(); let col = delegate.data_col(&header)?; - let at: std::collections::HashMap<_, _> = delegate - .row_ids() - .iter() - .enumerate() - .map(|(row, id)| (*id, row)) - .collect(); + let at = delegate.row_positions(); let rows = ids.iter().filter_map(|id| at.get(id).copied()); Some( rows.map(|row| (row, col, value.clone())) diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index b7271bbf..45271c3b 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -1055,12 +1055,7 @@ impl HistoryPanel { .map(|state| { let delegate = state.read(cx).delegate(); ( - delegate - .row_ids() - .iter() - .enumerate() - .map(|(p, id)| (*id, p)) - .collect(), + delegate.row_positions(), delegate .unsaved_history() .iter() diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index 92b2c59b..d7020436 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -258,7 +258,6 @@ impl PopOut { let mut kept = None; let rows = self.table().map_or_else(Vec::new, |state| { let delegate = state.read(cx).delegate(); - let all = delegate.row_ids(); match &self.pinned { // Where the pin was last found, while every row is still there: a cell edit is the // common change, and it moves nothing. @@ -267,15 +266,14 @@ impl PopOut { && ids .iter() .zip(&self.rows) - .all(|(id, &row)| all.get(row) == Some(id)) => + .all(|(id, &row)| delegate.row_id(row) == Some(*id)) => { self.rows.clone() } // Rows were added, removed or moved: one pass to find the pin again, dropping the // items that are gone so the pass above matches again from the next change. Some(ids) => { - let at: std::collections::HashMap<_, _> = - all.iter().enumerate().map(|(row, id)| (*id, row)).collect(); + let at = delegate.row_positions(); let (ids, rows): (Vec<_>, Vec<_>) = ids .iter() .filter_map(|id| Some((*id, *at.get(id)?))) @@ -336,9 +334,9 @@ impl PopOut { /// The row ids of `rows`, which is what a pin holds on to. fn ids(&self, rows: &[usize], cx: &App) -> Vec { self.table().map_or_else(Vec::new, |state| { - let ids = state.read(cx).delegate().row_ids(); + let delegate = state.read(cx).delegate(); rows.iter() - .filter_map(|&row| ids.get(row).copied()) + .filter_map(|&row| delegate.row_id(row)) .collect() }) } @@ -357,7 +355,7 @@ impl PopOut { let delegate = state.read(cx).delegate(); self.front() .and_then(|row| viewer::next_previewable(delegate, row, delta)) - .and_then(|view| delegate.row_ids().get(delegate.visible()[view]).copied()) + .and_then(|view| delegate.row_id(delegate.visible()[view])) }; if let Some(id) = target { self.pinned = Some(vec![id]); From f3f97284ed2626fdca40a5a263abf5c529c61e74 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:24:34 -0700 Subject: [PATCH 30/31] refactor(table): reach the centre table through TablePanelHandle::update The look-up, upgrade and update was written out at eight sites across five crates, and the grid's menu handlers kept a private copy. They all use one public TablePanelHandle::update now. --- crates/app/src/status_items/new_files.rs | 7 +---- crates/onboarding/src/lib.rs | 19 +++++-------- crates/table/src/cell.rs | 11 ++------ crates/table/src/lib.rs | 12 ++++++++ crates/table/src/panel.rs | 36 ++++++++++-------------- crates/workspace/src/panels/details.rs | 33 ++++++---------------- crates/workspace/src/pop_out.rs | 9 ++---- 7 files changed, 50 insertions(+), 77 deletions(-) diff --git a/crates/app/src/status_items/new_files.rs b/crates/app/src/status_items/new_files.rs index 7b361a5b..55f3bca0 100644 --- a/crates/app/src/status_items/new_files.rs +++ b/crates/app/src/status_items/new_files.rs @@ -40,12 +40,7 @@ impl Render for NewFilesButton { .label(format!("New files ({count})")) .tooltip("Files in the files folder that no row links to") .on_click(|_, window, cx| { - if let Some(table) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - table.update(cx, |table, cx| table.import_new_files(window, cx)); - } + TablePanelHandle::update(cx, |table, cx| table.import_new_files(window, cx)); }) .into_any_element() } diff --git a/crates/onboarding/src/lib.rs b/crates/onboarding/src/lib.rs index 4d8679e7..fba0df83 100644 --- a/crates/onboarding/src/lib.rs +++ b/crates/onboarding/src/lib.rs @@ -706,18 +706,13 @@ fn show_me(guide: &Entity, task: Task, window: &mut Window, cx: &mut App) } Task::AddFilesFolder => { let blank = guide.read(cx).kind == GuideKind::Blank; - if let Some(table) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - table.update(cx, |table, cx| { - if blank { - table.choose_import_paths(window, cx) - } else { - table.choose_files_root(window, cx) - } - }); - } + table::TablePanelHandle::update(cx, |table, cx| { + if blank { + table.choose_import_paths(window, cx) + } else { + table.choose_files_root(window, cx) + } + }); } Task::OpenRow | Task::SelectRow => { ensure_visible(guide, DETAILS_META.name, window, cx); diff --git a/crates/table/src/cell.rs b/crates/table/src/cell.rs index 920ef41d..cd459002 100644 --- a/crates/table/src/cell.rs +++ b/crates/table/src/cell.rs @@ -128,14 +128,9 @@ pub(crate) fn render_cell( return; }; window.defer(cx, move |window, cx| { - if let Some(panel) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - panel.update(cx, |panel, cx| { - panel.link_dropped_file(row_ix, col_ix, path, window, cx) - }); - } + crate::TablePanelHandle::update(cx, |panel, cx| { + panel.link_dropped_file(row_ix, col_ix, path, window, cx) + }); }); }) }) diff --git a/crates/table/src/lib.rs b/crates/table/src/lib.rs index 29644fd7..b1e1e841 100644 --- a/crates/table/src/lib.rs +++ b/crates/table/src/lib.rs @@ -44,6 +44,18 @@ pub use visual::remove_model as remove_visual_model; pub struct TablePanelHandle(pub WeakEntity); impl Global for TablePanelHandle {} +impl TablePanelHandle { + /// Run `run` on the centre table, when one is open. + pub fn update(cx: &mut App, run: impl FnOnce(&mut TablePanel, &mut gpui::Context)) { + if let Some(panel) = cx + .try_global::() + .and_then(|handle| handle.0.upgrade()) + { + panel.update(cx, run); + } + } +} + /// Settings key (in either scope) for the alternating-row-stripe toggle. pub const TABLE_STRIPES_KEY: &str = "table_stripes"; diff --git a/crates/table/src/panel.rs b/crates/table/src/panel.rs index dccc63d4..2306716d 100644 --- a/crates/table/src/panel.rs +++ b/crates/table/src/panel.rs @@ -1882,14 +1882,6 @@ impl TablePanel { /// App-level handlers for the grid's menu commands, so the menu bar reaches them wherever focus /// sits rather than only while the grid holds it. Undo, Redo and Deselect are the app's own. pub fn register_global_actions(cx: &mut App) { - fn on_panel(cx: &mut App, run: impl FnOnce(&mut TablePanel, &mut Context)) { - if let Some(panel) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - panel.update(cx, run); - } - } // A global handler runs while the dispatching window is taken out; wait for it to come back. fn in_window( cx: &mut App, @@ -1915,12 +1907,12 @@ pub fn register_global_actions(cx: &mut App) { } cx.on_action(|_: &InsertRowAbove, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural(|rows, _| crate::Structural::InsertRow { at: rows[0] }, cx) }) }); cx.on_action(|_: &InsertRowBelow, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural( |rows, _| crate::Structural::InsertRow { at: rows[rows.len() - 1] + 1, @@ -1930,7 +1922,7 @@ pub fn register_global_actions(cx: &mut App) { }) }); cx.on_action(|_: &DuplicateRow, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural( |rows, _| crate::Structural::DuplicateRow { row: rows[0] }, cx, @@ -1938,45 +1930,47 @@ pub fn register_global_actions(cx: &mut App) { }) }); cx.on_action(|_: &DeleteRow, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural(|rows, _| crate::Structural::DeleteRows(rows.to_vec()), cx) }) }); cx.on_action(|_: &InsertColumnLeft, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural(|_, col| crate::Structural::InsertColumn { at: col }, cx) }) }); cx.on_action(|_: &InsertColumnRight, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural(|_, col| crate::Structural::InsertColumn { at: col + 1 }, cx) }) }); cx.on_action(|_: &DeleteColumn, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.structural(|_, col| crate::Structural::DeleteColumn { col }, cx) }) }); cx.on_action(|_: &IndentRow, cx| { - on_panel(cx, |this, cx| arrange(this, crate::Arrangement::Indent, cx)) + crate::TablePanelHandle::update(cx, |this, cx| { + arrange(this, crate::Arrangement::Indent, cx) + }) }); cx.on_action(|_: &OutdentRow, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { arrange(this, crate::Arrangement::Outdent, cx) }) }); cx.on_action(|_: &DeleteSubtree, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { arrange(this, crate::Arrangement::DeleteSubtree, cx) }) }); cx.on_action(|_: &UnfreezeColumns, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { crate::set_frozen_columns(&this.state.clone(), 0, cx) }) }); cx.on_action(|_: &ExpandAll, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { let expanded = this.state.update(cx, |state, cx| { state.delegate_mut().expand_all(); let expanded = state.delegate().expanded_rows(); @@ -1988,7 +1982,7 @@ pub fn register_global_actions(cx: &mut App) { }) }); cx.on_action(|_: &CollapseAll, cx| { - on_panel(cx, |this, cx| { + crate::TablePanelHandle::update(cx, |this, cx| { this.state.update(cx, |state, cx| { state.delegate_mut().collapse_all(); state.refresh(cx); diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 281cdefb..10ccf055 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -1273,14 +1273,9 @@ impl Render for DetailsPanel { style.bg(cx.theme().secondary_hover) }) .on_drop(|paths: &ExternalPaths, window, cx| { - if let Some(table) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - table.update(cx, |table, cx| { - table.import_external_paths(paths.paths().to_vec(), window, cx) - }); - } + TablePanelHandle::update(cx, |table, cx| { + table.import_external_paths(paths.paths().to_vec(), window, cx) + }); }) .flex() .flex_col() @@ -1496,14 +1491,9 @@ impl Render for DetailsPanel { .small() .label("Locate file…") .on_click(move |_, window, cx| { - if let Some(table) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - table.update(cx, |table, cx| { - table.locate_file(row, window, cx) - }); - } + TablePanelHandle::update(cx, |table, cx| { + table.locate_file(row, window, cx) + }); }), ), ) @@ -1635,14 +1625,9 @@ impl Render for DetailsPanel { .size_full() .drag_over::(|style, _, _, cx| style.bg(cx.theme().secondary_hover)) .on_drop(|paths: &ExternalPaths, window, cx| { - if let Some(table) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - table.update(cx, |table, cx| { - table.import_external_paths(paths.paths().to_vec(), window, cx) - }); - } + TablePanelHandle::update(cx, |table, cx| { + table.import_external_paths(paths.paths().to_vec(), window, cx) + }); }) // The whole panel gives the bottom-strip crop back at once, rather than each scrolling // region padding itself: the split below sizes its panes against whatever height it is diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index d7020436..3788767b 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -687,12 +687,9 @@ impl PopOut { .mt_1() .label("Locate file…") .on_click(move |_, window, cx| { - if let Some(table) = cx - .try_global::() - .and_then(|handle| handle.0.upgrade()) - { - table.update(cx, |table, cx| table.locate_file(row, window, cx)); - } + TablePanelHandle::update(cx, |table, cx| { + table.locate_file(row, window, cx) + }); }), ), Some((_, file, _)) => { From b955d06a96f804a5657be12c859c4065d35a1060 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:30:17 -0700 Subject: [PATCH 31/31] test: share the WAV and table fixtures The silent-WAV header was built by hand in four tests across two crates; it is preview::playback::silent_wav now, behind a test-support feature workspace's tests turn on. The Details and pop-out tests share test_support::open_table, and the table panel's tests start from one project with autosave already off. --- crates/preview/Cargo.toml | 4 ++ crates/preview/src/audio.rs | 17 +------ crates/preview/src/playback.rs | 41 ++++++++++------- crates/table/src/panel.rs | 50 ++++++-------------- crates/workspace/Cargo.toml | 1 + crates/workspace/src/lib.rs | 38 +++++++++++++++ crates/workspace/src/panels/details.rs | 48 ++++++++----------- crates/workspace/src/pop_out.rs | 64 +++++++------------------- crates/workspace/src/viewer/mod.rs | 17 +------ 9 files changed, 120 insertions(+), 160 deletions(-) diff --git a/crates/preview/Cargo.toml b/crates/preview/Cargo.toml index f558e627..db810cd4 100644 --- a/crates/preview/Cargo.toml +++ b/crates/preview/Cargo.toml @@ -26,6 +26,10 @@ pdfium-render = { version = "0.9.3", default-features = false, features = ["pdfi # ship two whole symphonia trees in the binary. Move both together or neither. symphonia = { version = "0.5.5", default-features = false, features = ["mp3", "aac", "alac", "isomp4", "ogg", "vorbis", "flac", "wav", "pcm"] } +[features] +# Test fixtures other crates' tests use, such as `playback::silent_wav`. +test-support = [] + [dev-dependencies] # `#[gpui::test]`, TestAppContext, and run_test are all behind this feature. gpui = { workspace = true, features = ["test-support"] } diff --git a/crates/preview/src/audio.rs b/crates/preview/src/audio.rs index f9cd7183..7e1c391f 100644 --- a/crates/preview/src/audio.rs +++ b/crates/preview/src/audio.rs @@ -132,22 +132,7 @@ mod tests { /// rather than erroring in a way that would look the same as an unreadable file. #[test] fn a_valid_recording_without_artwork_is_not_an_error() { - // 44-byte canonical WAV header describing one sample of silence. - let mut wav = Vec::new(); - wav.extend(b"RIFF"); - wav.extend(36u32.to_le_bytes()); - wav.extend(b"WAVEfmt "); - wav.extend(16u32.to_le_bytes()); - wav.extend(1u16.to_le_bytes()); // PCM - wav.extend(1u16.to_le_bytes()); // mono - wav.extend(8000u32.to_le_bytes()); - wav.extend(16000u32.to_le_bytes()); - wav.extend(2u16.to_le_bytes()); - wav.extend(16u16.to_le_bytes()); - wav.extend(b"data"); - wav.extend(2u32.to_le_bytes()); - wav.extend(0u16.to_le_bytes()); - + let wav = crate::playback::silent_wav(1); let path = std::env::temp_dir().join("qrate-audio-silent.wav"); std::fs::write(&path, &wav).unwrap(); assert!(audio::cover(&path).is_none(), "no artwork, but no panic"); diff --git a/crates/preview/src/playback.rs b/crates/preview/src/playback.rs index ffde94c9..5b2c07f5 100644 --- a/crates/preview/src/playback.rs +++ b/crates/preview/src/playback.rs @@ -123,6 +123,28 @@ pub fn stop(owner: EntityId, cx: &mut App) { } } +/// A real WAV of `samples` of 8 kHz 16-bit mono silence — a canonical 44-byte header and the +/// zeros — for tests that need a recording on disk. +#[cfg(any(test, feature = "test-support"))] +pub fn silent_wav(samples: usize) -> Vec { + let data = (samples * 2) as u32; + let mut wav = Vec::new(); + wav.extend(b"RIFF"); + wav.extend((36 + data).to_le_bytes()); + wav.extend(b"WAVEfmt "); + wav.extend(16u32.to_le_bytes()); + wav.extend(1u16.to_le_bytes()); // PCM + wav.extend(1u16.to_le_bytes()); // mono + wav.extend(8000u32.to_le_bytes()); + wav.extend(16000u32.to_le_bytes()); + wav.extend(2u16.to_le_bytes()); + wav.extend(16u16.to_le_bytes()); + wav.extend(b"data"); + wav.extend(data.to_le_bytes()); + wav.extend(std::iter::repeat_n(0u8, data as usize)); + wav +} + #[cfg(test)] mod tests { // Never `use super::*` here — a chained `gpui::*` glob would shadow `#[test]`. @@ -132,23 +154,8 @@ mod tests { /// is every CI runner. Nothing here opens an output device. #[test] fn a_recordings_length_is_read_without_playing_it() { - // 44-byte canonical WAV header, then one second of 8 kHz 16-bit mono silence. - let samples = 8000usize; - let data = samples * 2; - let mut wav = Vec::new(); - wav.extend(b"RIFF"); - wav.extend((36 + data as u32).to_le_bytes()); - wav.extend(b"WAVEfmt "); - wav.extend(16u32.to_le_bytes()); - wav.extend(1u16.to_le_bytes()); // PCM - wav.extend(1u16.to_le_bytes()); // mono - wav.extend(8000u32.to_le_bytes()); - wav.extend(16000u32.to_le_bytes()); - wav.extend(2u16.to_le_bytes()); - wav.extend(16u16.to_le_bytes()); - wav.extend(b"data"); - wav.extend((data as u32).to_le_bytes()); - wav.extend(std::iter::repeat_n(0u8, data)); + // One second of 8 kHz silence. + let wav = crate::playback::silent_wav(8000); let path = std::env::temp_dir().join("qrate-playback-duration.wav"); std::fs::write(&path, &wav).unwrap(); diff --git a/crates/table/src/panel.rs b/crates/table/src/panel.rs index 2306716d..177e4b34 100644 --- a/crates/table/src/panel.rs +++ b/crates/table/src/panel.rs @@ -2259,7 +2259,13 @@ mod tests { fn project_with_notes(cx: &mut TestAppContext) { cx.update(|cx| { gpui_component::init(cx); - cx.set_global(settings::AppSettings::default()); + // Autosave off, so no test here writes the temp project file. + let mut app = settings::AppSettings::default(); + app.values.insert( + settings::AUTOSAVE_KEY.into(), + settings::Val::Text("off".into()), + ); + cx.set_global(app); cx.set_global(settings::project::CurrentProject { file: std::env::temp_dir().join("qrate-note-cancel.qrate"), data: settings::project::ProjectData { @@ -2281,10 +2287,6 @@ mod tests { use gpui::BorrowAppContext as _; cx.update_global::(|project, _| { project.data.rows = vec![vec!["Agnès Varda".into()], vec!["Varda, Agnès".into()]]; - project.data.values.insert( - settings::AUTOSAVE_KEY.into(), - settings::Val::Text("off".into()), - ); project.data.values.insert( settings::columns::COLUMN_SETTINGS_KEY.into(), settings::Val::Text(r#"{"Title":{"variant_review":true}}"#.into()), @@ -2394,15 +2396,6 @@ mod tests { #[gpui::test] fn grouped_fixes_write_every_cell_in_one_undo_step(cx: &mut TestAppContext) { project_with_notes(cx); - cx.update(|cx| { - use gpui::BorrowAppContext as _; - cx.update_global::(|project, _| { - project.data.values.insert( - settings::AUTOSAVE_KEY.into(), - settings::Val::Text("off".into()), - ); - }); - }); let (panel, cx) = cx.add_window_view(super::TablePanel::new); panel.update(cx, |panel, cx| { crate::set_cell_texts( @@ -2524,30 +2517,17 @@ mod tests { } /// What Backspace and Delete do to the selection, and the promise that it is one undo step. - /// Autosave off so the temp project file is never written. #[gpui::test] fn clearing_the_selection_blanks_it_and_undoes_as_one_step(cx: &mut TestAppContext) { + project_with_notes(cx); cx.update(|cx| { - gpui_component::init(cx); - let mut app = settings::AppSettings::default(); - app.values.insert( - settings::AUTOSAVE_KEY.into(), - settings::Val::Text("off".into()), - ); - cx.set_global(app); - cx.set_global(settings::project::CurrentProject { - file: std::env::temp_dir().join("qrate-clear-range.qrate"), - data: settings::project::ProjectData { - name: "T".into(), - columns: Vec::new(), - headers: vec!["Medium".into(), "Title".into()], - rows: vec![ - vec!["Film".into(), "one".into()], - vec!["Video".into(), "two".into()], - ], - row_ids: vec![1, 2], - values: Default::default(), - }, + use gpui::BorrowAppContext as _; + cx.update_global::(|project, _| { + project.data.headers = vec!["Medium".into(), "Title".into()]; + project.data.rows = vec![ + vec!["Film".into(), "one".into()], + vec!["Video".into(), "two".into()], + ]; }); }); let (panel, cx) = cx.add_window_view(super::TablePanel::new); diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 2cdb7770..7caece29 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -21,3 +21,4 @@ components = { path = "../components" } [dev-dependencies] # `#[gpui::test]`, TestAppContext, and run_test are all behind this feature. gpui = { workspace = true, features = ["test-support"] } +preview = { path = "../preview", features = ["test-support"] } diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index f8a8a1b5..fde23c5d 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -934,3 +934,41 @@ mod tests { assert_eq!(children[0]["panel_name"], "ViewsPanel"); } } + +/// What the panel and pop-out tests share: a real grid over a project, as the app builds one. +#[cfg(test)] +pub(crate) mod test_support { + use gpui::{Entity, TestAppContext}; + use gpui_component::table::TableState; + use table::QrateTableDelegate; + + /// A table panel in its own window over `data`, saved to `file` in the temp directory. Autosave + /// is off, so a committed edit never writes that file. + pub(crate) fn open_table( + cx: &mut TestAppContext, + file: &str, + data: settings::project::ProjectData, + ) -> Entity> { + cx.update(|cx| { + gpui_component::init(cx); + let mut app = settings::AppSettings::default(); + app.values.insert( + settings::AUTOSAVE_KEY.into(), + settings::Val::Text("off".into()), + ); + cx.set_global(app); + cx.set_global(settings::SettingsPersistence::default()); + cx.set_global(settings::project::CurrentProject { + file: std::env::temp_dir().join(file), + data, + }); + }); + cx.add_window_view(table::TablePanel::new); + cx.update(|cx| { + cx.global::() + .0 + .upgrade() + .expect("the table panel publishes its state handle") + }) + } +} diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 10ccf055..fb529e55 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -1888,36 +1888,26 @@ mod tests { cx.add_window_view(DetailsPanel::new); } - /// A real table behind the panel, with autosave off so a committed edit doesn't write the - /// temp project file. Same shape as `table::delegate`'s own fixture. + /// A real table behind the panel: three rows, two of which share a Medium. fn project_with_table(cx: &mut TestAppContext) { - cx.update(|cx| { - gpui_component::init(cx); - let mut app = settings::AppSettings::default(); - app.values.insert( - settings::AUTOSAVE_KEY.into(), - settings::Val::Text("off".into()), - ); - cx.set_global(app); - cx.set_global(settings::project::CurrentProject { - file: std::env::temp_dir().join("qrate-details-edit.qrate"), - data: settings::project::ProjectData { - name: "T".into(), - columns: Vec::new(), - headers: vec!["Medium".into(), "Title".into()], - rows: vec![ - vec!["Film".into(), "one".into()], - vec!["Video".into(), "two".into()], - // Shares a Medium with row 0 but not a Title, so a selection of the two - // has one agreed field and one mixed. - vec!["Film".into(), "three".into()], - ], - row_ids: vec![1, 2, 3], - values: Default::default(), - }, - }); - }); - cx.add_window_view(table::TablePanel::new); + crate::test_support::open_table( + cx, + "qrate-details-edit.qrate", + settings::project::ProjectData { + name: "T".into(), + columns: Vec::new(), + headers: vec!["Medium".into(), "Title".into()], + rows: vec![ + vec!["Film".into(), "one".into()], + vec!["Video".into(), "two".into()], + // Shares a Medium with row 0 but not a Title, so a selection of the two has + // one agreed field and one mixed. + vec!["Film".into(), "three".into()], + ], + row_ids: vec![1, 2, 3], + values: Default::default(), + }, + ); } #[gpui::test] diff --git a/crates/workspace/src/pop_out.rs b/crates/workspace/src/pop_out.rs index 3788767b..9ab551b8 100644 --- a/crates/workspace/src/pop_out.rs +++ b/crates/workspace/src/pop_out.rs @@ -1042,38 +1042,22 @@ mod tests { Entity>, &mut VisualTestContext, ) { - cx.update(|cx| { - gpui_component::init(cx); - let mut app = settings::AppSettings::default(); - app.values.insert( - settings::AUTOSAVE_KEY.into(), - settings::Val::Text("off".into()), - ); - cx.set_global(app); - cx.set_global(settings::SettingsPersistence::default()); - cx.set_global(settings::project::CurrentProject { - file: std::env::temp_dir().join("qrate-pop-out.qrate"), - data: settings::project::ProjectData { - name: "Aderman Collection".into(), - columns: Vec::new(), - headers: vec!["Identifier".into(), "Title".into()], - rows: vec![ - vec!["ADR-0042".into(), "Beacon Hill Park".into()], - vec!["ADR-0043".into(), "Sawmill crew".into()], - vec!["ADR-0044".into(), "Saanich mill".into()], - ], - row_ids: vec![11, 12, 13], - values: Default::default(), - }, - }); - }); - cx.add_window_view(table::TablePanel::new); - let state = cx.update(|cx| { - cx.global::() - .0 - .upgrade() - .expect("the table panel publishes its state handle") - }); + let state = crate::test_support::open_table( + cx, + "qrate-pop-out.qrate", + settings::project::ProjectData { + name: "Aderman Collection".into(), + columns: Vec::new(), + headers: vec!["Identifier".into(), "Title".into()], + rows: vec![ + vec!["ADR-0042".into(), "Beacon Hill Park".into()], + vec!["ADR-0043".into(), "Sawmill crew".into()], + vec!["ADR-0044".into(), "Saanich mill".into()], + ], + row_ids: vec![11, 12, 13], + values: Default::default(), + }, + ); let (pop_out, cx) = cx.add_window_view(PopOut::new); (pop_out, state, cx) } @@ -1254,21 +1238,7 @@ mod tests { /// and holds trivially on one without. #[gpui::test] fn leaving_a_file_stops_only_its_own_recording(cx: &mut TestAppContext) { - let data = 8000usize * 2; - let mut wav = Vec::new(); - wav.extend(b"RIFF"); - wav.extend((36 + data as u32).to_le_bytes()); - wav.extend(b"WAVEfmt "); - wav.extend(16u32.to_le_bytes()); - wav.extend(1u16.to_le_bytes()); - wav.extend(1u16.to_le_bytes()); - wav.extend(8000u32.to_le_bytes()); - wav.extend(16000u32.to_le_bytes()); - wav.extend(2u16.to_le_bytes()); - wav.extend(16u16.to_le_bytes()); - wav.extend(b"data"); - wav.extend((data as u32).to_le_bytes()); - wav.extend(std::iter::repeat_n(0u8, data)); + let wav = preview::playback::silent_wav(8000); let shown = std::env::temp_dir().join("qrate-pop-out-shown.wav"); std::fs::write(&shown, &wav).unwrap(); diff --git a/crates/workspace/src/viewer/mod.rs b/crates/workspace/src/viewer/mod.rs index bfee2553..fac89225 100644 --- a/crates/workspace/src/viewer/mod.rs +++ b/crates/workspace/src/viewer/mod.rs @@ -1801,22 +1801,7 @@ mod tests { #[gpui::test] fn closing_the_viewer_leaves_nothing_playing(cx: &mut TestAppContext) { let cx = with_window(cx); - // 44-byte canonical WAV header, then a second of 8 kHz 16-bit mono silence. - let data = 8000usize * 2; - let mut wav = Vec::new(); - wav.extend(b"RIFF"); - wav.extend((36 + data as u32).to_le_bytes()); - wav.extend(b"WAVEfmt "); - wav.extend(16u32.to_le_bytes()); - wav.extend(1u16.to_le_bytes()); // PCM - wav.extend(1u16.to_le_bytes()); // mono - wav.extend(8000u32.to_le_bytes()); - wav.extend(16000u32.to_le_bytes()); - wav.extend(2u16.to_le_bytes()); - wav.extend(16u16.to_le_bytes()); - wav.extend(b"data"); - wav.extend((data as u32).to_le_bytes()); - wav.extend(std::iter::repeat_n(0u8, data)); + let wav = preview::playback::silent_wav(8000); let path = std::env::temp_dir().join("qrate-viewer-close-stops.wav"); std::fs::write(&path, &wav).unwrap();