From 2ee257a5621d3c2714945e117d958383c14ae513 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:27:18 +0200 Subject: [PATCH] task_ui: Move status indicator into tab bar of terminal panel (#10846) I'm not a huge fan of this change (& I expect the placement to change). The plan is to have the button in a toolbar of terminal panel, but I'm not sure if occupying a whole line of vertical space for a single button is worth it; I suppose we might want to put more of tasks ui inside of that toolbar. Release Notes: - Removed task status indicator and added "Spawn task" action to terminal panel context menu. --- Cargo.lock | 2 +- crates/tasks_ui/Cargo.toml | 1 - crates/tasks_ui/src/lib.rs | 5 +- crates/tasks_ui/src/modal.rs | 3 +- crates/tasks_ui/src/status_indicator.rs | 98 ---------------------- crates/terminal_view/Cargo.toml | 1 + crates/terminal_view/src/terminal_panel.rs | 50 ++++++++--- crates/workspace/src/pane.rs | 4 +- crates/zed/src/zed.rs | 2 - 9 files changed, 46 insertions(+), 120 deletions(-) delete mode 100644 crates/tasks_ui/src/status_indicator.rs diff --git a/Cargo.lock b/Cargo.lock index 52942e16b64950bf4b67dc07edbbe10b8aef81fc..00b6922e07fd6ac4cdb7832738fcaab13f0589be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9765,7 +9765,6 @@ dependencies = [ "serde_json", "settings", "task", - "terminal", "tree-sitter-rust", "tree-sitter-typescript", "ui", @@ -9863,6 +9862,7 @@ dependencies = [ "shellexpand", "smol", "task", + "tasks_ui", "terminal", "theme", "ui", diff --git a/crates/tasks_ui/Cargo.toml b/crates/tasks_ui/Cargo.toml index b19122f1de91c1a7bc9deeef41e43042894a8895..67f62e2dcff27595d690009a89aa88beb9010c3c 100644 --- a/crates/tasks_ui/Cargo.toml +++ b/crates/tasks_ui/Cargo.toml @@ -22,7 +22,6 @@ serde.workspace = true settings.workspace = true ui.workspace = true util.workspace = true -terminal.workspace = true workspace.workspace = true language.workspace = true diff --git a/crates/tasks_ui/src/lib.rs b/crates/tasks_ui/src/lib.rs index 2433ca93cef9d240adf22dac7887a8d8304517d6..4898c8af0d98692d9dc1e3bad2f20a36c03dab7f 100644 --- a/crates/tasks_ui/src/lib.rs +++ b/crates/tasks_ui/src/lib.rs @@ -8,7 +8,7 @@ use anyhow::Context; use editor::Editor; use gpui::{AppContext, ViewContext, WindowContext}; use language::{BasicContextProvider, ContextProvider, Language}; -use modal::{Spawn, TasksModal}; +use modal::TasksModal; use project::{Location, TaskSourceKind, WorktreeId}; use task::{ResolvedTask, TaskContext, TaskTemplate, TaskVariables}; use util::ResultExt; @@ -16,9 +16,8 @@ use workspace::Workspace; mod modal; mod settings; -mod status_indicator; -pub use status_indicator::TaskStatusIndicator; +pub use modal::Spawn; pub fn init(cx: &mut AppContext) { settings::TaskSettings::register(cx); diff --git a/crates/tasks_ui/src/modal.rs b/crates/tasks_ui/src/modal.rs index db91c817f6786d8cced3194649c7db79e1eb7e1c..ab04cd5909202e24ee654fb0dedeb7d6f57cbafc 100644 --- a/crates/tasks_ui/src/modal.rs +++ b/crates/tasks_ui/src/modal.rs @@ -31,10 +31,11 @@ pub struct Spawn { } impl Spawn { - pub(crate) fn modal() -> Self { + pub fn modal() -> Self { Self { task_name: None } } } + /// Rerun last task #[derive(PartialEq, Clone, Deserialize, Default)] pub struct Rerun { diff --git a/crates/tasks_ui/src/status_indicator.rs b/crates/tasks_ui/src/status_indicator.rs deleted file mode 100644 index 8ed837cf481c0b1013695bbe2f4023c77a80e5dd..0000000000000000000000000000000000000000 --- a/crates/tasks_ui/src/status_indicator.rs +++ /dev/null @@ -1,98 +0,0 @@ -use gpui::{IntoElement, Render, View, WeakView}; -use settings::Settings; -use ui::{ - div, ButtonCommon, Clickable, Color, FluentBuilder, IconButton, IconName, Tooltip, - VisualContext, WindowContext, -}; -use workspace::{item::ItemHandle, StatusItemView, Workspace}; - -use crate::{modal::Spawn, settings::TaskSettings}; - -enum TaskStatus { - Failed, - Running, - Succeeded, -} - -/// A status bar icon that surfaces the status of running tasks. -/// It has a different color depending on the state of running tasks: -/// - red if any open task tab failed -/// - else, yellow if any open task tab is still running -/// - else, green if there tasks tabs open, and they have all succeeded -/// - else, no indicator if there are no open task tabs -pub struct TaskStatusIndicator { - workspace: WeakView, -} - -impl TaskStatusIndicator { - pub fn new(workspace: WeakView, cx: &mut WindowContext) -> View { - cx.new_view(|_| Self { workspace }) - } - fn current_status(&self, cx: &mut WindowContext) -> Option { - self.workspace - .update(cx, |this, cx| { - let mut status = None; - let project = this.project().read(cx); - - for handle in project.local_terminal_handles() { - let Some(handle) = handle.upgrade() else { - continue; - }; - let handle = handle.read(cx); - let task_state = handle.task(); - if let Some(state) = task_state { - match state.status { - terminal::TaskStatus::Running => { - let _ = status.insert(TaskStatus::Running); - } - terminal::TaskStatus::Completed { success } => { - if !success { - let _ = status.insert(TaskStatus::Failed); - return status; - } - status.get_or_insert(TaskStatus::Succeeded); - } - _ => {} - }; - } - } - status - }) - .ok() - .flatten() - } -} - -impl Render for TaskStatusIndicator { - fn render(&mut self, cx: &mut gpui::ViewContext) -> impl IntoElement { - if !TaskSettings::get_global(cx).show_status_indicator { - return div().into_any_element(); - } - let current_status = self.current_status(cx); - let color = current_status.map(|status| match status { - TaskStatus::Failed => Color::Error, - TaskStatus::Running => Color::Warning, - TaskStatus::Succeeded => Color::Success, - }); - IconButton::new("tasks-activity-indicator", IconName::Play) - .when_some(color, |this, color| this.icon_color(color)) - .on_click(cx.listener(|this, _, cx| { - this.workspace - .update(cx, |this, cx| { - crate::spawn_task_or_modal(this, &Spawn::modal(), cx) - }) - .ok(); - })) - .tooltip(|cx| Tooltip::for_action("Spawn tasks", &Spawn { task_name: None }, cx)) - .into_any_element() - } -} - -impl StatusItemView for TaskStatusIndicator { - fn set_active_pane_item( - &mut self, - _: Option<&dyn ItemHandle>, - _: &mut ui::prelude::ViewContext, - ) { - } -} diff --git a/crates/terminal_view/Cargo.toml b/crates/terminal_view/Cargo.toml index d9e3606d4da5533fb17bdf6e2f6754e5193dc755..f6e53bc22f69828ba51c50ade96aa7867c46440f 100644 --- a/crates/terminal_view/Cargo.toml +++ b/crates/terminal_view/Cargo.toml @@ -24,6 +24,7 @@ itertools.workspace = true language.workspace = true project.workspace = true task.workspace = true +tasks_ui.workspace = true search.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index 6f3fe0a15c1724eb25a48b6fe4b7c8c3b265eec6..8682ac05b1fb7a7b247a9db66c17a2b56417d1d1 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -5,9 +5,9 @@ use collections::{HashMap, HashSet}; use db::kvp::KEY_VALUE_STORE; use futures::future::join_all; use gpui::{ - actions, Action, AppContext, AsyncWindowContext, Entity, EventEmitter, ExternalPaths, - FocusHandle, FocusableView, IntoElement, ParentElement, Pixels, Render, Styled, Subscription, - Task, View, ViewContext, VisualContext, WeakView, WindowContext, + actions, Action, AppContext, AsyncWindowContext, DismissEvent, Entity, EventEmitter, + ExternalPaths, FocusHandle, FocusableView, IntoElement, ParentElement, Pixels, Render, Styled, + Subscription, Task, View, ViewContext, VisualContext, WeakView, WindowContext, }; use itertools::Itertools; use project::{Fs, ProjectEntryId}; @@ -16,7 +16,10 @@ use serde::{Deserialize, Serialize}; use settings::Settings; use task::{RevealStrategy, SpawnInTerminal, TaskId}; use terminal::terminal_settings::{Shell, TerminalDockPosition, TerminalSettings}; -use ui::{h_flex, ButtonCommon, Clickable, IconButton, IconSize, Selectable, Tooltip}; +use ui::{ + h_flex, ButtonCommon, Clickable, ContextMenu, FluentBuilder, IconButton, IconSize, Selectable, + Tooltip, +}; use util::{ResultExt, TryFutureExt}; use workspace::{ dock::{DockPosition, Panel, PanelEvent}, @@ -59,7 +62,6 @@ pub struct TerminalPanel { impl TerminalPanel { fn new(workspace: &Workspace, cx: &mut ViewContext) -> Self { - let terminal_panel = cx.view().downgrade(); let pane = cx.new_view(|cx| { let mut pane = Pane::new( workspace.weak_handle(), @@ -73,19 +75,43 @@ impl TerminalPanel { pane.set_can_navigate(false, cx); pane.display_nav_history_buttons(None); pane.set_render_tab_bar_buttons(cx, move |pane, cx| { - let terminal_panel = terminal_panel.clone(); h_flex() .gap_2() .child( IconButton::new("plus", IconName::Plus) .icon_size(IconSize::Small) - .on_click(move |_, cx| { - terminal_panel - .update(cx, |panel, cx| panel.add_terminal(None, None, cx)) - .log_err(); - }) - .tooltip(|cx| Tooltip::text("New Terminal", cx)), + .on_click(cx.listener(|pane, _, cx| { + let focus_handle = pane.focus_handle(cx); + let menu = ContextMenu::build(cx, |menu, _| { + menu.action( + "New Terminal", + workspace::NewTerminal.boxed_clone(), + ) + .entry( + "Spawn task", + Some(tasks_ui::Spawn::modal().boxed_clone()), + move |cx| { + // We want the focus to go back to terminal panel once task modal is dismissed, + // hence we focus that first. Otherwise, we'd end up without a focused element, as + // context menu will be gone the moment we spawn the modal. + cx.focus(&focus_handle); + cx.dispatch_action( + tasks_ui::Spawn::modal().boxed_clone(), + ); + }, + ) + }); + cx.subscribe(&menu, |pane, _, _: &DismissEvent, _| { + pane.new_item_menu = None; + }) + .detach(); + pane.new_item_menu = Some(menu); + })) + .tooltip(|cx| Tooltip::text("New...", cx)), ) + .when_some(pane.new_item_menu.as_ref(), |el, new_item_menu| { + el.child(Pane::render_menu_overlay(new_item_menu)) + }) .child({ let zoomed = pane.is_zoomed(); IconButton::new("toggle_zoom", IconName::Maximize) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index de561320fae6632162d4717016c683d77030d4ce..c4f62715b309b8a2c4734dc1ab724c084f0e8eea 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -193,7 +193,7 @@ pub struct Pane { last_focus_handle_by_item: HashMap, nav_history: NavHistory, toolbar: View, - new_item_menu: Option>, + pub new_item_menu: Option>, split_item_menu: Option>, // tab_context_menu: View, pub(crate) workspace: WeakView, @@ -1747,7 +1747,7 @@ impl Pane { ) } - fn render_menu_overlay(menu: &View) -> Div { + pub fn render_menu_overlay(menu: &View) -> Div { div().absolute().bottom_0().right_0().size_0().child( deferred( anchored() diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index cf68f8a0e9d1d30f00609daaa000df3471798850..963b7c323726310ca2d711eb01169860d7a6a44a 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -131,7 +131,6 @@ pub fn initialize_workspace(app_state: Arc, cx: &mut AppContext) { cx.new_view(|cx| diagnostics::items::DiagnosticIndicator::new(workspace, cx)); let activity_indicator = activity_indicator::ActivityIndicator::new(workspace, app_state.languages.clone(), cx); - let tasks_indicator = tasks_ui::TaskStatusIndicator::new(workspace.weak_handle(), cx); let active_buffer_language = cx.new_view(|_| language_selector::ActiveBufferLanguage::new(workspace)); let vim_mode_indicator = cx.new_view(|cx| vim::ModeIndicator::new(cx)); @@ -141,7 +140,6 @@ pub fn initialize_workspace(app_state: Arc, cx: &mut AppContext) { status_bar.add_left_item(diagnostic_summary, cx); status_bar.add_left_item(activity_indicator, cx); status_bar.add_right_item(copilot, cx); - status_bar.add_right_item(tasks_indicator, cx); status_bar.add_right_item(active_buffer_language, cx); status_bar.add_right_item(vim_mode_indicator, cx); status_bar.add_right_item(cursor_position, cx);