diff --git a/CLAUDE.md b/CLAUDE.md index b6a348eb..6e4814fc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ A 2026-07-17 whole-repo audit removed exactly this kind of code; don't reintrodu - **No single-line forwarding wrappers.** A function whose body is one call to another function with no added logic gets deleted — make the target `pub` and call it directly (e.g. the removed `persist_layout_on_quit`). Exception: thin accessors that are the only route to a private field (`QrateTableDelegate::cell`/`row_image`) are encapsulation, keep those. - **No `new()`/`Default` for fieldless unit structs.** Construct `StatusBar`-style structs directly: `cx.new(|_| StatusBar)`. - **One builder function per UI component, conditions inline.** Don't split a gpui component into a helper fn per visual part — use closures, `FluentBuilder::map`/`when`, and `match` inside a single function (see `render_image_frame` in `crates/workspace/src/panels/details.rs`). If a builder is genuinely shared across multiple `Render` impls, return `AnyElement` — propagating gpui's nested builder generics into several callers overflows rustc's stack at type-check time. -- **No speculative scaffolding.** Code "kept for later" outside the module tree, provider stubs full of `todo!()`, and builder methods only tests call are deletions, not investments — git history is the archive. +- **No speculative scaffolding.** Code "kept for later" outside the module tree, provider stubs full of `todo!()`, and builder methods only tests call are deletions, not investments — git history is the archive. **Two deliberate exceptions, do not flag or delete them:** the `ai` crate (`crates/ai/` — traits + Cohere/mock providers) is the planned home for AI review/embedding and stays despite its `todo!()` bodies; `crates/workspace/src/panels/log_viewer.rs` is the reusable line-coloring viewer that becomes the diagnostics content for `ProblemsPanel`, kept out of `mod.rs` until then. - **Explanation comments.** If you have to write a comment of more than 1 line to justify a decision, that is the wrong decision. Go back and rethink the implimentation from the start in a different manner. ## gpui test modules diff --git a/crates/app/src/actions.rs b/crates/app/src/actions.rs index e3d24a80..bac30b30 100644 --- a/crates/app/src/actions.rs +++ b/crates/app/src/actions.rs @@ -35,9 +35,7 @@ pub fn key_bindings() -> Vec { // Settings. Declared in `app_menus` (it's a menu action first); the handler is already // registered globally in `main.rs`, so this only adds the key. KeyBinding::new("ctrl-,", OpenSettings, None), - // Find in the table. The action is declared in `crate::table` (app → table is one-way, so - // an app-declared action would be invisible to `TablePanel`). Scoped to the `TablePanel` - // context, not global, so it doesn't shadow Ctrl+F inside the cell editor's `Input`. + // `Search` is declared in `crate::table` (app→table is one-way); scoped so it spares the cell editor's Ctrl+F. KeyBinding::new("ctrl-f", table::Search, Some("TablePanel")), ] } diff --git a/crates/app/src/app_settings/mod.rs b/crates/app/src/app_settings/mod.rs index fc0a718d..f118ada2 100644 --- a/crates/app/src/app_settings/mod.rs +++ b/crates/app/src/app_settings/mod.rs @@ -80,9 +80,7 @@ fn columns_page(cx: &App) -> SettingPage { let (get_key, set_key, remove_key) = (key.clone(), key.clone(), key.clone()); page = page.group( SettingGroup::new() - // The column name is the group *title* so it also lists as a sub-entry under - // "Columns" in the sidebar, which only picks up titled groups. That makes it - // text-only, so Remove gets its own right-aligned row directly beneath it. + // Column name is the group title so it lists under "Columns" in the sidebar (titled groups only). .title(name) .item(SettingItem::render(move |_opts, _window, _cx| { let remove_key = remove_key.clone(); diff --git a/crates/app/src/main.rs b/crates/app/src/main.rs index 763a2f5d..67300cf6 100644 --- a/crates/app/src/main.rs +++ b/crates/app/src/main.rs @@ -41,9 +41,7 @@ use gpui_component::dock::DockPlacement; use workspace::Workspace; /// Opens the Settings window, focusing the existing one if it's already open. -// `gpui::App`, spelled out: a bare `App` here would resolve to this file's own -// `pub struct App` below, not the GPUI context type — local items always win -// over glob imports regardless of source order. +// `gpui::App` spelled out: a bare `App` binds to this file's own `struct App`, not the context type. pub(crate) fn open_settings_window(cx: &mut gpui::App) { if WindowRegistry::focus_or_clear(SETTINGS_WINDOW_KIND, cx) { return; @@ -99,9 +97,7 @@ fn set_main_window_title(window: &mut Window, cx: &gpui::App) { pub(crate) fn open_main_window(cx: &mut gpui::App) { if WindowRegistry::focus_or_clear(MAIN_WINDOW_KIND, cx) { - // The window already exists (opening a project just switched `CurrentProject` - // to a different one) — reload its layout instead of leaving the previous - // project's panels in place. + // Window already exists (a project switch); reload its layout instead of keeping the old one's. if let Some(handle) = WindowRegistry::get(MAIN_WINDOW_KIND, cx) { handle .update(cx, |_, window, cx| { @@ -183,10 +179,7 @@ impl App { } }); - // Block the OS close button while a background task is running, and - // otherwise run the same final flush that `on_app_quit` does — the - // native X skips the app-quit path on Windows (zed#40385/#40290), so - // relying on `on_app_quit` alone loses state on OS-native close. + // Native X skips `on_app_quit` on Windows (zed#40385/#40290), so flush here too — unless locked. window.on_window_should_close(cx, |_, cx| { if WindowLock::is_locked(cx) { return false; @@ -324,20 +317,14 @@ fn main() { cx.quit(); }); - // Guaranteed flush before exit (gpui gives `on_app_quit` handlers 100ms). - // Two gaps close here: (1) both writers debounce at 450ms, so a write - // enqueued right before quit would otherwise be lost; (2) dock - // open/close toggles and edge resizes never emit `LayoutChanged`, so - // the final layout may never have been captured at all. + // Flush before exit: writers debounce 450ms, and dock toggles/resizes never emit `LayoutChanged`. cx.on_app_quit(|cx| { flush_all_state(cx); async {} }) .detach(); - // The launcher (Recent Projects + Create New) is the real startup window — it opens - // the main window itself for a recent project, or a project-creation wizard window for - // "Create New". + // The launcher is the real startup window; it opens the main window or the wizard itself. project_wizard::open_launcher_window(cx); }); } diff --git a/crates/app/src/status_items/mod.rs b/crates/app/src/status_items/mod.rs index 4d2d56ff..1a71d5d9 100644 --- a/crates/app/src/status_items/mod.rs +++ b/crates/app/src/status_items/mod.rs @@ -22,7 +22,7 @@ pub fn build_status_bar_registry(cx: &mut App, dock: WeakEntity) -> St IconName::Info, ) }); - registry.add_left(left_panel); + registry.items_mut().add_left(left_panel); // ponytail: diagnostics don't exist yet — count is a placeholder "0". Wire the real // error/warning totals when ProblemsPanel grows content. @@ -35,12 +35,12 @@ pub fn build_status_bar_registry(cx: &mut App, dock: WeakEntity) -> St ) .label("0") }); - registry.add_left(problems); + registry.items_mut().add_left(problems); // Text readout of the table's selected cell. Added to the right *before* the agent star so it // lands leftmost in the right group (text items go right-leftmost, star stays rightmost). let cell_location = cx.new(CellLocation::new); - registry.add_right(cell_location); + registry.items_mut().add_right(cell_location); let agent = cx.new(|_| { DockToggleButton::new( @@ -50,7 +50,7 @@ pub fn build_status_bar_registry(cx: &mut App, dock: WeakEntity) -> St IconName::Star, ) }); - registry.add_right(agent); + registry.items_mut().add_right(agent); registry } diff --git a/crates/app/src/title_items/mod.rs b/crates/app/src/title_items/mod.rs index 18cce3ce..5b8a1c2e 100644 --- a/crates/app/src/title_items/mod.rs +++ b/crates/app/src/title_items/mod.rs @@ -15,7 +15,7 @@ pub fn build_title_bar_registry(cx: &mut App, dock: WeakEntity) -> Tit let mut registry = TitleBarRegistry::new(); let menus = cx.new(|_| TitleMenus); - registry.add_left(menus); + registry.items_mut().add_left(menus); for (id, placement, icon) in [ ("title-panel-left", DockPlacement::Left, IconName::PanelLeft), @@ -31,7 +31,7 @@ pub fn build_title_bar_registry(cx: &mut App, dock: WeakEntity) -> Tit ), ] { let btn = cx.new(|_| DockToggleButton::new(id, dock.clone(), placement, icon)); - registry.add_right(btn); + registry.items_mut().add_right(btn); } registry diff --git a/crates/cloud-sync/src/lib.rs b/crates/cloud-sync/src/lib.rs index fa9ac029..60ad1c7b 100644 --- a/crates/cloud-sync/src/lib.rs +++ b/crates/cloud-sync/src/lib.rs @@ -90,9 +90,7 @@ pub struct SheetData { /// off the UI thread. pub fn fetch_sheet(link: &str) -> Result { let SheetRef { id, gid } = parse_sheet_ref(link)?; - // ponytail: gid is forwarded so a link to a specific tab exports that tab - // when Google honors it; if Google ignores it for xlsx we still fall back - // to the first tab, same as before. Real multi-tab selection needs API v4. + // ponytail: gid forwarded as a hint, falls back to first tab; real multi-tab selection needs API v4. let gid_param = gid.map(|g| format!("&gid={g}")).unwrap_or_default(); let url = format!("https://docs.google.com/spreadsheets/d/{id}/export?format=xlsx{gid_param}"); // Explicit timeout so a stalled connection returns an error instead of diff --git a/crates/project-wizard/src/data.rs b/crates/project-wizard/src/data.rs index b18618cc..249450b7 100644 --- a/crates/project-wizard/src/data.rs +++ b/crates/project-wizard/src/data.rs @@ -24,9 +24,7 @@ impl SpreadsheetPreview { } impl From for SpreadsheetPreview { - // ponytail: notes are dropped here — the wizard preview has no field for - // them yet. Carry `sheet.notes` through when the Problems/column-notes - // panels land (see the ".qrate project file" + "Problems panel" tasks). + // ponytail: `sheet.notes` dropped — no preview field yet; carry them through when the Problems panel lands. fn from(sheet: cloud_sync::SheetData) -> Self { Self { headers: sheet.headers, diff --git a/crates/project-wizard/src/launcher.rs b/crates/project-wizard/src/launcher.rs index 03c61487..39d45bb9 100644 --- a/crates/project-wizard/src/launcher.rs +++ b/crates/project-wizard/src/launcher.rs @@ -179,10 +179,7 @@ impl Render for Launcher { .child( v_flex() .flex_1() - // min_h(0) lets this column be bounded to the row - // height so the inner list — not the column — scrolls, - // keeping the heading pinned. Without it the flex - // item's default min-height:auto grows to fit content. + // min_h(0) overrides the flex default min-height:auto so the inner list scrolls, not the column. .min_h(px(0.)) .min_w(px(0.)) .gap_2() diff --git a/crates/project-wizard/src/steps/link.rs b/crates/project-wizard/src/steps/link.rs index 46360b18..74d7a6f8 100644 --- a/crates/project-wizard/src/steps/link.rs +++ b/crates/project-wizard/src/steps/link.rs @@ -102,9 +102,7 @@ impl ProjectWizard { ) .content( v_flex().gap_2().mt_2().child( - // The pattern input lives *inside* the option card - // (appended to `option_card`'s own v_flex), revealed - // once the card is selected. + // Pattern input lives inside the option card, revealed once it's selected. option_card( "link-pattern", "Use a custom pattern", diff --git a/crates/project-wizard/src/steps/review.rs b/crates/project-wizard/src/steps/review.rs index f1434fbe..ddb0e59e 100644 --- a/crates/project-wizard/src/steps/review.rs +++ b/crates/project-wizard/src/steps/review.rs @@ -23,9 +23,7 @@ impl ProjectWizard { LinkMethod::ExactFilename => "exact filename", LinkMethod::CustomPattern => "custom pattern", }); - // Blank projects go through the Columns step too, so honor any config - // loaded there; otherwise fall back to spreadsheet headers (empty for - // Blank), every column defaulting to Text. + // Prefer config from the Columns step; else fall back to spreadsheet headers, all Text. let columns: Vec = self .config_preview .as_ref() diff --git a/crates/project-wizard/src/wizard.rs b/crates/project-wizard/src/wizard.rs index ee61430c..9ba49051 100644 --- a/crates/project-wizard/src/wizard.rs +++ b/crates/project-wizard/src/wizard.rs @@ -191,9 +191,7 @@ impl ProjectWizard { Ok(()) } WizardStep::Files => match self.entry_kind { - // `skip_files` only waives the *folder* requirement below — it - // means "skip linking a files folder", not "skip checking the - // spreadsheet/sheet exists" (see its label in steps/files.rs). + // `skip_files` waives only the folder requirement, not the spreadsheet/sheet check. EntryKind::Csv => { if self.csv_preview.is_none() { return Err("Choose a valid CSV spreadsheet".into()); @@ -354,9 +352,7 @@ impl ProjectWizard { } else { "Next →" }; - // When the sheet still needs checking, Next doubles as "Check" and is - // live regardless of the blocker. Otherwise a blocker greys out Next and - // its reason is shown beside it. + // While the sheet needs checking, Next acts as "Check" (no blocker); otherwise a blocker greys it out. let blocker = if self.needs_sheet_check(cx) { None } else { @@ -411,9 +407,7 @@ impl Render for ProjectWizard { WizardStep::Review => self.render_review_step(window, cx).into_any_element(), }; - // Shared scaffold for every step: pinned breadcrumb on top, scrolling - // body in the middle, pinned Back/Next footer at the bottom. Each step - // only supplies `body`; navigation and validation live in the footer. + // Shared scaffold: pinned breadcrumb, scrolling body, pinned footer; each step supplies only `body`. let breadcrumb = self.render_breadcrumb(cx).into_any_element(); let footer = self.render_footer(cx).into_any_element(); @@ -425,10 +419,7 @@ impl Render for ProjectWizard { v_flex() .id("wizard-body") .flex_1() - // min_h(0) bounds this to the window height (below the title - // bar) so the scroll region — not the whole page — grows. - // Without it the flex item's default min-height:auto expands - // to fit all steps' content, pushing the footer off-screen. + // min_h(0) overrides flex min-height:auto so the scroll region grows, not the page. .min_h(px(0.)) .p_5() .gap_3() diff --git a/crates/table/src/filter.rs b/crates/table/src/filter.rs index 988e3334..ddf28854 100644 --- a/crates/table/src/filter.rs +++ b/crates/table/src/filter.rs @@ -236,9 +236,7 @@ fn row( // key would leave hover/focus on the wrong row. Checkbox::new(SharedString::from(format!("filter-value-{value}"))) .checked(!is_excluded) - // The value goes in as a *child*, not `.label()`: the label path forces - // `line_height(1.0)` under an `overflow_hidden` parent, which clips descenders — - // underscores in ids vanished entirely. + // Value as a child, not `.label()`: the label's `line_height(1.0)` clips descenders here. .child( div() .w_full() diff --git a/crates/table/src/panel.rs b/crates/table/src/panel.rs index 87bdced5..b787664e 100644 --- a/crates/table/src/panel.rs +++ b/crates/table/src/panel.rs @@ -21,10 +21,7 @@ use crate::{ /// Settings key for the saved column layout (order + widths) in the project's `.qrate` file. const COLUMN_LAYOUT_KEY: &str = "table_columns"; -// Free-text find across the grid. Declared here in `crate::table` (not `crate::app`) because the -// dependency edge is app → table: an action declared in app is invisible to `TablePanel`, which -// must handle it. `crates/app/src/actions.rs` binds Ctrl+F to it, scoped to the `TablePanel` -// context so it doesn't steal the shortcut from the cell editor's own `Input` context. +// Free-text find, declared here (not in `app`) since app→table is one-way; `app` binds Ctrl+F to it. actions!(qrate, [Search]); /// Center panel: the virtualized text table, with a pinned row-number column, native @@ -98,9 +95,7 @@ impl TablePanel { .col_selectable(true) .col_resizable(true) .col_movable(true) - // Our numbered `#` column (row_index.rs) plays row header, so hide the - // library's blank strip. With it hidden, clicking the already-selected cell - // escalates to selecting the whole row. + // Our `#` column is the row header, so hide the library's blank strip. .row_header(false) }); // Publish the handle so cross-crate readers (status bar, Details panel) can reach the @@ -125,9 +120,7 @@ impl TablePanel { |_this, state, event: &TableEvent, window, cx| { match event { TableEvent::SelectCell(row, col) if *col == row_index::COL_IX => { - // Native cell selection ignores per-column `selectable(false)`, so a - // click (or arrow-left) can land on the pinned `#` column — bounce to - // the first data cell. The nested SelectCell updates the cursor. + // Native selection ignores `selectable(false)`, so bounce a hit on the `#` column to col 1. let (row, cols) = (*row, state.read(cx).delegate().columns_count(cx)); if cols > 1 { state.update(cx, |s, cx| s.set_selected_cell(row, 1, cx)); @@ -186,9 +179,7 @@ impl TablePanel { cx.observe_global::(|this: &mut Self, cx| { let project = cx.global::(); let file = project.file.clone(); - // Same project, so this fired for a project-scoped *setting* write. Re-apply the - // per-column settings and repaint — reloading the data here would throw away the - // user's filters, edit state and selection on every settings toggle. + // A project-scoped setting write: re-apply per-column settings only, don't reload (loses state). if this.loaded_project.as_ref() == Some(&file) { let column_settings = settings::columns::load(cx); this.state.update(cx, |state, cx| { @@ -238,9 +229,7 @@ impl TablePanel { }, ); - // The column-filter dropdown's "search values" box lives on the delegate; repaint when it - // changes so the open dropdown re-narrows its checklist. Narrowing can leave the scroll - // offset past the end of the shorter list, showing a blank box — so jump back to the top. + // Repaint on filter-search change so the dropdown re-narrows; reset scroll to top to avoid a blank box. let _filter_search_sub = cx.subscribe(&filter_search, |this, _input, _event: &InputEvent, cx| { this.state @@ -296,9 +285,7 @@ impl TablePanel { /// Step `delta` matches forward (+1) or back (-1), wrapping, and scroll/select the landing /// cell. No-op with no matches. fn goto_match(&mut self, delta: isize, cx: &mut Context) { - // Re-scan first: matches are `(view_row, col)`, and a column filter changed since the last - // keystroke would have shifted every view index under us — stepping a stale list lands on - // whatever row now happens to sit at that position. + // Re-scan first: matches are view indices, and a filter change since last keystroke invalidates them. let needle = self.search_input.read(cx).value().to_string(); self.search_matches = self .state @@ -321,8 +308,7 @@ impl TablePanel { let Some(&(view_row, data_col)) = self.search_matches.get(self.search_ix) else { return; }; - // `set_selected_cell` scrolls the row to center itself, so no separate `scroll_to_row` — - // two scrolls per match fight each other. + // `set_selected_cell` already centre-scrolls the row, so no separate `scroll_to_row`. // +1 for the pinned row-index column. self.state.update(cx, |state, cx| { state.set_selected_cell(view_row, data_col + 1, cx) @@ -425,9 +411,7 @@ impl Panel for TablePanel { /// `.xsmall().ghost()` by the library. `title_suffix` is the other option, but it sits by the /// title instead — this is the hook for buttons that belong *beside* the ⋯. fn toolbar_buttons(&mut self, _w: &mut Window, cx: &mut Context) -> Option> { - // Toggle the find bar directly through a weak handle to this panel — more robust than - // dispatching the `Search` action, which would only reach us if focus happened to sit - // inside the `TablePanel` context when the button is clicked. + // Toggle via a weak handle, not the `Search` action, which only fires when focus is inside `TablePanel`. let this = cx.entity().downgrade(); Some(vec![ Button::new("table-search") @@ -463,10 +447,7 @@ impl Render for TablePanel { v_flex() .size_full() - // `TablePanel` key context so Ctrl+F (bound in `crates/app`) reaches the find toggle - // without stealing the shortcut from the cell editor's own `Input` context. Tracking - // the focus handle keeps this node in the focus dispatch path even when no cell holds - // focus, so the binding fires. + // `TablePanel` context + tracked focus so Ctrl+F reaches the toggle even when no cell holds focus. .key_context("TablePanel") .track_focus(&self.focus_handle) .on_action(cx.listener(|this, _: &Search, window, cx| this.toggle_search(window, cx))) @@ -474,10 +455,7 @@ impl Render for TablePanel { .on_action(cx.listener(|this, _: &Escape, window, cx| this.dismiss_search(window, cx))) .p_2() .gap_2() - // The find bar lives in this panel's own render, not in `title_suffix`: that is drawn - // by the parent `TabPanel`, which never observes its child panels, so toggling - // `search_open` here would notify only us and the bar would sit invisible until some - // unrelated interaction happened to redraw the tab bar. + // Find bar renders here, not in `title_suffix`: the parent `TabPanel` never observes us to redraw it. .when(self.search_open, |this| { let opts = self.search_opts; // A compact toggle button (case / word / regex), highlighted while active. diff --git a/crates/window-wrapper/src/bar.rs b/crates/window-wrapper/src/bar.rs index b91d59ea..9c81dbea 100644 --- a/crates/window-wrapper/src/bar.rs +++ b/crates/window-wrapper/src/bar.rs @@ -22,21 +22,8 @@ impl BarItems { /// A bar's global registry. The data lives in [`BarItems`]; implementors exist only to /// give each bar a distinct type (GPUI globals are keyed by type, so title and status -/// bars can't share one registry struct). `add_left`/`add_right` forward to the items. +/// bars can't share one registry struct). Push items via `items_mut().add_left(view)`. pub trait BarRegistry: Global { fn items(&self) -> &BarItems; fn items_mut(&mut self) -> &mut BarItems; - - fn add_left(&mut self, view: impl Into) - where - Self: Sized, - { - self.items_mut().add_left(view); - } - fn add_right(&mut self, view: impl Into) - where - Self: Sized, - { - self.items_mut().add_right(view); - } } diff --git a/crates/workspace/src/image_viewer.rs b/crates/workspace/src/image_viewer.rs new file mode 100644 index 00000000..34bc7ef1 --- /dev/null +++ b/crates/workspace/src/image_viewer.rs @@ -0,0 +1,204 @@ +//! A full-screen-ish image viewer overlay: the photo fit to the content area, then zoomable and +//! pannable, with the filename and zoom/close controls. +//! +//! It is *not* a `gpui_component` dialog: a dialog's layer occludes the whole window, so it would +//! cover the custom title bar and steal its window controls. Instead the open viewer is held in +//! the [`ActiveImageViewer`] global and mounted by `Workspace` as an overlay over its own content +//! — which already sits below the title bar — so close/minimize stay reachable and the overlay +//! sizes itself to whatever area the workspace has rather than to fixed viewport fractions. + +use std::path::PathBuf; + +use gpui::*; +use gpui_component::{ + ActiveTheme, IconName, Sizable, + button::{Button, ButtonVariants}, +}; + +/// The currently-open viewer, if any. `Workspace` observes this and mounts/unmounts the overlay. +#[derive(Default)] +pub struct ActiveImageViewer(pub Option>); + +impl Global for ActiveImageViewer {} + +/// Opens `path` in the shared viewer overlay, replacing any viewer already open. +pub fn open_image_viewer(path: PathBuf, cx: &mut App) { + let viewer = cx.new(|cx| ImageViewer { + path, + zoom: 1.0, + offset: Point::default(), + drag_from: None, + focus_handle: cx.focus_handle(), + focused: false, + }); + cx.set_global(ActiveImageViewer(Some(viewer))); +} + +fn close(cx: &mut App) { + cx.set_global(ActiveImageViewer(None)); +} + +pub struct ImageViewer { + path: PathBuf, + /// 1.0 = fit-to-area (`Contain`); [`ImageViewer::set_zoom`] clamps it. + zoom: f32, + /// Pan translation from the centered position. + offset: Point, + /// Last pointer position while dragging; `None` when not panning. + drag_from: Option>, + focus_handle: FocusHandle, + /// Grabs focus on first render so Escape reaches [`Self`]; set once so we don't re-focus. + focused: bool, +} + +impl ImageViewer { + /// Clamp zoom to [0.1, 8] — below 1 zooms out past the initial fit — 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) { + self.zoom = zoom.clamp(0.1, 8.0); + if self.zoom <= 1.0 { + self.offset = Point::default(); + } + } +} + +impl Render for ImageViewer { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + if !self.focused { + window.focus(&self.focus_handle, cx); + self.focused = true; + } + let (zoom, offset) = (self.zoom, self.offset); + let name: SharedString = self + .path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("Image") + .to_string() + .into(); + let pill = cx.theme().background.opacity(0.8); + + div() + .track_focus(&self.focus_handle) + .id("image-viewer") + // Fills the workspace overlay slot; `absolute` so it stacks over the dock content. + .absolute() + .size_full() + .overflow_hidden() + // Register an opaque hitbox over the whole overlay so clicks and scrolls land here + // instead of falling through to the table painted behind it. + .occlude() + // Dim what's behind so the photo reads as the focus. + .bg(cx.theme().background.opacity(0.9)) + .on_key_down(cx.listener(|_, ev: &KeyDownEvent, _, cx| { + if ev.keystroke.key == "escape" { + close(cx); + } + })) + // Any scroll zooms — trackpad pixel deltas, wheel line deltas, and ctrl+scroll alike; + // drag is what pans, so scroll is free to mean zoom. + .on_scroll_wheel(cx.listener(|this, ev: &ScrollWheelEvent, _, cx| { + let step = match ev.delta { + ScrollDelta::Lines(d) => d.y * 0.1, + ScrollDelta::Pixels(d) => f32::from(d.y) * 0.005, + }; + this.set_zoom(this.zoom * (1.0 + step)); + cx.notify(); + })) + .on_mouse_down( + MouseButton::Left, + cx.listener(|this, ev: &MouseDownEvent, _, cx| { + this.drag_from = Some(ev.position); + cx.notify(); + }), + ) + .on_mouse_move(cx.listener(|this, ev: &MouseMoveEvent, _, cx| { + if let Some(last) = this.drag_from { + this.offset.x += ev.position.x - last.x; + this.offset.y += ev.position.y - last.y; + this.drag_from = Some(ev.position); + cx.notify(); + } + })) + .on_mouse_up( + MouseButton::Left, + cx.listener(|this, _: &MouseUpEvent, _, cx| { + this.drag_from = None; + cx.notify(); + }), + ) + .child( + // Padding is the only sizing input: the image `Contain`-fits the padded content + // area at zoom 1, so it scales with the window/dock instead of a fixed fraction. + div() + .size_full() + .flex() + .items_center() + .justify_center() + .p_8() + .child( + // `flex_shrink_0` keeps `relative(zoom)` past 1; `relative` + `left`/`top` pan it. + img(self.path.clone()) + .flex_shrink_0() + .relative() + .w(relative(zoom)) + .h(relative(zoom)) + .left(offset.x) + .top(offset.y) + .object_fit(ObjectFit::Contain), + ), + ) + .child( + div() + .absolute() + .top_4() + .left_4() + .px_2() + .py_1() + .rounded(cx.theme().radius) + .bg(pill) + .child(name), + ) + .child( + div() + .absolute() + .top_4() + .right_4() + .flex() + .gap_1() + .p_1() + .rounded(cx.theme().radius) + .bg(pill) + .child( + Button::new("zoom-out") + .icon(IconName::Minus) + .ghost() + .small() + .tooltip("Zoom out") + .on_click(cx.listener(|this, _, _, cx| { + this.set_zoom(this.zoom / 1.25); + cx.notify(); + })), + ) + .child( + Button::new("zoom-in") + .icon(IconName::Plus) + .ghost() + .small() + .tooltip("Zoom in") + .on_click(cx.listener(|this, _, _, cx| { + this.set_zoom(this.zoom * 1.25); + cx.notify(); + })), + ) + .child( + Button::new("close-viewer") + .icon(IconName::Close) + .ghost() + .small() + .tooltip("Close") + .on_click(cx.listener(|_, _, _, cx| close(cx))), + ), + ) + } +} diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index f81b19a9..45d556af 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -2,7 +2,10 @@ //! (center table, left details, right agent, bottom problems) with layout persistence. mod dock_button; +mod image_viewer; mod panels; + +pub use image_viewer::open_image_viewer; // `panels/log_viewer.rs` is intentionally NOT declared here — it is a set-aside, // reusable line-coloring viewer kept for when `ProblemsPanel` grows real content. @@ -54,6 +57,8 @@ pub struct Workspace { dock_area: Entity, /// Persists the layout to settings whenever the dock emits `LayoutChanged`. _layout_sub: Subscription, + /// Re-renders to mount/unmount the image viewer overlay when it opens or closes. + _viewer_sub: Subscription, } impl Workspace { @@ -75,10 +80,7 @@ impl Workspace { let dock_area = cx.new(|cx| DockArea::new("qrate-main", Some(DOCK_LAYOUT_VERSION), window, cx)); - // Restore the saved arrangement if there is one; only build the default layout when - // there isn't. Building the default first and then `load`ing over it would construct a - // throwaway *second* table, leaving cross-crate readers (Details panel, status bar) - // bound to the orphaned first one — which never sees selection events. + // Restore if saved, else build default; building first then loading would orphan a throwaway table. if !Self::restore_layout(&dock_area, window, cx) { let weak = dock_area.downgrade(); // Default static layout: center table, left details, right agent, bottom problems. @@ -88,11 +90,7 @@ impl Workspace { let problems = cx.new(|cx| ProblemsPanel::new(window, cx)); dock_area.update(cx, |area, cx| { - // `DockItem::panel` embeds the table directly, with no `TabPanel` and so no - // title bar. Note this only holds until a layout is *saved*: `DockAreaState` - // round-trips a bare panel back as `DockItem::tabs` (state.rs's - // `PanelInfo::Panel` arm), so every run after the first restores it wrapped — - // title bar, ⋯ menu and all. `TablePanel::toolbar_buttons` renders there. + // `DockItem::panel` embeds the table bare (no title bar); only until saved, then it restores wrapped. area.set_center(DockItem::panel(Arc::new(table.clone())), window, cx); area.set_left_dock( DockItem::tab(details, &weak, window, cx), @@ -122,12 +120,7 @@ impl Workspace { // toggle arrows. Done in both paths — `load` doesn't carry this runtime-only flag. dock_area.update(cx, |area, cx| area.set_toggle_button_visible(false, cx)); - // Persist on every layout change. Set up last so the default/restore building above - // doesn't trigger a redundant write. With a project open the layout is saved into its - // `.qrate` file (per-project panel state); otherwise into the global app settings. - // NOTE: gpui_component only emits `LayoutChanged` for *inner* panel changes (tab - // drags, split resizes) — a dock open/close or edge resize just `cx.notify()`s, so - // `toggle_dock` below and the app-quit flush persist those explicitly. + // Persist on `LayoutChanged` (inner changes only); `toggle_dock` and the app-quit flush cover the rest. let _layout_sub = cx.subscribe(&dock_area, |_this, area, event: &DockEvent, cx| { if matches!(event, DockEvent::LayoutChanged) { Self::persist_layout(&area, cx); @@ -136,9 +129,13 @@ impl Workspace { } }); + let _viewer_sub = + cx.observe_global::(|_this, cx| cx.notify()); + Self { dock_area, _layout_sub, + _viewer_sub, } } @@ -157,9 +154,7 @@ impl Workspace { self.dock_area.update(cx, |area, cx| { area.toggle_dock(placement, window, cx); }); - // `Dock::set_open` never emits `LayoutChanged` (it only notifies), so the - // subscription in `new` can't see toggles — persist here or open/closed - // state is lost on reopen. + // `Dock::set_open` only notifies, never emits `LayoutChanged`, so persist the toggle here. Self::persist_layout(&self.dock_area, cx); cx.notify(); } @@ -240,9 +235,7 @@ impl Workspace { impl Render for Workspace { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { - // When the bottom dock is closed, stretch the dock area 29px past the bottom edge and - // clip it, so the library's residual "closed bottom" strip is cropped out of view. - // When it's open, sit flush (bottom: 0) so no real content is lost. + // Bottom dock closed: overhang 29px and clip to hide the library's residual strip; open: sit flush. let bottom_open = self .dock_area .read(cx) @@ -260,14 +253,24 @@ impl Render for Workspace { cx.set_global(crop); } - div().size_full().relative().overflow_hidden().child( - div() - .absolute() - .top_0() - .left_0() - .right_0() - .bottom(overshoot) - .child(self.dock_area.clone()), - ) + // A sibling overlay, not a dialog: it covers the dock but leaves the title bar's controls reachable. + let viewer = cx + .try_global::() + .and_then(|a| a.0.clone()); + + div() + .size_full() + .relative() + .overflow_hidden() + .child( + div() + .absolute() + .top_0() + .left_0() + .right_0() + .bottom(overshoot) + .child(self.dock_area.clone()), + ) + .children(viewer) } } diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 2f916872..684545bb 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -172,6 +172,9 @@ fn render_image_frame(image_path: Option, cx: &App) -> AnyElement { div() .relative() .size_full() + // `min_w_0`/`min_h_0` drop the min-content floor so the frame tracks the pane and the image letterboxes. + .min_w_0() + .min_h_0() .rounded(cx.theme().radius) .border_1() .border_color(cx.theme().border) @@ -181,11 +184,7 @@ fn render_image_frame(image_path: Option, cx: &App) -> AnyElement { Some(path) => frame .map(|frame| { if show_image { - // `rounded` goes on the `img` itself, not just the frame: gpui's - // overflow mask is a plain rect (`Style::overflow_mask`), so - // `overflow_hidden` above clips square and a full-bleed photo would - // paint over the frame's rounded corners. `Img` reads its *own* - // corner radii when painting, which is what actually rounds it. + // `rounded` on the `img` itself: gpui's overflow mask is a rect, so `Img` must round itself. frame.child( img(path.clone()) .size_full() @@ -205,6 +204,21 @@ fn render_image_frame(image_path: Option, cx: &App) -> AnyElement { .flex() .gap_1() .bg(cx.theme().background.opacity(0.7)) + // Fullscreen only makes sense for something we can actually render; an + // icon-placeholder file has nothing to zoom into. + .when(show_image, |group| { + let path = path.clone(); + group.child( + Button::new("fullscreen-image") + .icon(IconName::Maximize) + .ghost() + .small() + .tooltip("View fullscreen") + .on_click(move |_, _, cx| { + crate::open_image_viewer(path.clone(), cx) + }), + ) + }) .child(action( "open-image", IconName::ExternalLink, @@ -248,17 +262,14 @@ impl Render for DetailsPanel { .into_any_element(); }; - // A bordered two-column grid, not `DescriptionList` — the label/value list read as - // free-floating text, and the fields *are* tabular. Hand-built rather than a second - // `DataTable`: that would mean another `TableDelegate` + `TableState` entity to render - // what is a fixed 2-column, no-sort, no-resize, no-header view of data the center - // table's delegate already hands over as pairs (`QrateTableDelegate::row_fields`). + // Hand-built 2-column grid, not `DescriptionList`/`DataTable`: the fields are fixed, tabular pairs. // ponytail: revisit if these fields ever need sorting or inline editing. let border = cx.theme().border; let rows = fields.into_iter().enumerate().map(|(ix, (k, v))| { div() .flex() - .items_start() + // `items_stretch` so the label cell's `border_r` divider spans the full (wrapped) row height. + .items_stretch() .border_b_1() .border_color(border) .when(ix % 2 == 1, |r| r.bg(cx.theme().muted.opacity(0.4))) @@ -273,15 +284,28 @@ impl Render for DetailsPanel { .text_color(cx.theme().muted_foreground) .child(k), ) - .child(div().flex_1().px_2().py_1p5().child(v)) + // `min_w_0` overrides flex `min-width: auto` so the value wraps instead of overflowing right. + // Click-to-copy rather than drag-select: `TextView` parses markdown/html and mangles raw metadata. + .child( + div() + .id(ix) + .flex_1() + .min_w_0() + .px_2() + .py_1p5() + .cursor_pointer() + .on_click({ + let value = v.clone(); + move |_, _, cx| { + cx.write_to_clipboard(ClipboardItem::new_string(value.to_string())) + } + }) + .child(v), + ) }); - // Split, not one scrolling column: the image lives in its own panel so it stays put - // while only the fields scroll (the old single `overflow_y_scrollbar` div scrolled the - // photo away), and the drag handle between them lets the user trade image height for - // field rows instead of the photo distorting as the dock is resized. - // `.size()` is the *initial* size only — once the user drags, `ResizableState` owns it, - // so re-reading the persisted value each render is a restore, not a fight. + // Split so the image stays put while only the fields scroll, with a drag handle to trade heights. + // `.size()` is the initial size only — once dragged, `ResizableState` owns it, so re-reading restores. let image_height = cx .try_global::() .and_then(|p| p.data.values.get(IMAGE_PANE_HEIGHT_KEY)) @@ -308,21 +332,15 @@ impl Render for DetailsPanel { .child(render_image_frame(image_path, cx)), ) .child( - resizable_panel().child( + // `pr_2` on the panel insets the scrollbar from the resize edge so dragging it doesn't catch. + resizable_panel().pr_2().child( div() .size_full() - // Scrolls the fields alone. `min_h_0` is load-bearing: a flex child's - // default `min-height: auto` refuses to shrink below its content, so - // without it this box grows past the panel and the last rows fall off - // the bottom with no scrollbar to reach them — the reported bug. + // `min_h_0` overrides flex `min-height: auto` so this scrolls instead of growing past the panel. .min_h_0() .overflow_y_scrollbar() .px_3() - // Normal padding *plus* whatever the workspace's bottom-strip crop is - // clipping off this dock right now (29px while the bottom dock is - // closed, 0 while it's open) — otherwise the crop eats the last field - // row. Padding the scroll content, not the panel, so the extra space - // is scrollable-to rather than a dead gap. + // Pad by the bottom-strip crop (29px closed / 0 open) so it doesn't eat the last field row. .pb(px(12.) + crop) .child( div() @@ -340,9 +358,7 @@ impl Render for DetailsPanel { #[cfg(test)] mod tests { - // No `use super::*` here: it would chain-glob `gpui::*`, whose `test` proc-macro shadows - // the built-in `#[test]` that `#[gpui::test]`'s expansion emits — making the macro expand - // into itself until rustc's recursion limit (and then its stack) blows. + // No `use super::*`: chain-globbing `gpui::*` shadows the built-in `#[test]` and recurses (see CLAUDE.md). use std::path::{Path, PathBuf}; use gpui::{Context, IntoElement, Render, TestAppContext, Window};