diff --git a/crates/tty7-cli/src/output.rs b/crates/tty7-cli/src/output.rs index 069021fb..9ee0e3a3 100644 --- a/crates/tty7-cli/src/output.rs +++ b/crates/tty7-cli/src/output.rs @@ -506,6 +506,7 @@ mod tests { ports: vec![PortEntry { port: 3000, pid: 200, + addr: "*".into(), name: "node".into(), }], }; diff --git a/crates/tty7-core/src/daemon/procinfo.rs b/crates/tty7-core/src/daemon/procinfo.rs index 9e3356c7..ddd9c81b 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -231,15 +231,29 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { match tag { "p" => current = rest.parse().unwrap_or(0), "n" => { - let Some(port) = parse_listen_port(rest) else { + let Some((addr, port)) = parse_listen_addr(rest) else { continue; }; - if ports.iter().any(|e| e.port == port && e.pid == current) { + // One process listening on the same port over IPv4 and IPv6 is + // one port to show. Which of the two lines survives used to be + // whichever lsof printed first; now that the address is carried + // through to a clickable URL, the reachable one wins — a + // process bound to both `192.168.1.5` and `*` is on localhost, + // and the row should say so. + if let Some(seen) = ports + .iter_mut() + .find(|e| e.port == port && e.pid == current) + { + if !PortEntry::reaches_loopback(&seen.addr) && PortEntry::reaches_loopback(addr) + { + seen.addr = addr.to_string(); + } continue; } ports.push(PortEntry { port, pid: current, + addr: addr.to_string(), name: by_pid .get(¤t) .copied() @@ -259,10 +273,19 @@ fn listening_ports(_procs: &[ProcEntry]) -> Vec { Vec::new() } -fn parse_listen_port(name: &str) -> Option { +/// The address and port `lsof -Fn` reports a listener on — `*:3000`, +/// `127.0.0.1:8080`, `[::1]:5173`. +/// +/// The address used to be dropped on the floor, which was harmless while the +/// port was a number to read. It stopped being harmless when the panel started +/// handing the port over as an address to open: a server bound only to +/// `172.17.0.1` or a LAN address is not on `localhost`, and offering it as one +/// sends the browser to a refused connection or, worse, to whatever else holds +/// that port on loopback. +fn parse_listen_addr(name: &str) -> Option<(&str, u16)> { let name = name.split_whitespace().next()?; - let (_, port) = name.rsplit_once(':')?; - port.parse().ok() + let (addr, port) = name.rsplit_once(':')?; + Some((addr, port.parse().ok()?)) } #[cfg(test)] @@ -325,10 +348,35 @@ mod tests { #[test] fn parses_lsof_listen_addresses() { - assert_eq!(parse_listen_port("*:3000"), Some(3000)); - assert_eq!(parse_listen_port("127.0.0.1:8080"), Some(8080)); - assert_eq!(parse_listen_port("[::1]:5173"), Some(5173)); - assert_eq!(parse_listen_port("*:5432 (LISTEN)"), Some(5432)); - assert_eq!(parse_listen_port("/tmp/some.sock"), None); + assert_eq!(parse_listen_addr("*:3000"), Some(("*", 3000))); + assert_eq!( + parse_listen_addr("127.0.0.1:8080"), + Some(("127.0.0.1", 8080)) + ); + assert_eq!(parse_listen_addr("[::1]:5173"), Some(("[::1]", 5173))); + assert_eq!(parse_listen_addr("*:5432 (LISTEN)"), Some(("*", 5432))); + assert_eq!(parse_listen_addr("/tmp/some.sock"), None); + } + + #[test] + fn an_address_only_becomes_localhost_when_localhost_reaches_it() { + // What the panel copies and opens. A wildcard or a loopback bind is + // spelled the way anyone would type it; an interface-specific bind is + // kept, because `localhost` is not that server. + let entry = |addr: &str| PortEntry { + port: 8080, + pid: 1, + addr: addr.into(), + name: "server".into(), + }; + for addr in ["", "*", "0.0.0.0", "::", "[::]", "127.0.0.1", "[::1]"] { + assert_eq!( + entry(addr).authority(), + "localhost:8080", + "{addr} is reachable on loopback" + ); + } + assert_eq!(entry("172.17.0.1").authority(), "172.17.0.1:8080"); + assert_eq!(entry("192.168.1.20").authority(), "192.168.1.20:8080"); } } diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index 547b6615..8c71f170 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -393,6 +393,39 @@ pub struct PortEntry { pub port: u16, pub pid: u32, pub name: String, + /// The address the socket is bound to, as `lsof` spells it — `*`, + /// `0.0.0.0`, `127.0.0.1`, `[::1]`, or a specific interface. + /// + /// `serde(default)` because a daemon from before this field existed + /// answers `QueryProcs` without it, and an empty address is read the same + /// way that daemon's callers read every address: as localhost. + #[serde(default)] + pub addr: String, +} + +impl PortEntry { + /// Whether a bound address can be reached on this machine's loopback — + /// true for the wildcards and the loopback addresses themselves, false for + /// a socket pinned to one specific non-loopback interface. + pub fn reaches_loopback(addr: &str) -> bool { + matches!( + addr, + "" | "*" | "0.0.0.0" | "::" | "[::]" | "127.0.0.1" | "::1" | "[::1]" | "localhost" + ) + } + + /// What to copy, and what to open in a browser: `host:port`. + /// + /// Loopback and wildcard binds are spelled `localhost`, which is what + /// anyone typing the address by hand would write. A socket bound to one + /// specific interface keeps that interface — the panel presents this as + /// the address of the pane's server, and `localhost` would not be it. + pub fn authority(&self) -> String { + match Self::reaches_loopback(&self.addr) { + true => format!("localhost:{}", self.port), + false => format!("{}:{}", self.addr, self.port), + } + } } #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] diff --git a/docs/window/side-panel.mdx b/docs/window/side-panel.mdx index d2f4258c..71277d56 100644 --- a/docs/window/side-panel.mdx +++ b/docs/window/side-panel.mdx @@ -6,6 +6,9 @@ description: "Info, Source Control, and Files — plus the built-in editor." ⌘ J opens a panel on the right of the window with three tabs. It is hidden by default; whichever tab you leave it on is where it opens next time. +The three icons at the top switch tabs, and clicking the lit one puts the panel +away again. + The tty7 side panel @@ -20,10 +23,16 @@ Everything tty7 knows about the focused pane, in one column: | **Processes** | the process tree inside the pane, with the foreground process marked | | **Ports** | every port those processes are listening on | -On a local pane, the working directory row has **Reveal in Finder** / **Open -Folder** beside it. +Hovering a row shows what it can do at the right-hand end of it: **Copy** on the +working directory, the SSH host and the branch, and **Reveal in Finder** / **Open +Folder** alongside it on a working directory this machine can see. The `+N −M` +counts open the [diff overlay](/git/diffs), the same click the sidebar's counts +answer to — and the same **Settings → Window & Tabs → Open diff preview from +sidebar counts** turns both of them back into plain text. + The ports section is the quickest answer to "what is this pane serving, and -where" — the same data `tty7 procs` prints. +where" — the same data `tty7 procs` prints. A port row copies `localhost:PORT`, +and on a local pane opens it in your browser. ## Source Control @@ -36,7 +45,8 @@ commit, and push without leaving the window. ## Files A file tree rooted at the pane's working directory, with git status decorations -on every row and a search box at the top. +on every row and a search box at the top. The search box carries a clear button +while there is something in it. - **Click a file** to open it in the built-in editor. - **Drag a file out** to Finder or Explorer to copy it there. diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 56b6957c..80c57a7b 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -962,6 +962,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::PanelAgentDone => "done", L10nKey::PanelRevealInFinder => "Reveal in Finder", L10nKey::PanelOpenFolder => "Open Folder", + L10nKey::PanelOpenInBrowser => "Open in Browser", L10nKey::ScmGroupMerge => "Merge Changes", L10nKey::ScmGroupStaged => "Staged Changes", L10nKey::ScmGroupChanges => "Changes", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 8cd0ed54..588f6a55 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1013,6 +1013,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::PanelAgentDone => "完了", L10nKey::PanelRevealInFinder => "Finder で表示", L10nKey::PanelOpenFolder => "フォルダを開く", + L10nKey::PanelOpenInBrowser => "ブラウザで開く", L10nKey::ScmGroupMerge => "マージの競合", L10nKey::ScmGroupStaged => "ステージされた変更", L10nKey::ScmGroupChanges => "変更", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index d85c83ff..a3a9b573 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -738,6 +738,7 @@ l10n_keys! { PanelAgentDone, PanelRevealInFinder, PanelOpenFolder, + PanelOpenInBrowser, ScmGroupMerge, ScmGroupStaged, ScmGroupChanges, @@ -1613,6 +1614,7 @@ mod tests { L10nKey::PanelAgentDone, L10nKey::PanelRevealInFinder, L10nKey::PanelOpenFolder, + L10nKey::PanelOpenInBrowser, L10nKey::WindowStop, L10nKey::WindowDelete, L10nKey::WindowThisWorkspace, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 06be13b9..7595bd51 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -913,6 +913,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::PanelAgentDone => "已完成", L10nKey::PanelRevealInFinder => "在 Finder 中显示", L10nKey::PanelOpenFolder => "打开文件夹", + L10nKey::PanelOpenInBrowser => "在浏览器中打开", L10nKey::ScmGroupMerge => "合并冲突", L10nKey::ScmGroupStaged => "暂存的更改", L10nKey::ScmGroupChanges => "更改", diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index 7c55158b..eac1302d 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -9,7 +9,7 @@ use std::path::PathBuf; use crate::core::config::{Config, RightPanelTab}; use crate::daemon::protocol::PaneProcs; use crate::ui::app::{ - CONTENT_INSET, TILE_GLYPH_SM, TILE_SIZE_SM, Tty7App, tile_trailing_inset, + CONTENT_INSET, TILE_GLYPH_XS, TILE_SIZE_XS, Tty7App, tile_trailing_inset, tile_trailing_inset_sm, }; use crate::ui::i18n::{L10nKey, t}; @@ -58,6 +58,54 @@ const HEADING: f32 = 11. * STEP; // superseded by the interface font scale's rems tokens; the Source Control // panel still names its own px steps locally until it moves onto that scale.) +/// Rows are laid out inside this inset and then pad themselves back out, so a +/// hovered row's background is wider than its text on both sides. +/// +/// The text lands on `CONTENT_INSET` whatever this is — a list subtracts it +/// outside the row and the row adds it back inside — so all this number sets +/// is how far the hover fill bleeds past the text. It lives here rather than +/// in one tab because every tab of this panel is the same list of rows seen +/// from a different angle, and a fill that bleeds 4px under Source Control and +/// 6px under Info is a panel whose rows visibly do not belong to each other. +pub(crate) const ROW_INSET: f32 = 4.; + +/// The strip the row and group action buttons live in, revealed by hovering +/// `row`. +/// +/// Absolutely positioned and opaque, so it covers the tail of the row's text +/// rather than pushing it aside: hovering a row must not move a single pixel +/// of it, or the list crawls under the pointer. +/// +/// It stops the mouse-down by hand instead of calling `occlude()`, which is +/// the obvious way to keep a click off the row underneath and was what made +/// the buttons vanish the moment the pointer reached them. `occlude()` is a +/// *hitbox* behaviour, and gpui inserts hitboxes in prepaint, which never +/// looks at `visibility` — so the strip blocked the mouse even while it was +/// invisible. Blocking cuts the hit test short at the blocking hitbox, and the +/// row's hitbox is behind this one because a parent prepaints before its +/// children; `group_hover` is nothing more than "is the group's hitbox +/// hovered", so the row stopped counting as hovered and the strip hid itself +/// — background, buttons and all — with the pointer sitting right on it. The +/// buttons' own hitboxes came from prepaint and outlived the paint, so they +/// went on answering tooltips for glyphs that were no longer drawn. +/// +/// Stopping propagation buys the same "this click is ours, not the row's" +/// without lying to the hit test: children register their handlers after this +/// one and gpui bubbles back to front, so a button still gets its click first. +pub(crate) fn action_strip(row: &gpui::SharedString, backing: u32) -> gpui::Div { + h_flex() + .absolute() + .right(px(ROW_INSET)) + .top_0() + .bottom_0() + .items_center() + .gap(px(1.)) + .bg(gpui::rgb(backing)) + .invisible() + .group_hover(row.clone(), |s| s.visible()) + .on_any_mouse_down(|_, _, cx| cx.stop_propagation()) +} + /// Height of the search strip. /// /// gpui-component sizes an `Input` border-box, and `.xsmall()` is @@ -84,6 +132,78 @@ pub(crate) struct RightPanelState { const PROCS_POLL: std::time::Duration = std::time::Duration::from_millis(2000); +/// What a session row draws in its value column. +/// +/// Every row used to be a `(&str, String)` pair rendered identically, and the +/// column paid for it twice: `changes` came out as an inert mono `+0 −0` — +/// the same fact the sidebar draws in green and red and opens the diff overlay +/// from — and the agent's state came out as a word where the sidebar has a +/// coloured dot. A row carries its own shape now, so one pane's facts read the +/// same whichever surface is showing them. +enum InfoValue { + /// Mono text, truncated from the tail. + Text(String), + /// A filesystem path, shrunk from the head so the leaf survives. + Path(String), + /// `+N −M` in the sidebar's two colours, and a click into the diff + /// overlay when the setting that governs the sidebar's counts allows it. + Diff { + added: u32, + removed: u32, + open: Option<(crate::ui::host_ops::HostId, PathBuf)>, + }, + /// An agent and what it is doing, behind the status dot the sidebar draws + /// on the tab — `hollow` for Waiting, which is a different *shape* rather + /// than one more hue, for the same reason the tab's dot is. + Agent { + text: String, + dot: Option, + hollow: bool, + }, +} + +/// One label/value line of the Session section. +struct InfoRow { + label: &'static str, + value: InfoValue, + /// What this row's copy tile puts on the clipboard, where copying it is + /// plausibly what someone wants — a path, a host, a branch. `None` on the + /// rows where it is not ("zsh"), because a hover affordance that appears + /// on every row teaches nothing about which rows can do something. + copy: Option, + /// Set on the working-directory row when the path is on the machine the + /// file manager can see, which is the only case Reveal means anything in. + reveal: Option, +} + +impl InfoRow { + fn text(label: &'static str, value: String) -> Self { + Self { + label, + value: InfoValue::Text(value), + copy: None, + reveal: None, + } + } + + fn copyable(mut self) -> Self { + self.copy = match &self.value { + InfoValue::Text(v) | InfoValue::Path(v) => Some(v.clone()), + _ => None, + }; + self + } + + /// Whether the row does anything if you click or hover it. It is what + /// decides the hover fill, so the fill never promises an action the row + /// does not have. + fn interactive(&self) -> bool { + self.copy.is_some() + || self.reveal.is_some() + || matches!(self.value, InfoValue::Diff { open: Some(_), .. }) + } +} + /// Widest of the labels actually on screen, so the values line up without a /// fixed width guessing at them. /// @@ -93,11 +213,7 @@ const PROCS_POLL: std::time::Duration = std::time::Duration::from_millis(2000); /// clamp keeps the longest of those from eating the panel; anything past it /// runs into the gap rather than folding, which `whitespace_nowrap` on the /// label guarantees. -fn info_label_column( - rows: &[(&'static str, String)], - window: &mut Window, - cx: &gpui::App, -) -> gpui::Pixels { +fn info_label_column(rows: &[InfoRow], window: &mut Window, cx: &gpui::App) -> gpui::Pixels { // Shaping needs real pixels, so this is the one place the rem has to be // resolved by hand. Both bounds were measured against a 12px label, so // they are carried as multiples of it rather than as pixels — otherwise @@ -115,11 +231,12 @@ fn info_label_column( }; let widest = rows .iter() - .map(|(k, _)| { + .map(|row| { + let k = row.label; window .text_system() .shape_line( - gpui::SharedString::from(*k), + gpui::SharedString::from(k), px(label_px), &[gpui::TextRun { len: k.len(), @@ -418,7 +535,12 @@ impl Tty7App { div() .flex_1() .min_w_0() - .child(Input::new(input).appearance(false).xsmall()), + // A filter with no way out of it but selecting the text + // and deleting it is a filter people leave on and then + // wonder where their files went. The button only exists + // while there is something to clear, so an empty field + // still reads as one line of chrome. + .child(Input::new(input).appearance(false).xsmall().cleanable(true)), ) .into_any_element() } @@ -468,28 +590,50 @@ impl Tty7App { fn render_panel_info(&mut self, window: &mut Window, cx: &mut Context) -> AnyElement { let title = self.panel_title(t(L10nKey::PanelInfoTitle), None, None, window, cx); - let mut rows: Vec<(&'static str, String)> = Vec::new(); - let mut cwd_for_actions: Option<(PathBuf, bool)> = None; + let mut rows: Vec = Vec::new(); let mut pane_id: Option = None; let mut forwards_pane: Option = None; + // Whether the ports below are this machine's. They are listed by the + // daemon that owns the pane, so what decides it is which machine that + // daemon runs on — not whether the shell has since ssh'd somewhere, + // which would hide the browser tile on a `ssh -L` pane whose forwarded + // listener is on this machine and reachable. + let mut local_pane = false; + // Where the `changes` row's counts lead. Same source as the sidebar's, + // and gated on the same setting, so turning the preview off turns it + // off in both places rather than in one of them. + let mut diff_target: Option<(crate::ui::host_ops::HostId, PathBuf)> = None; + let mut git: Option = None; if let Some(tab) = self.tabs.get(self.active) { if let Some(leaf) = tab.detail_pane(window, cx) { let view = leaf.read(cx); pane_id = Some(view.pane_id); + local_pane = view.host_id().is_local(); + diff_target = crate::ui::tab_sidebar::diff_click_cwd( + cx.global::(), + view.git_status_cwd() + .map(|cwd| (view.host_id(), cwd.to_path_buf())), + ); if let Some(cwd) = view.effective_cwd() { - rows.push((t(L10nKey::PanelCwd), compact_path(&cwd))); - // Copy Path is right either way; Reveal only means anything - // when the path is on the machine the file manager can see. - cwd_for_actions = Some((cwd, view.local_cwd().is_some())); + rows.push(InfoRow { + label: t(L10nKey::PanelCwd), + value: InfoValue::Path(compact_path(&cwd)), + // The compacted `~/…` spelling is for reading; what + // goes on the clipboard is the path a shell can use. + copy: Some(cwd.display().to_string()), + // Reveal only means anything when the path is on the + // machine the file manager can see. + reveal: view.local_cwd().is_some().then(|| cwd.clone()), + }); } let shell = match view.shell_spec().map(|s| s.program.clone()) { Some(program) => crate::core::shells::default_shell_name(Some(&program)), None => self.default_shell_label(cx), }; - rows.push((t(L10nKey::PanelShell), shell)); + rows.push(InfoRow::text(t(L10nKey::PanelShell), shell)); if let Some(ssh) = view.ssh_spec() { - rows.push((t(L10nKey::PanelSsh), ssh.host.clone())); + rows.push(InfoRow::text(t(L10nKey::PanelSsh), ssh.host.clone()).copyable()); } let connected_ssh = view .remote_context() @@ -501,21 +645,47 @@ impl Tty7App { if connected_ssh || view.workspace().is_some() { forwards_pane = Some(view.pane_id); } + git = view.git_status(cx); } - if let Some(git) = tab.git_status(Some(window), cx) { - rows.push((t(L10nKey::PanelBranch), git.branch.clone())); - rows.push(( - t(L10nKey::PanelChangesRow), - format!("+{} −{}", git.added, git.removed), - )); + // Read off the same pane the rows above describe, rather than off + // `Tab::git_status`, which resolves a split tab to its *first* leaf + // while `detail_pane` resolves it to the *last focused* one. The + // two agreed while the row was inert text; now that the counts open + // a diff, disagreeing means a click that opens a repository other + // than the one whose numbers were clicked. + if let Some(git) = git { + rows.push(InfoRow::text(t(L10nKey::PanelBranch), git.branch.clone()).copyable()); + rows.push(InfoRow { + label: t(L10nKey::PanelChangesRow), + value: InfoValue::Diff { + added: git.added, + removed: git.removed, + // A clean tree has no diff to open, so the row keeps + // its place in the table but stops being a button. + open: (git.added > 0 || git.removed > 0) + .then_some(diff_target.clone()) + .flatten(), + }, + copy: None, + reveal: None, + }); } if let Some(agent) = tab.agent(cx) { let name = agent.display_name(); - let status = match tab.agent_status(cx) { - Some(s) => format!("{name} · {}", agent_status_label(s)), - None => name.to_string(), - }; - rows.push((t(L10nKey::PanelAgent), status)); + let status = tab.agent_status(cx); + rows.push(InfoRow { + label: t(L10nKey::PanelAgent), + value: InfoValue::Agent { + text: match status { + Some(s) => format!("{name} · {}", agent_status_label(s)), + None => name.to_string(), + }, + dot: status.and_then(|s| s.dot_rgb()), + hollow: status == Some(crate::core::cli_agent::AgentStatus::Waiting), + }, + copy: None, + reveal: None, + }); } } @@ -533,111 +703,257 @@ impl Tty7App { let route = forwards_pane.map(|id| self.forward_route(id, cx)); self.sync_procs(pane_id, route, cx); - let mono = cx.theme().mono_font_family.clone(); - let cwd_label = t(L10nKey::PanelCwd); let label_w = info_label_column(&rows, window, cx); - let mut list = v_flex().px(px(CONTENT_INSET)).py(px(2.)).gap(px(3.)); - for (k, v) in rows { - list = list.child( - h_flex() - .items_baseline() - .gap(px(9.)) - .py(px(1.)) - .text_size(rems(TEXT)) - .child( - div() - .flex_none() - .w(label_w) - .whitespace_nowrap() - .text_color(cx.theme().muted_foreground) - .child(k), - ) - .child(match k == cwd_label { - // A path identifies a pane by its last segment, and - // plain truncation eats exactly that: a deep checkout - // read "/private/tmp/claude-501…" and told you - // nothing. Let the head absorb the shrinking so the - // leaf survives, the way a file manager shows a path. - true => { - let (head, leaf) = split_path_leaf(&v); - h_flex() - .flex_1() - .min_w_0() - .text_size(rems(TEXT_MONO)) - .font_family(mono.clone()) - .text_color(cx.theme().foreground) - .child(div().min_w_0().flex_shrink(999.).truncate().child(head)) - .child(div().min_w_0().flex_shrink(1.).truncate().child(leaf)) - .into_any_element() - } - false => div() - .flex_1() - .min_w_0() - .truncate() - .text_size(rems(TEXT_MONO)) - .font_family(mono.clone()) - .text_color(cx.theme().foreground) - .child(v) - .into_any_element(), - }), - ); + // Rows pad themselves back out to `CONTENT_INSET`, so their hover fill + // bleeds past the text on both sides — the geometry the Source Control + // tab's rows are on, one tab over. + let mut list = v_flex().px(px(CONTENT_INSET - ROW_INSET)).py(px(2.)); + for (i, row) in rows.into_iter().enumerate() { + list = list.child(self.info_row(i, row, label_w, cx)); } let inner = v_flex() .child(self.panel_subtitle(t(L10nKey::PanelSessionSubtitle), false, None, cx)) .child(list) - .when_some(cwd_for_actions, |this, (cwd, local)| { - this.child(self.cwd_actions(cwd, local, cx)) - }) .children(self.procs_section(pane_id, cx)) - .children(self.ports_section(pane_id, cx)) + .children(self.ports_section(pane_id, local_pane, cx)) .children(self.forwards_section(forwards_pane, cx)) .into_any_element(); self.panel_scroll(inner, title) } - fn cwd_actions(&self, cwd: PathBuf, local: bool, cx: &mut Context) -> AnyElement { - let reveal_label = reveal_label(); - h_flex() - .gap(px(2.)) - .px(px(tile_trailing_inset_sm())) - .pt(px(6.)) - .when(local, |this| { - this.child( - crate::ui::tab_strip::chrome_tile_sized( - Button::new("panel-info-reveal").icon(Icon::new(IconName::FolderOpen)), - TILE_SIZE_SM, - TILE_GLYPH_SM, - false, - cx, - ) - .rounded_md() - .tooltip(reveal_label) - .on_click({ - let cwd = cwd.clone(); - move |_, _window, cx| cx.reveal_path(&cwd) - }), + /// One label/value line, with whatever it can do revealed on hover. + /// + /// The two cwd buttons used to sit in a strip of their own under the whole + /// table, unlabelled, four rows below the path they acted on and closer to + /// the Processes heading than to it — "copy" and "open" with no stated + /// object. Hanging them off the row they belong to is what makes them + /// answerable, and it buys the panel the hover feedback it had none of. + fn info_row( + &self, + i: usize, + row: InfoRow, + label_w: gpui::Pixels, + cx: &mut Context, + ) -> AnyElement { + let sf = cx.global::().sidebar; + let mono = cx.theme().mono_font_family.clone(); + let id = gpui::SharedString::from(format!("panel-info-row-{i}")); + let interactive = row.interactive(); + let tiles_wide = usize::from(row.reveal.is_some()) + usize::from(row.copy.is_some()); + // "Copy" is honest on a branch or a host, but on the working directory + // it is the file tree's *Copy Path*, and the two live a right-click + // apart from each other. Say the same words for the same act. + let copy_label = match row.value { + InfoValue::Path(_) => t(L10nKey::FileTreeContextCopyPath), + _ => t(L10nKey::CmdCopy), + }; + + let value = match row.value { + // A path identifies a pane by its last segment, and plain + // truncation eats exactly that: a deep checkout read + // "/private/tmp/claude-501…" and told you nothing. Let the head + // absorb the shrinking so the leaf survives, the way a file + // manager shows a path. + InfoValue::Path(v) => { + let (head, leaf) = split_path_leaf(&v); + h_flex() + .flex_1() + .min_w_0() + .text_size(rems(TEXT_MONO)) + .font_family(mono.clone()) + .text_color(cx.theme().foreground) + .child(div().min_w_0().flex_shrink(999.).truncate().child(head)) + .child(div().min_w_0().flex_shrink(1.).truncate().child(leaf)) + .into_any_element() + } + InfoValue::Text(v) => div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(TEXT_MONO)) + .font_family(mono.clone()) + .text_color(cx.theme().foreground) + .child(v) + .into_any_element(), + InfoValue::Diff { + added, + removed, + open, + } => { + let clean = added == 0 && removed == 0; + // Sized to the two numbers, not to the row: `flex_1` here made + // the whole rest of the line a button, so a click on the empty + // half of the row opened the overlay and a pointer crossing it + // underlined counts it was nowhere near. The slack belongs to + // the value slot around this, which is what holds it. + let counts = h_flex() + .flex_none() + .items_baseline() + .gap(px(6.)) + .text_size(rems(TEXT_MONO)) + .font_family(mono.clone()) + // A clean tree said "+0 −0", which is two numbers to read + // before learning there was nothing to read. The dash is + // the table convention for an empty cell, and it needs no + // translating. + .when(clean, |this| { + this.child( + div() + .text_color(cx.theme().muted_foreground) + .child("—".to_string()), + ) + }) + .when(added > 0, |this| { + this.child( + div() + .text_color(cx.theme().success) + .child(format!("+{added}")), + ) + }) + .when(removed > 0, |this| { + this.child( + div() + .text_color(cx.theme().danger) + .child(format!("−{removed}")), + ) + }); + match open { + // The row's hover fill says the line reacts; the underline + // says where the button inside it starts — the same pair + // the sidebar's counts wear. + Some((host, cwd)) => counts + .id(("panel-info-diff", i)) + .cursor_pointer() + .hover(|s| s.underline()) + .on_click(cx.listener(move |this, _, window, cx| { + cx.stop_propagation(); + this.toggle_diff_overlay(host, cwd.clone(), window, cx); + })) + .into_any_element(), + None => counts.into_any_element(), + } + } + // The dot hangs out of the flow rather than sitting in it. A + // childless box has no baseline of its own, so as a flex item it + // offers up its bottom edge instead — and the row, which aligns + // its label and its value on their shared baseline, then hoisted + // the whole value six pixels and left "agent" sitting under its + // own value. Out of flow it cannot be mistaken for the thing that + // sets the line. + InfoValue::Agent { text, dot, hollow } => div() + .flex_1() + .min_w_0() + .relative() + .child( + div() + .min_w_0() + .truncate() + .when(dot.is_some(), |d| d.pl(rems(PIP_SIZE + PIP_GAP))) + .text_size(rems(TEXT_MONO)) + .font_family(mono.clone()) + .text_color(cx.theme().foreground) + .child(text), ) + .children(dot.map(|rgb| { + status_pip(rgb, hollow, crate::ui::theme::workspace_surface_color(cx)) + })) + .into_any_element(), + }; + + // The strip is opaque and pinned to the row's right edge, so whatever + // sits under it is unreadable for as long as the pointer is on the row + // — and on the working-directory row what sits there is the leaf, the + // one segment the head-first elision exists to keep. Hold that much + // width back from the value for good rather than only while hovered: + // taking it on hover would re-elide the path under the pointer, which + // is the pixel-shifting the strip is absolutely positioned to avoid. + let value = h_flex() + .flex_1() + .min_w_0() + .items_baseline() + .when(tiles_wide > 0, |this| { + this.pr(px(tiles_wide as f32 * (TILE_SIZE_XS + 1.) + 4.)) }) - .child( - crate::ui::tab_strip::chrome_tile_sized( - Button::new("panel-info-copy-path").icon(Icon::new(IconName::Copy)), - TILE_SIZE_SM, - TILE_GLYPH_SM, - false, + .child(value); + + let mut tiles = action_strip(&id, sf.hover); + let mut has_tiles = false; + if let Some(cwd) = row.reveal { + has_tiles = true; + tiles = tiles.child( + self.info_tile( + "panel-info-reveal", + IconName::FolderOpen, + reveal_label(), cx, ) - .rounded_md() - .tooltip(t(L10nKey::FileTreeContextCopyPath)) - .on_click(move |_, _window, cx| { - cx.write_to_clipboard(gpui::ClipboardItem::new_string( - cwd.display().to_string(), - )); - }), + .on_click(move |_, _window, cx| cx.reveal_path(&cwd)), + ); + } + if let Some(text) = row.copy { + has_tiles = true; + tiles = tiles.child( + self.info_tile(("panel-info-copy", i), IconName::Copy, copy_label, cx) + .on_click(move |_, _window, cx| { + cx.write_to_clipboard(gpui::ClipboardItem::new_string(text.clone())); + }), + ); + } + // A row with nothing to reveal gets no strip at all, rather than an + // empty one carrying a hover subscription for a set of buttons that + // does not exist. + let actions = has_tiles.then_some(tiles); + + h_flex() + .id(id.clone()) + .group(id) + .relative() + .items_baseline() + .gap(px(9.)) + .px(px(ROW_INSET)) + .py(px(2.)) + .rounded(px(5.)) + .text_size(rems(TEXT)) + // Only rows that can do something light up, so the fill is never a + // promise the row cannot keep. + .when(interactive, |this| { + this.hover(|s| s.bg(gpui::rgb(sf.hover))) + }) + .child( + div() + .flex_none() + .w(label_w) + .whitespace_nowrap() + .text_color(cx.theme().muted_foreground) + .child(row.label), ) + .child(value) + .children(actions) .into_any_element() } + /// The tile an Info row's hover strip is made of — the [`TILE_SIZE_XS`] + /// box the Source Control rows use, because three `TILE_SIZE_SM` squares + /// would eat a quarter of the width a path has to live in. + fn info_tile( + &self, + id: impl Into, + icon: IconName, + tooltip: &'static str, + cx: &mut Context, + ) -> Button { + crate::ui::tab_strip::chrome_tile_sized( + Button::new(id).icon(Icon::new(icon)), + TILE_SIZE_XS, + TILE_GLYPH_XS, + false, + cx, + ) + .rounded(px(4.)) + .tooltip(tooltip) + } + pub(crate) fn panel_subtitle( &self, text: &str, @@ -698,11 +1014,16 @@ impl Tty7App { .pl(px(f32::from(p.depth) * 10.)) .text_size(rems(TEXT_MONO)) .font_family(mono.clone()) - .text_color(if p.foreground { - cx.theme().foreground - } else { - cx.theme().muted_foreground + // Which of these has the terminal is the one thing + // the list is read for, and a hue apart from its + // neighbours was carrying it alone — a difference + // a light theme flattens and colour vision can + // miss. Weight says it a second way. + .when(p.foreground, |d| { + d.font_weight(gpui::FontWeight::MEDIUM) + .text_color(cx.theme().foreground) }) + .when(!p.foreground, |d| d.text_color(cx.theme().muted_foreground)) .child(p.name.clone()), ) .child(info_chip( @@ -721,18 +1042,82 @@ impl Tty7App { ) } - fn ports_section(&self, pane_id: Option, cx: &mut Context) -> Option { + /// The listening ports of the pane's processes. + /// + /// `local` is whether the daemon that listed these ports is this machine's, + /// and it is what decides whether the browser tile appears: a port on a + /// remote host is not this machine's port, and opening it here is not a + /// near miss, it is a different service. It is deliberately about the + /// *host* and not about whether the shell has ssh'd somewhere — the ports + /// come from the pane's own process tree either way, so a `ssh -L` pane's + /// forwarded listener really is on this machine and really does open. + fn ports_section( + &self, + pane_id: Option, + local: bool, + cx: &mut Context, + ) -> Option { let ports = &self.procs(pane_id)?.ports; if ports.is_empty() { return None; } + let sf = cx.global::().sidebar; let mono = cx.theme().mono_font_family.clone(); - let mut list = v_flex().px(px(CONTENT_INSET)).py(px(1.)).gap(px(2.)); - for p in ports { + let mut list = v_flex().px(px(CONTENT_INSET - ROW_INSET)).py(px(1.)); + for (i, p) in ports.iter().enumerate() { + // "What is this pane serving, and where" is the question the + // section answers, and the next thing anyone does with the answer + // is go there — so the row hands over an address instead of making + // it something to read off the screen and retype. + let authority = p.authority(); + // Keyed by the row, not by the port: `listening_ports` drops a + // duplicate only when the port *and* the pid match, so a + // pre-forking server — nginx, gunicorn, a node cluster — puts one + // row per worker on screen, all on port 8000. Sharing an id makes + // gpui hand them one interactive state between them, and a click on + // the last row lights up the tooltip and the pressed fill on all + // the others. + let id = gpui::SharedString::from(format!("panel-port-{}-{}", p.port, p.pid)); + let mut tiles_wide = 1; + let mut actions = action_strip(&id, sf.hover); + if local { + tiles_wide += 1; + let url = format!("http://{authority}"); + actions = actions.child( + self.info_tile( + ("panel-port-open", i), + IconName::Globe, + t(L10nKey::PanelOpenInBrowser), + cx, + ) + .on_click(move |_, _window, cx| cx.open_url(&url)), + ); + } + actions = actions.child( + self.info_tile( + ("panel-port-copy", i), + IconName::Copy, + t(L10nKey::CmdCopy), + cx, + ) + .on_click({ + let authority = authority.clone(); + move |_, _window, cx| { + cx.write_to_clipboard(gpui::ClipboardItem::new_string(authority.clone())); + } + }), + ); list = list.child( h_flex() + .id(id.clone()) + .group(id) + .relative() .items_center() .gap(px(8.)) + .px(px(ROW_INSET)) + .py(px(1.)) + .rounded(px(5.)) + .hover(|s| s.bg(gpui::rgb(sf.hover))) .child(info_chip( &p.port.to_string(), cx.theme().accent, @@ -744,11 +1129,16 @@ impl Tty7App { .flex_1() .min_w_0() .truncate() + // Room held back for the strip, so the process name + // ends where the buttons begin instead of under + // them. Same reservation the Info rows make. + .pr(px(tiles_wide as f32 * (TILE_SIZE_XS + 1.) + 4.)) .text_size(rems(TEXT_MONO)) .font_family(mono.clone()) .text_color(cx.theme().muted_foreground) .child(p.name.clone()), - ), + ) + .child(actions), ); } Some( @@ -916,6 +1306,50 @@ pub(crate) fn git_badge(letter: &str, color: gpui::Hsla, mono: &gpui::SharedStri .into_any_element() } +/// Diameter of the agent dot in the Info panel, and the gap between it and the +/// word it qualifies. +/// +/// Seven sixteenths of a rem — seven pixels at the default interface size, +/// because a dot on a line of text has to survive being read at a glance +/// without becoming a bullet, and a rem rather than a pixel because the line it +/// sits in is sized in rems: pinned in pixels it slid towards the cap height of +/// its own row the moment the interface font scale moved off 100%. +const PIP_SIZE: f32 = 7. * STEP; +const PIP_GAP: f32 = 7. * STEP; + +/// How far down the value box the dot starts, again as a fraction of the text +/// it is centred in rather than a pixel count. +const PIP_TOP: f32 = 6. * STEP; + +/// The dot a tab wears for its agent's state, at the size a line of panel text +/// can carry it. +/// +/// Same colours and the same hollow-for-Waiting rule as the sidebar's, because +/// it is the same fact: a reader who has learned that amber-with-a-hole means +/// "it wants you" on a tab must not have to learn it a second time here. Same +/// *shape*, too — [`Tty7App::status_dot`] punches a small hole out of a filled +/// dot, so drawing this one as a thin ring would have been a second dialect of +/// the one rule the doc above promises is shared. `hole` is the colour behind +/// the dot, which is what a hole in it has to be painted in; the agent row is +/// never interactive, so that colour is the panel's own and does not move +/// under the pointer. +fn status_pip(rgb: u32, hollow: bool, hole: gpui::Hsla) -> AnyElement { + div() + .absolute() + .left_0() + .top(rems(PIP_TOP)) + .size(rems(PIP_SIZE)) + .rounded_full() + .bg(gpui::rgb(rgb)) + .when(hollow, |dot| { + dot.flex() + .items_center() + .justify_center() + .child(div().size(rems(PIP_SIZE * 0.36)).rounded_full().bg(hole)) + }) + .into_any_element() +} + /// A small filled pill around a mono token — a pid, a port number. /// /// The padding and the radius are derived from the text size: at @@ -981,7 +1415,87 @@ fn compact_path(path: &std::path::Path) -> String { #[cfg(test)] mod tests { - use super::split_path_leaf; + use super::{InfoRow, InfoValue, split_path_leaf}; + + fn diff(added: u32, removed: u32, open: bool) -> InfoRow { + InfoRow { + label: "changes", + value: InfoValue::Diff { + added, + removed, + open: open.then(|| { + ( + crate::ui::host_ops::HostId::LOCAL, + std::path::PathBuf::from("/w/repo"), + ) + }), + }, + copy: None, + reveal: None, + } + } + + #[test] + fn a_row_lights_up_only_when_there_is_something_behind_it() { + // The hover fill is the panel's only "this line does something", so a + // row that cannot do anything must not draw one. + assert!( + !InfoRow::text("shell", "zsh".into()).interactive(), + "a plain readout is not a control" + ); + assert!( + InfoRow::text("branch", "main".into()) + .copyable() + .interactive(), + "a copy tile is something to hover for" + ); + assert!( + InfoRow { + reveal: Some(std::path::PathBuf::from("/w/repo")), + ..InfoRow::text("cwd", "/w/repo".into()) + } + .interactive(), + "so is Reveal, even with nothing else on the row" + ); + } + + #[test] + fn counts_are_a_button_only_when_there_is_a_diff_to_open() { + assert!( + diff(3, 1, true).interactive(), + "changes with somewhere to go open the overlay" + ); + // Both halves have to hold: a clean tree has no diff to show, and the + // setting that governs the sidebar's counts can take the target away + // from a dirty one. + assert!( + !diff(0, 0, false).interactive(), + "a clean tree is a readout, not a link" + ); + assert!( + !diff(3, 1, false).interactive(), + "no target means no link, however dirty the tree" + ); + } + + #[test] + fn copyable_takes_the_text_the_row_shows_and_nothing_else() { + // `copyable()` reads the value it was given; rows built with an + // explicit clipboard string (the cwd, which copies the real path + // rather than the `~/…` spelling) set `copy` themselves. + assert_eq!( + InfoRow::text("ssh", "box".into()) + .copyable() + .copy + .as_deref(), + Some("box") + ); + assert_eq!( + diff(3, 1, true).copyable().copy, + None, + "there is no sensible clipboard form of two coloured numbers" + ); + } #[test] fn the_head_and_leaf_rejoin_into_the_path_they_came_from() { diff --git a/src/ui/scm/detail.rs b/src/ui/scm/detail.rs index d10e3a19..66aa7058 100644 --- a/src/ui/scm/detail.rs +++ b/src/ui/scm/detail.rs @@ -39,7 +39,7 @@ use tty7_core::core::git::status::DecoStatus; use crate::terminal::git_diff::DiffSource; use crate::ui::app::{CONTENT_INSET, Tty7App}; use crate::ui::i18n::{L10nKey, t, t_plural}; -use crate::ui::right_panel::{git_badge, info_chip}; +use crate::ui::right_panel::{ROW_INSET, git_badge, info_chip}; use crate::ui::scm::path::{relative_time, split_display_path}; use crate::ui::scm::state::{CommitDetailView, RepoKey}; use crate::ui::scm::status::{status_color, status_glyph}; @@ -50,14 +50,11 @@ use crate::ui::scm::status::{status_color, status_glyph}; /// Both numbers are a 12px row's. gpui leads a plain `div` at phi, so the row's /// mono name occupies `round(12 × 1.618) = 19px`, and 24 gives that line 2.5px /// of air on each side — dense, which is what a 260px column of paths wants. -/// The text lands on `CONTENT_INSET` whatever `ROW_INSET` is, since the list -/// subtracts it outside the row and the row adds it back inside. -/// -/// Both have to equal `panel.rs`'s pair. That file carries the same two -/// constants for the same reason, and a reader who opens a commit must not -/// feel the pitch change under them. +/// The text lands on `CONTENT_INSET` whatever [`ROW_INSET`] is, since the list +/// subtracts it outside the row and the row adds it back inside — which is why +/// the inset itself is the panel-wide constant rather than a third copy here: a +/// reader who opens a commit must not feel the pitch change under them. const ROW_H: f32 = 24.; -const ROW_INSET: f32 = 4.; /// How much of the body is shown before it folds. Four lines is a paragraph; /// past that it is a changelog, and the file list is what the reader came for. diff --git a/src/ui/scm/panel.rs b/src/ui/scm/panel.rs index 309580c7..604de68b 100644 --- a/src/ui/scm/panel.rs +++ b/src/ui/scm/panel.rs @@ -29,7 +29,7 @@ use crate::terminal::git_diff::DiffSource; use crate::ui::app::{CONTENT_INSET, TILE_GLYPH_XS, TILE_SIZE_XS, Tty7App}; use crate::ui::host_ops::{HostId, SharedHost}; use crate::ui::i18n::{L10nKey, t, t_fmt, t_plural}; -use crate::ui::right_panel::{SEARCH_H, git_badge, info_chip}; +use crate::ui::right_panel::{ROW_INSET, SEARCH_H, action_strip, git_badge, info_chip}; use crate::ui::rounding::{CARD_RADIUS, HAIRLINE, RoundedCorners as _, segment_corners}; use crate::ui::scm::ScmIntent; use crate::ui::scm::path::{elide_middle, split_display_path}; @@ -53,15 +53,6 @@ const ROW_H: f32 = 24.; /// different numbers. const BADGE_W: f32 = 14.; -/// Rows are laid out inside this inset and then pad themselves back out, so a -/// hovered row's background is wider than its text on both sides. -/// -/// The text lands on `CONTENT_INSET` whatever this is — the list subtracts it -/// outside the row and the row adds it back inside — so all this number sets -/// is how far the hover fill bleeds past the text. `scm/detail.rs` carries the -/// same pair for the same reason. -const ROW_INSET: f32 = 4.; - /// The key context the message box installs, and the one `ScmCommit` is /// bound inside. The two are the same string on purpose: a binding whose /// context nothing attaches is a binding that never fires. @@ -1651,43 +1642,6 @@ impl Tty7App { } } -/// The strip the row and group action buttons live in, revealed by hovering -/// `row`. -/// -/// Absolutely positioned and opaque, so it covers the tail of the directory -/// rather than pushing it aside: hovering a row must not move a single pixel -/// of it, or the list crawls under the pointer. -/// -/// It stops the mouse-down by hand instead of calling `occlude()`, which is -/// the obvious way to keep a click off the row underneath and was what made -/// the buttons vanish the moment the pointer reached them. `occlude()` is a -/// *hitbox* behaviour, and gpui inserts hitboxes in prepaint, which never -/// looks at `visibility` — so the strip blocked the mouse even while it was -/// invisible. Blocking cuts the hit test short at the blocking hitbox, and the -/// row's hitbox is behind this one because a parent prepaints before its -/// children; `group_hover` is nothing more than "is the group's hitbox -/// hovered", so the row stopped counting as hovered and the strip hid itself -/// — background, buttons and all — with the pointer sitting right on it. The -/// buttons' own hitboxes came from prepaint and outlived the paint, so they -/// went on answering tooltips for glyphs that were no longer drawn. -/// -/// Stopping propagation buys the same "this click is ours, not the row's" -/// without lying to the hit test: children register their handlers after this -/// one and gpui bubbles back to front, so a button still gets its click first. -fn action_strip(row: &SharedString, backing: u32) -> gpui::Div { - h_flex() - .absolute() - .right(px(ROW_INSET)) - .top_0() - .bottom_0() - .items_center() - .gap(px(1.)) - .bg(gpui::rgb(backing)) - .invisible() - .group_hover(row.clone(), |s| s.visible()) - .on_any_mouse_down(|_, _, cx| cx.stop_propagation()) -} - /// Which sections an entry shows up in. /// /// A file can be staged and unstaged at once (`XY == "MM"`), and then it diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index 4ac3f399..0429e106 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -1337,7 +1337,14 @@ fn group_names(roots: &[&PathBuf]) -> Vec { } } -fn diff_click_cwd(cfg: &Config, target: Option) -> Option { +/// Whether a `+N −M` is a button, and what it opens if it is. +/// +/// One function because the setting is one setting: the sidebar's counts and +/// the Info panel's `changes` row are the same number about the same working +/// tree, and "Open diff preview from sidebar counts" turning one of them into +/// plain text while the other stayed clickable would be a setting that half +/// works. +pub(crate) fn diff_click_cwd(cfg: &Config, target: Option) -> Option { cfg.sidebar_diff_preview.then_some(target).flatten() } diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index 653d5110..1f214029 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -740,8 +740,18 @@ impl Tty7App { "ToggleRightPanel", cx, )) - .on_click(cx.listener(|this, _, _window, cx| { + // On macOS this tile is drawn inside the panel's own + // titlebar while the panel is open, so closing from it + // destroys the element holding the focus — and a keymap + // scoped to a focused thing goes quiet with it, leaving the + // ⌘J that would undo this doing nothing. Hand the terminal + // back what it lost, the same way the tab tiles below do. + .on_click(cx.listener(|this, _, window, cx| { + let closing = this.right_panel_open(cx); this.toggle_right_panel(cx); + if closing { + this.focus_active(window, cx); + } })), ), ) @@ -796,8 +806,28 @@ impl Tty7App { } _ => SharedString::from(t(label_key)), }) - .on_click(cx.listener(move |this, _, _window, cx| { - this.set_right_panel_tab(tab, cx); + // A tile for another tab switches to it; the lit one puts + // the panel away, the way an activity bar behaves + // everywhere else. Pressing it used to do nothing at all + // — a dead click on the one control in the row that looks + // like it should undo itself. (These tiles only exist + // while the panel is open, so `ToggleRightPanel` and the + // chrome tile beside them are still what brings it back.) + .on_click(cx.listener(move |this, _, window, cx| { + match this.right_panel_open(cx) && this.right_panel_tab == tab { + true => { + this.toggle_right_panel(cx); + // These tiles live inside the panel, so + // closing from one destroys the element that + // holds the focus and leaves it nowhere — + // and a keymap whose bindings are scoped to a + // focused thing goes quiet with it, so the + // ⌘J that would undo this did nothing at all. + // Hand the terminal back what it lost. + this.focus_active(window, cx); + } + false => this.set_right_panel_tab(tab, cx), + } })), ) .into_any_element()