diff --git a/CHANGELOG.md b/CHANGELOG.md index b13eb19a..c7c40624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] - ReleaseDate +### Changed + +- Improve UX of query text box + - The query is now auto-applied when changed (with a 500ms debounce), and drops focus on the text box when Enter is pressed + ## [2.3.0] - 2024-11-11 ### Added diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 299d92f6..8a5c5dbf 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -262,14 +262,14 @@ impl Tui { )?; } - Message::EditFile { path, on_complete } => { + Message::FileEdit { path, on_complete } => { let command = get_editor_command(&path)?; self.run_command(command)?; on_complete(path); // The callback may queue an event to read the file, so we can't // delete it yet. Caller is responsible for cleaning up } - Message::ViewFile { path } => { + Message::FileView { path } => { let command = get_viewer_command(&path)?; self.run_command(command)?; // We don't need to read the contents back so we can clean up @@ -316,6 +316,8 @@ impl Tui { self.view.handle_input(event, action); } + Message::Local(event) => self.view.local(event), + Message::Notify(message) => self.view.notify(message), Message::PromptStart(prompt) => { self.view.open_modal(prompt); diff --git a/crates/tui/src/message.rs b/crates/tui/src/message.rs index 988e5fdb..5382fc1d 100644 --- a/crates/tui/src/message.rs +++ b/crates/tui/src/message.rs @@ -1,7 +1,7 @@ //! Async message passing! This is how inputs and other external events trigger //! state updates. -use crate::view::Confirm; +use crate::view::{Confirm, LocalEvent}; use anyhow::Context; use derive_more::From; use slumber_config::Action; @@ -14,7 +14,7 @@ use slumber_core::{ template::{Prompt, Prompter, Select, Template, TemplateChunk}, util::ResultTraced, }; -use std::{path::PathBuf, sync::Arc}; +use std::{fmt::Debug, path::PathBuf, sync::Arc}; use tokio::sync::mpsc::UnboundedSender; use tracing::trace; @@ -78,8 +78,11 @@ pub enum Message { /// Copy some text to the clipboard CopyText(String), + /// An error occurred in some async process and should be shown to the user + Error { error: anyhow::Error }, + /// Open a file in the user's external editor - EditFile { + FileEdit { path: PathBuf, /// Function to call once the edit is done. The original path will be /// passed back @@ -87,10 +90,7 @@ pub enum Message { on_complete: Callback, }, /// Open a file to be viewed in the user's external viewer - ViewFile { path: PathBuf }, - - /// An error occurred in some async process and should be shown to the user - Error { error: anyhow::Error }, + FileView { path: PathBuf }, /// Launch an HTTP request from the given recipe/profile. HttpBeginRequest(RequestConfig), @@ -113,6 +113,9 @@ pub enum Message { action: Option, }, + /// Trigger a localized UI event + Local(Box), + /// Send an informational notification to the user Notify(String), diff --git a/crates/tui/src/view.rs b/crates/tui/src/view.rs index 88424563..07cc30d8 100644 --- a/crates/tui/src/view.rs +++ b/crates/tui/src/view.rs @@ -12,6 +12,7 @@ mod util; pub use common::modal::{IntoModal, ModalPriority}; pub use context::{UpdateContext, ViewContext}; +pub use event::LocalEvent; pub use styles::Styles; pub use util::{Confirm, PreviewPrompter}; @@ -141,6 +142,11 @@ impl View { ViewContext::push_event(Event::Notify(notification)); } + /// Trigger a localized UI event + pub fn local(&mut self, event: Box) { + ViewContext::push_event(Event::Local(event)); + } + /// Queue an event to update the view according to an input event from the /// user. If possible, a bound action is provided which tells us what /// abstract action the input maps to. diff --git a/crates/tui/src/view/common/text_box.rs b/crates/tui/src/view/common/text_box.rs index 3ea9f818..4671d07e 100644 --- a/crates/tui/src/view/common/text_box.rs +++ b/crates/tui/src/view/common/text_box.rs @@ -2,10 +2,13 @@ use crate::{ context::TuiContext, + message::Message, view::{ context::UpdateContext, draw::{Draw, DrawMetadata}, event::{Event, EventHandler, Update}, + util::Debounce, + ViewContext, }, }; use crossterm::event::{KeyCode, KeyModifiers}; @@ -17,6 +20,9 @@ use ratatui::{ Frame, }; use slumber_config::Action; +use std::time::Duration; + +const DEBOUNCE: Duration = Duration::from_millis(500); /// Single line text submission component #[derive(derive_more::Debug, Default)] @@ -28,12 +34,17 @@ pub struct TextBox { #[debug(skip)] validator: Option, + // State state: TextState, + on_change_debounce: Option, // Callbacks /// Called when user clicks to start editing #[debug(skip)] on_click: Option, + /// Called when user changes the text content. Can optionally be debounced + #[debug(skip)] + on_change: Option, /// Called when user exits with submission (e.g. Enter) #[debug(skip)] on_submit: Option, @@ -76,20 +87,31 @@ impl TextBox { self } /// Set the callback to be called when the user clicks the textbox - pub fn on_click(mut self, on_click: impl 'static + Fn()) -> Self { - self.on_click = Some(Box::new(on_click)); + pub fn on_click(mut self, f: impl 'static + Fn()) -> Self { + self.on_click = Some(Box::new(f)); + self + } + + /// Set the callback to be called when the user changes the text value. + /// Callback can optionally be debounced, so it isn't called repeatedly + /// while the user is typing + pub fn on_change(mut self, f: impl 'static + Fn(), debounce: bool) -> Self { + self.on_change = Some(Box::new(f)); + if debounce { + self.on_change_debounce = Some(Debounce::new(DEBOUNCE)); + } self } /// Set the callback to be called when the user hits escape - pub fn on_cancel(mut self, on_cancel: impl 'static + Fn()) -> Self { - self.on_cancel = Some(Box::new(on_cancel)); + pub fn on_cancel(mut self, f: impl 'static + Fn()) -> Self { + self.on_cancel = Some(Box::new(f)); self } /// Set the callback to be called when the user hits enter - pub fn on_submit(mut self, on_submit: impl 'static + Fn()) -> Self { - self.on_submit = Some(Box::new(on_submit)); + pub fn on_submit(mut self, f: impl 'static + Fn()) -> Self { + self.on_submit = Some(Box::new(f)); self } @@ -121,6 +143,70 @@ impl TextBox { .unwrap_or(true) } + /// Handle a key input event, to modify text state. Return `true` if the + /// event was handled, `false` if it should be propagated + fn handle_key(&mut self, code: KeyCode, modifiers: KeyModifiers) -> bool { + let text_changed = match code { + // Don't handle keystrokes if the user is holding a modifier + KeyCode::Char(c) + if (modifiers - KeyModifiers::SHIFT).is_empty() => + { + self.state.insert(c); + true + } + KeyCode::Backspace => self.state.delete_left(), + KeyCode::Delete => self.state.delete_right(), + KeyCode::Left => { + if modifiers == KeyModifiers::CONTROL { + self.state.home(); + } else { + self.state.left(); + } + false + } + KeyCode::Right => { + if modifiers == KeyModifiers::CONTROL { + self.state.end(); + } else { + self.state.right(); + } + false + } + KeyCode::Home => { + self.state.home(); + false + } + KeyCode::End => { + self.state.end(); + false + } + _ => return false, // Event should be propagated + }; + // If text _content_ changed, trigger the auto-submit debounce + if text_changed { + self.change(); + } + true // We DID handle this event + } + + /// Call parent's on_change callback. Should be called whenever text + /// _content_ is changed + fn change(&mut self) { + if let Some(debounce) = &self.on_change_debounce { + let messages_tx = ViewContext::messages_tx(); + // WARNING: There is a bug here. If there are multiple text boxes + // active on the screen, there's no guarantee this will go to the + // right one. Fortunately that UX doesn't really make sense. This + // can be truly fixed with unique target IDs for local events, but + // that's a much bigger refactor. For now it should be fine. + debounce.start(move || { + messages_tx.send(Message::Local(Box::new(ChangeEvent))) + }); + } else { + call(&self.on_change); + } + } + /// Call parent's submission callback fn submit(&mut self) { if self.is_valid() { @@ -132,6 +218,11 @@ impl TextBox { impl EventHandler for TextBox { fn update(&mut self, _: &mut UpdateContext, event: Event) -> Update { match event { + Event::Local(local) + if local.downcast_ref::().is_some() => + { + call(&self.on_change) + } Event::Input { action: Some(Action::Submit), .. @@ -147,35 +238,12 @@ impl EventHandler for TextBox { Event::Input { event: crossterm::event::Event::Key(key_event), .. - } => match key_event.code { - // Don't handle keystrokes if the user is holding a modifier - KeyCode::Char(c) - if (key_event.modifiers - KeyModifiers::SHIFT) - .is_empty() => - { - self.state.insert(c) - } - KeyCode::Backspace => self.state.delete_left(), - KeyCode::Delete => self.state.delete_right(), - KeyCode::Left => { - if key_event.modifiers == KeyModifiers::CONTROL { - self.state.home(); - } else { - self.state.left(); - } + } => { + if !self.handle_key(key_event.code, key_event.modifiers) { + // Propagate any keystrokes we don't handle (e.g. f keys) + return Update::Propagate(event); } - KeyCode::Right => { - if key_event.modifiers == KeyModifiers::CONTROL { - self.state.end(); - } else { - self.state.right(); - } - } - KeyCode::Home => self.state.home(), - KeyCode::End => self.state.end(), - // Propagate any keystrokes we don't handle (e.g. f keys) - _ => return Update::Propagate(event), - }, + } _ => return Update::Propagate(event), } Update::Consumed @@ -294,18 +362,26 @@ impl TextState { } } - /// Delete character immediately left of the cursor - fn delete_left(&mut self) { + /// Delete character immediately left of the cursor. Return `true` if text + /// was modified + fn delete_left(&mut self) -> bool { if !self.is_at_home() { self.left(); self.text.remove(self.cursor); + true + } else { + false } } - /// Delete character immediately rightof the cursor - fn delete_right(&mut self) { + /// Delete character immediately rightof the cursor. Return `true` if text + /// was modified + fn delete_right(&mut self) -> bool { if !self.is_at_end() { self.text.remove(self.cursor); + true + } else { + false } } @@ -327,6 +403,10 @@ impl PersistedContainer for TextBox { } } +/// Local event for triggering debounced on_change calls +#[derive(Debug)] +struct ChangeEvent; + /// Call a callback if defined fn call(f: &Option) { if let Some(f) = f { @@ -398,6 +478,7 @@ mod tests { #[with(10, 1)] terminal: TestTerminal, ) { let click_count = Counter::default(); + let change_count = Counter::default(); let submit_count = Counter::default(); let cancel_count = Counter::default(); let mut component = TestComponent::new( @@ -405,6 +486,7 @@ mod tests { &terminal, TextBox::default() .on_click(click_count.callback()) + .on_change(change_count.callback(), false) .on_submit(submit_count.callback()) .on_cancel(cancel_count.callback()), (), @@ -444,6 +526,8 @@ mod tests { component.click(0, 0).assert_empty(); assert_eq!(click_count, 1); + assert_eq!(change_count, 7); + component.send_key(KeyCode::Enter).assert_empty(); assert_eq!(submit_count, 1); @@ -451,6 +535,37 @@ mod tests { assert_eq!(cancel_count, 1); } + /// Test on_change debouncing + #[rstest] + #[tokio::test] + async fn test_debounce( + mut harness: TestHarness, + #[with(10, 1)] terminal: TestTerminal, + ) { + let change_count = Counter::default(); + let mut component = TestComponent::new( + &harness, + &terminal, + TextBox::default().on_change(change_count.callback(), true), + (), + ); + + // Type some text + component.send_text("hello!").assert_empty(); + // on_change isn't called immediately + assert_eq!(change_count, 0); + + // It gets called after waiting + let event = assert_matches!( + harness.pop_message_wait().await, + Message::Local(event) => event, + ); + // We have to feed the event from the message channel to the component + // manually. This is normally done by the main loop + component.update_draw(Event::Local(event)).assert_empty(); + assert_eq!(change_count, 1); + } + /// Test text navigation and deleting. [TextState] has its own tests so /// we're mostly just testing that keys are mapped correctly #[rstest] diff --git a/crates/tui/src/view/component/queryable_body.rs b/crates/tui/src/view/component/queryable_body.rs index 4c5be0a0..928c2b17 100644 --- a/crates/tui/src/view/component/queryable_body.rs +++ b/crates/tui/src/view/component/queryable_body.rs @@ -86,19 +86,17 @@ impl QueryableBody { let input_engine = &TuiContext::get().input_engine; let binding = input_engine.binding_display(Action::Search); + let send_local = |callback| { + move || ViewContext::push_event(Event::new_local(callback)) + }; let text_box = TextBox::default() .placeholder(format!("'{binding}' to filter body with JSONPath")) .validator(|text| JsonPath::parse(text).is_ok()) // Callback trigger an events, so we can modify our own state - .on_click(|| { - ViewContext::push_event(Event::new_local(QueryCallback::Focus)) - }) - .on_cancel(|| { - ViewContext::push_event(Event::new_local(QueryCallback::Cancel)) - }) - .on_submit(|| { - ViewContext::push_event(Event::new_local(QueryCallback::Submit)) - }); + .on_click(send_local(QueryCallback::Focus)) + .on_change(send_local(QueryCallback::Change), true) + .on_cancel(send_local(QueryCallback::Cancel)) + .on_submit(send_local(QueryCallback::Submit)); Self { state: Default::default(), query_available: Cell::new(false), @@ -134,6 +132,19 @@ impl QueryableBody { .get() .map(|state| Ref::map(state, |state| &*state.text)) } + + fn update_query(&mut self) { + let text = self.query_text_box.data().text(); + self.query = if text.is_empty() { + None + } else { + text.parse() + // Log the error, then throw it away + .with_context(|| format!("Error parsing query {text:?}")) + .traced() + .ok() + }; + } } impl Default for QueryableBody { @@ -151,6 +162,7 @@ impl EventHandler for QueryableBody { } else if let Some(callback) = event.local::() { match callback { QueryCallback::Focus => self.query_focused = true, + QueryCallback::Change => self.update_query(), QueryCallback::Cancel => { // Reset text to whatever was submitted last self.query_text_box.data_mut().set_text( @@ -162,18 +174,8 @@ impl EventHandler for QueryableBody { self.query_focused = false; } QueryCallback::Submit => { - let text = self.query_text_box.data().text(); - self.query = if text.is_empty() { - None - } else { - text.parse() - // Log the error, then throw it away - .with_context(|| { - format!("Error parsing query {text:?}") - }) - .traced() - .ok() - }; + self.update_query(); + self.query_focused = false; } } } else { @@ -251,9 +253,10 @@ impl PersistedContainer for QueryableBody { } /// All callback events from the query text box -#[derive(Debug)] +#[derive(Copy, Clone, Debug)] enum QueryCallback { Focus, + Change, Cancel, Submit, } @@ -396,7 +399,8 @@ mod tests { /// Render a parsed body with query text box #[rstest] - fn test_parsed( + #[tokio::test] + async fn test_parsed( harness: TestHarness, #[with(32, 5)] terminal: TestTerminal, json_response: ResponseRecord, @@ -440,14 +444,16 @@ mod tests { let data = component.data(); assert_eq!(data.query, Some("$.greeting".parse().unwrap())); assert_eq!(data.parsed_text().as_deref(), Some("[\n \"hello\"\n]")); - assert!(data.query_focused); // Still focused + assert!(!data.query_focused); // Cancelling out of the text box should reset the query value + component.send_key(KeyCode::Char('/')).assert_empty(); component.send_text("more text").assert_empty(); component.send_key(KeyCode::Esc).assert_empty(); let data = component.data(); assert_eq!(data.query, Some("$.greeting".parse().unwrap())); assert_eq!(data.query_text_box.data().text(), "$.greeting"); + assert!(!data.query_focused); // Check the view again terminal.assert_buffer_lines([ diff --git a/crates/tui/src/view/component/recipe_pane/body.rs b/crates/tui/src/view/component/recipe_pane/body.rs index d25ae94c..55662bbd 100644 --- a/crates/tui/src/view/component/recipe_pane/body.rs +++ b/crates/tui/src/view/component/recipe_pane/body.rs @@ -164,7 +164,7 @@ impl RawBody { return; }; - ViewContext::send_message(Message::EditFile { + ViewContext::send_message(Message::FileEdit { path, on_complete: Box::new(|path| { ViewContext::push_event(Event::new_local(SaveBodyOverride( @@ -317,7 +317,7 @@ mod tests { component.send_key(KeyCode::Char('e')).assert_empty(); let (path, on_complete) = assert_matches!( harness.pop_message_now(), - Message::EditFile { path, on_complete } => (path, on_complete), + Message::FileEdit { path, on_complete } => (path, on_complete), ); assert_eq!(fs::read(&path).unwrap(), b"hello!"); diff --git a/crates/tui/src/view/util.rs b/crates/tui/src/view/util.rs index b581e524..e27145b9 100644 --- a/crates/tui/src/view/util.rs +++ b/crates/tui/src/view/util.rs @@ -14,7 +14,8 @@ use slumber_core::{ template::{Prompt, PromptChannel, Prompter, Select}, util::ResultTraced, }; -use std::{io::Write, path::Path}; +use std::{io::Write, path::Path, time::Duration}; +use tokio::{select, sync::broadcast, time}; /// A data structure for representation a yes/no confirmation. This is similar /// to [Prompt], but it only asks a yes/no question. @@ -41,6 +42,46 @@ impl Prompter for PreviewPrompter { } } +/// Utility for debouncing repeated calls to a callback +#[derive(Debug)] +pub struct Debounce { + duration: Duration, + /// Broadcast channel to send on when previous tasks should be cancelled + cancel_send: broadcast::Sender<()>, +} + +impl Debounce { + pub fn new(duration: Duration) -> Self { + let (cancel_send, _) = broadcast::channel(1); + Self { + duration, + cancel_send, + } + } + + /// Trigger a debounced callback. The given callback will be invoked after + /// the debounce period _if_ this method is not called again during the + /// debounce period. + pub fn start(&self, on_complete: impl 'static + Fn() + Send + Sync) { + // Cancel existing tasks, _then_ start a new listener, so we don't + // cancel ourselves. An error on the send just means there are no + // listeners, so we can ignore it + let _ = self.cancel_send.send(()); + let mut cancel_recv = self.cancel_send.subscribe(); + + let duration = self.duration; + tokio::spawn(async move { + // Start a timer. If it expires before cancellation, then submit + select! { + _ = time::sleep(duration) => { + on_complete() + }, + _ = cancel_recv.recv() => {} + }; + }); + } +} + /// Created a rectangle centered on the given `Rect`. pub fn centered_rect( width: Constraint, @@ -110,7 +151,7 @@ pub fn view_text(text: &Text) { .with_context(|| format!("Error writing to file {path:?}")) .traced(); match result { - Ok(()) => ViewContext::send_message(Message::ViewFile { path }), + Ok(()) => ViewContext::send_message(Message::FileView { path }), Err(error) => ViewContext::send_message(Message::Error { error }), } }