diff --git a/AGENTS.md b/AGENTS.md index eb4dea8..0423bc6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,10 +71,12 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - A menu goes through `menus::menu`: rows, submenus, keys, focus and presses outside live there, for DropdownMenu, OverflowMenu, SplitButton, ContextMenu, SearchableMenu and MenuBar. A press on the host's own box is the host's to handle, so a trigger toggles and a right click moves a context menu without a close and reopen in one event. - gpui focuses the nearest focusable ancestor on mouse down, and apps wrap everything in a `FocusScope`. A press that must leave focus alone calls `prevent_default`, as buttons and context regions do. - gpui draws a window's first frame inside `open_window`. State the first frame reads, such as `cx.set_menus`, is set before it opens. -- gpui clicks a focused element when Enter or Space is released. An overlay that hands focus back picks on release too; a pick on press returns focus first, and the release clicks the opener again. An edit in place that hands focus back keeps its text on Enter's release (`forms::InlineEdit`). +- gpui clicks a focused element when Enter or Space is released. An overlay that hands focus back picks on release too; a pick on press returns focus first, and the release clicks the opener again. An edit in place that hands focus back keeps its text on Enter's release (`forms::InlineEdit`). The press itself bubbles, so a container that takes Space leaves a focused button its own: `overlays::media_button` stops it. - A focus handle a component hands back lives across all its modes: take it with `tab_stop` before any early return, or keyed state drops it while the other mode draws and the handed-back focus lands nowhere. - A dialog goes through `overlays::Dialog`: scrim, focus trap, Escape and focus return live there. AlertDialog, ConfirmDialog and PromptDialog build on it; a fullscreen dialog fills the window below its title bar. A dialog's card fits the window, a title bar's height clear at each end; its body scrolls and its actions stay. - Controls on a dark layer over pictures are `overlays::media_button`, light icons that dim when they have nowhere to go, for the lightbox and a video player's bar alike. +- A component's letter keys step aside while Command or Control is held, so the app's shortcuts reach the app (`media::ImageAnnotator`, `VideoPlayer`, `VideoThumbnailStrip`). +- A tip over a track splits the free width around it by the pointer's share: it stays inside the track and under the pointer, and nothing is measured (`media::Scrubber`). - `primitives::Severity` names info, success, warning and danger with their colors and icons, for alerts and feedback alike. - Rows that move are keyed. `motion::Flip` places them absolutely from last frame's heights, so no frame shows a row at its new place before it glides there, and drives the glide from elapsed time: an animation id that changes per move would rebuild the row's keyed state. `Reorder` holds a row by key, drags with an empty ghost and moves the held row itself. - A marker that slides between items (segment thumb, tab line) measures them with `motion::slide` and eases with `glide`. diff --git a/TASKS.md b/TASKS.md index d2c04d6..08836a4 100644 --- a/TASKS.md +++ b/TASKS.md @@ -311,3 +311,4 @@ Per-component lines live in `tasks/`. Tags there: | T26a | 2 | PASS | | | T26b | 1 | FAIL | a shaped crop's corner ignored the pointer's move up or down | | T26b | 2 | PASS | | +| T26c | 1 | FAIL | the caption faded with the controls; Space on a focused bar button acted twice; Command letters fired the player and the strip; a long caption, cue words and the scrubber's tip ran past their boxes at 280px | diff --git a/src/forms/inline.rs b/src/forms/inline.rs index d7a5157..4b77b63 100644 --- a/src/forms/inline.rs +++ b/src/forms/inline.rs @@ -10,6 +10,7 @@ use super::{Input, InputEvent, TextInput}; use crate::{ primitives::{FocusRing, Icon, IconName, tab_stop}, theme::{ActiveTheme, ControlSize, IconSize, Radius}, + typography::Ellipsis, }; pub(crate) type OnCommit = Rc; @@ -155,6 +156,15 @@ impl RenderOnce for InlineEdit { let empty = self.value.is_empty(); let (value, start, back) = (self.value.clone(), state, focus.clone()); let group = SharedString::from(format!("inline-edit-{:?}", self.id)); + let label = div() + .debug_selector(|| format!("inline-edit {}", self.id)) + .flex_1() + .min_w_0() + .child(Ellipsis::new(if empty { + self.placeholder + } else { + self.value + })); div() .id(self.id) .group(group.clone()) @@ -177,9 +187,10 @@ impl RenderOnce for InlineEdit { start.update(cx, |editing, _| editing.returning(back.clone())); Editing::begin(&start, value.to_string(), window, cx); }) - .child(if empty { self.placeholder } else { self.value }) + .child(label) .child( div() + .flex_none() .invisible() .group_hover(group, |style| style.visible()) .child( diff --git a/src/media/player.rs b/src/media/player.rs index 81e7672..661e4bd 100644 --- a/src/media/player.rs +++ b/src/media/player.rs @@ -146,7 +146,11 @@ impl RenderOnce for VideoPlayer { moved: started, waking: false, }); - let focus = tab_stop((self.id.clone(), "focus").into(), true, window, cx); + let keys = self.on_play.is_some() + || self.on_seek.is_some() + || self.on_fullscreen.is_some() + || (self.on_captions.is_some() && self.captions.is_some()); + let focus = tab_stop((self.id.clone(), "focus").into(), keys, window, cx); let now = cx.background_executor().now(); let resting = self.playing && now.duration_since(watch.read(cx).moved) >= IDLE @@ -166,20 +170,29 @@ impl RenderOnce for VideoPlayer { cx, ) }; - let play = self.on_play.clone().map(|play| { + let toggle: Option = self.on_play.clone().map(|play| { + Rc::new(move |window: &mut Window, cx: &mut App| { + log::info!("video player: {}", if playing { "pause" } else { "play" }); + play(!playing, window, cx) + }) as Run + }); + let turn: Option = self + .on_captions + .clone() + .zip(self.captions) + .map(|(set, on)| { + Rc::new(move |window: &mut Window, cx: &mut App| { + log::info!("video player: captions {}", if on { "off" } else { "on" }); + set(!on, window, cx) + }) as Run + }); + let play = toggle.clone().map(|run| { let icon = if playing { IconName::Pause } else { IconName::Play }; - button( - "play", - icon, - Rc::new(move |window, cx| { - log::info!("video player: {}", if playing { "pause" } else { "play" }); - play(!playing, window, cx) - }), - ) + button("play", icon, run) }); let speed_button = self.on_speed.clone().map(|set| { let next = SPEEDS[(SPEEDS @@ -206,22 +219,14 @@ impl RenderOnce for VideoPlayer { .child(format!("{speed}×")), ) }); - let captions = self - .on_captions - .clone() - .zip(self.captions) - .map(|(set, on)| { - let icon = if on { - IconName::Captions - } else { - IconName::CaptionsOff - }; - button( - "captions", - icon, - Rc::new(move |window, cx| set(!on, window, cx)), - ) - }); + let captions = turn.clone().map(|run| { + let icon = if self.captions == Some(true) { + IconName::Captions + } else { + IconName::CaptionsOff + }; + button("captions", icon, run) + }); let picture = self .on_picture .clone() @@ -274,6 +279,8 @@ impl RenderOnce for VideoPlayer { let caption = self.caption.map(|words| { div().flex().justify_center().child( div() + .debug_selector(|| "video-caption".into()) + .min_w_0() .max_w(theme.prose_width()) .px_2() .py_0p5() @@ -284,10 +291,22 @@ impl RenderOnce for VideoPlayer { .child(words), ) }); + let bar = bar.when(resting, |bar| { + bar.debug_selector(|| "video-controls-resting".into()) + }); + let (from, to) = if resting { (1.0, 0.0) } else { (0.0, 1.0) }; + let bar = if changes == 0 { + bar.opacity(to).into_any_element() + } else { + let fade = motion_duration(crate::motion::BASE, cx); + bar.with_animation( + (self.id.clone(), format!("controls-{changes}")), + Animation::new(fade), + move |bar, t| bar.opacity(crate::motion::lerp(from, to, t)), + ) + .into_any_element() + }; let controls = div() - .when(resting, |controls| { - controls.debug_selector(|| "video-controls-resting".into()) - }) .absolute() .left_0() .right_0() @@ -298,28 +317,14 @@ impl RenderOnce for VideoPlayer { .gap_2() .children(caption) .child(bar); - let (from, to) = if resting { (1.0, 0.0) } else { (0.0, 1.0) }; - let controls = if changes == 0 { - controls.opacity(to).into_any_element() - } else { - let fade = motion_duration(crate::motion::BASE, cx); - controls - .with_animation( - ("controls", changes), - Animation::new(fade), - move |controls, t| controls.opacity(crate::motion::lerp(from, to, t)), - ) - .into_any_element() - }; let frame = self .frame .map(|frame| img(frame).id((self.id.clone(), "frame")).size_full()); let moved = watch.clone(); let keyed = ( - self.on_play.clone(), + toggle, self.on_seek.clone(), - self.on_captions.clone(), - self.captions, + turn, self.on_fullscreen.clone(), ); framed(self.ratio, cx) @@ -328,8 +333,7 @@ impl RenderOnce for VideoPlayer { .bg(colors.media_backdrop) .border_1() .border_color(transparent_black()) - .track_focus(&focus) - .focus_ring(cx) + .when(keys, |stage| stage.track_focus(&focus).focus_ring(cx)) .children(frame) .child(controls) .on_mouse_move(move |_, window, cx| { @@ -358,11 +362,15 @@ impl RenderOnce for VideoPlayer { cx.notify(); }) }) - .detach(); + .detach_and_log_err(cx); } }) .on_key_down(move |event, window, cx| { - let (play, seek, captions, on, fullscreen) = &keyed; + let held = &event.keystroke.modifiers; + if held.platform || held.control { + return; + } + let (play, seek, captions, fullscreen) = &keyed; let step = |by: f64| { Duration::from_secs_f64( (at.as_secs_f64() + by).clamp(0.0, length.as_secs_f64()), @@ -370,7 +378,7 @@ impl RenderOnce for VideoPlayer { }; match event.keystroke.key.as_str() { "space" | "k" => match play { - Some(play) => play(!playing, window, cx), + Some(play) => play(window, cx), None => return, }, "j" | "l" | "left" | "right" => { @@ -385,9 +393,9 @@ impl RenderOnce for VideoPlayer { }; seek(step(by), window, cx) } - "c" => match (captions, on) { - (Some(set), Some(on)) => set(!on, window, cx), - _ => return, + "c" => match captions { + Some(turn) => turn(window, cx), + None => return, }, "f" => match fullscreen { Some(run) => run(window, cx), diff --git a/src/media/scrubber.rs b/src/media/scrubber.rs index bcf7b57..7ef9e83 100644 --- a/src/media/scrubber.rs +++ b/src/media/scrubber.rs @@ -12,6 +12,7 @@ use crate::{ primitives::{FocusRing, tab_stop}, theme::{ActiveTheme, Radius, TextSize}, typography::{ + Ellipsis, format::{DurationStyle, duration}, tabular, }, @@ -180,27 +181,29 @@ impl RenderOnce for Scrubber { Some(name) => format!("{} · {name}", clock(time)), None => clock(time), }; + let side = |share: f32| div().flex_basis(relative(share)); div() .absolute() - .left(relative(hovered)) + .left_0() .bottom_full() - .size_0() + .w_full() .flex() - .justify_center() .items_end() + .child(side(hovered)) .child( tabular(div()) .flex_none() + .max_w_full() .mb_1() .px_1p5() .rounded(theme.radius(Radius::Sm)) .bg(colors.tooltip_bg) .text_color(colors.tooltip_fg) .text_size(theme.text_size(TextSize::Xs)) - .whitespace_nowrap() - .child(text), + .debug_selector(|| "scrubber-tip".into()) + .child(Ellipsis::new(text)), ) - .debug_selector(|| "scrubber-tip".into()) + .child(side(1.0 - hovered)) }); let knob = (hover.is_some() || focused).then(|| { div() diff --git a/src/media/strip.rs b/src/media/strip.rs index addc52d..0098e4d 100644 --- a/src/media/strip.rs +++ b/src/media/strip.rs @@ -290,6 +290,10 @@ impl RenderOnce for VideoThumbnailStrip { ); }) .on_key_down(move |event, window, cx| { + let command = &event.keystroke.modifiers; + if command.platform || command.control { + return; + } let (held, time) = match event.keystroke.key.as_str() { "left" => (None, at.saturating_sub(span)), "right" => (None, (at + span).min(length)), diff --git a/src/media/subtitles.rs b/src/media/subtitles.rs index 2527740..d6622b8 100644 --- a/src/media/subtitles.rs +++ b/src/media/subtitles.rs @@ -90,7 +90,7 @@ pub(crate) fn retimed(cues: &[Cue], key: &str, edge: Edge, text: &str) -> Result Edge::End => cue.end = time, } if cue.start >= cue.end { - return Err("A cue ends after it starts".to_string()); + return Err("A cue must end after it starts".to_string()); } next.sort_by_key(|cue| cue.start); Ok(next) @@ -356,7 +356,7 @@ mod tests { assert!(retimed(&cues, "a", Edge::End, "1:x").is_err()); assert_eq!( retimed(&cues, "a", Edge::End, "00:00.500"), - Err("A cue ends after it starts".to_string()) + Err("A cue must end after it starts".to_string()) ); } } diff --git a/src/media/tests/video.rs b/src/media/tests/video.rs index 1d9d6fe..e5dc470 100644 --- a/src/media/tests/video.rs +++ b/src/media/tests/video.rs @@ -1,8 +1,8 @@ use std::{path::PathBuf, time::Duration}; use gpui::{ - Context, Entity, IntoElement, Modifiers, MouseButton, ParentElement, Pixels, Point, Render, - Styled, TestAppContext, VisualTestContext, Window, div, point, px, + Context, Entity, InteractiveElement, IntoElement, Modifiers, MouseButton, ParentElement, + Pixels, Point, Render, Styled, TestAppContext, VisualTestContext, Window, div, point, px, }; use super::{picture, press, settle, setup}; @@ -89,21 +89,45 @@ fn the_pointer_shows_its_time_over_the_track(cx: &mut TestAppContext) { let tip = cx .debug_bounds("scrubber-tip") .expect("a tip over the track"); - assert!((tip.left() - half.x).abs() < px(1.0), "{tip:?}"); + assert!( + (tip.center().x - half.x).abs() < px(1.0), + "centered on the pointer: {tip:?}" + ); } -/// A player of a hundred seconds, what it was told, and whether it plays. +#[gpui::test] +fn the_tip_stays_over_the_track_at_its_ends(cx: &mut TestAppContext) { + let (_, cx) = scrubbing(cx); + for share in [0.0, 1.0] { + let at = across("scrubber-rail", share, cx); + cx.simulate_mouse_move(at, None, Modifiers::none()); + settle(cx); + settle(cx); + let tip = cx + .debug_bounds("scrubber-tip") + .expect("a tip over the track"); + let rail = cx.debug_bounds("scrubber-rail").expect("the rail draws"); + let inside = tip.left() >= rail.left() - px(0.5) && tip.right() <= rail.right() + px(0.5); + assert!(inside, "at {share}: {tip:?} over {rail:?}"); + } +} + +/// A player of a hundred seconds, this wide, with this caption, and what it was told. struct Watching { path: PathBuf, + width: f32, + caption: &'static str, at: Duration, playing: bool, + plays: Vec, captions: bool, + fullscreens: usize, } impl Render for Watching { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - let (play, seek, caption) = (cx.entity(), cx.entity(), cx.entity()); - div().w(px(480.0)).child( + let (play, seek, caption, full) = (cx.entity(), cx.entity(), cx.entity(), cx.entity()); + div().w(px(self.width)).child( VideoPlayer::new( "player", Some(self.path.clone().into()), @@ -112,10 +136,11 @@ impl Render for Watching { self.at, ) .playing(self.playing) - .captions(self.captions, Some("Light falls on the stair".into())) + .captions(self.captions, Some(self.caption.into())) .on_play(move |on, _, cx| { play.update(cx, |view, cx| { view.playing = on; + view.plays.push(on); cx.notify(); }) }) @@ -130,19 +155,34 @@ impl Render for Watching { view.captions = on; cx.notify(); }) + }) + .on_fullscreen(move |_, cx| { + full.update(cx, |view, cx| { + view.fullscreens += 1; + cx.notify(); + }) }), ) } } -fn watching(cx: &mut TestAppContext) -> (Entity, &mut VisualTestContext) { +fn watching_at<'a>( + width: f32, + caption: &'static str, + captions: bool, + cx: &'a mut TestAppContext, +) -> (Entity, &'a mut VisualTestContext) { setup(cx); let path = picture("frame", 160, 100); let (view, cx) = cx.add_window_view(|_, _| Watching { path, + width, + caption, at: SECOND * 30, playing: false, - captions: false, + plays: Vec::new(), + captions, + fullscreens: 0, }); cx.update(|window, _| window.activate_window()); settle(cx); @@ -152,6 +192,81 @@ fn watching(cx: &mut TestAppContext) -> (Entity, &mut VisualTestContex (view, cx) } +fn watching(cx: &mut TestAppContext) -> (Entity, &mut VisualTestContext) { + watching_at(480.0, "Light falls on the stair", false, cx) +} + +#[gpui::test] +fn space_on_a_focused_bar_button_presses_it_once(cx: &mut TestAppContext) { + let (view, cx) = watching(cx); + cx.update(|window, _| { + window.blur(); + for _ in 0..3 { + window.focus_next(); + } + }); + press("space", cx); + assert_eq!(view.read_with(cx, |view, _| view.plays.clone()), [true]); +} + +#[gpui::test] +fn command_letters_leave_the_player_to_the_app(cx: &mut TestAppContext) { + let (view, cx) = watching(cx); + for key in ["cmd-f", "cmd-c", "cmd-k", "ctrl-f"] { + press(key, cx); + } + let heard = view.read_with(cx, |view, _| { + (view.fullscreens, view.captions, view.plays.clone()) + }); + assert_eq!(heard, (0, false, vec![])); +} + +#[gpui::test] +fn a_long_caption_wraps_inside_a_narrow_player(cx: &mut TestAppContext) { + let (view, cx) = watching_at(280.0, "Light falls", true, cx); + let line = cx.debug_bounds("video-caption").expect("the caption draws"); + view.update(cx, |view, cx| { + view.caption = "The shadow walks the length of the hall while the morning comes in."; + cx.notify(); + }); + settle(cx); + let (chip, frame) = ( + cx.debug_bounds("video-caption").expect("the caption draws"), + cx.debug_bounds("video-player").expect("the player draws"), + ); + assert!( + chip.left() >= frame.left() && chip.right() <= frame.right(), + "{chip:?} in {frame:?}" + ); + assert!( + chip.size.height > line.size.height * 1.5, + "{chip:?} wraps past {line:?}" + ); +} + +#[gpui::test] +fn a_caption_stays_while_the_controls_rest(cx: &mut TestAppContext) { + let (view, cx) = watching_at(480.0, "Light falls on the stair", true, cx); + cx.update(|window, _| window.blur()); + view.update(cx, |view, cx| { + view.playing = true; + cx.notify(); + }); + let frame = cx.debug_bounds("video-player").expect("the player draws"); + cx.simulate_mouse_move(frame.center(), None, Modifiers::none()); + settle(cx); + cx.executor().advance_clock(Duration::from_millis(2600)); + cx.run_until_parked(); + let faded = cx + .debug_bounds("video-controls-resting") + .expect("the bar rests"); + let caption = cx.debug_bounds("video-caption").expect("the caption draws"); + assert!( + caption.bottom() <= faded.top(), + "{caption:?} outside {faded:?}" + ); +} + #[gpui::test] fn space_plays_l_and_j_jump_ten_seconds_and_c_turns_captions(cx: &mut TestAppContext) { let (view, cx) = watching(cx); @@ -252,6 +367,19 @@ fn a_press_seeks_and_a_handle_drag_moves_its_end_to_the_length(cx: &mut TestAppC ); } +#[gpui::test] +fn command_o_leaves_the_trim_to_the_app(cx: &mut TestAppContext) { + let (view, cx) = trimming(cx); + let press_at = across("video-strip", 0.3, cx); + cx.simulate_click(press_at, Modifiers::none()); + settle(cx); + press("cmd-o", cx); + assert_eq!( + view.read_with(cx, |view, _| view.trim), + (SECOND * 10, SECOND * 50) + ); +} + #[gpui::test] fn i_and_o_set_the_trim_at_the_playhead(cx: &mut TestAppContext) { let (view, cx) = trimming(cx); @@ -273,35 +401,48 @@ fn i_and_o_set_the_trim_at_the_playhead(cx: &mut TestAppContext) { struct Captioning { cues: Vec, at: Duration, + width: f32, } impl Render for Captioning { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { let owner = cx.entity(); - div().w(px(420.0)).child( - SubtitleEditor::new("subs", self.cues.clone(), self.at).on_change( - move |cues, _, cx| { - owner.update(cx, |view, cx| { - view.cues = cues.to_vec(); - cx.notify(); - }) - }, - ), - ) + div() + .w(px(self.width)) + .debug_selector(|| "subtitle-host".into()) + .child( + SubtitleEditor::new("subs", self.cues.clone(), self.at).on_change( + move |cues, _, cx| { + owner.update(cx, |view, cx| { + view.cues = cues.to_vec(); + cx.notify(); + }) + }, + ), + ) } } fn captioning(cx: &mut TestAppContext) -> (Entity, &mut VisualTestContext) { + captioning_at(420.0, "b", cx) +} + +fn captioning_at<'a>( + width: f32, + words: &'static str, + cx: &'a mut TestAppContext, +) -> (Entity, &'a mut VisualTestContext) { setup(cx); - let cue = |key: &str, start: u64, end: u64| Cue { + let cue = |key: &str, text: &str, start: u64, end: u64| Cue { key: key.to_string().into(), start: SECOND * start as u32, end: SECOND * end as u32, - text: key.to_string().into(), + text: text.to_string().into(), }; - let (view, cx) = cx.add_window_view(|_, _| Captioning { - cues: vec![cue("a", 1, 3), cue("b", 5, 8)], + let (view, cx) = cx.add_window_view(move |_, _| Captioning { + cues: vec![cue("a", "a", 1, 3), cue("b", words, 5, 8)], at: SECOND * 6, + width, }); cx.update(|window, _| window.activate_window()); settle(cx); @@ -339,3 +480,14 @@ fn a_time_that_does_not_read_keeps_the_cue_and_says_why(cx: &mut TestAppContext) assert_eq!(view.read_with(cx, |view, _| view.cues.clone()), before); assert!(cx.debug_bounds("subtitle-complaint").is_some()); } + +#[gpui::test] +fn long_words_give_way_inside_a_narrow_editor(cx: &mut TestAppContext) { + let long = "The shadow walks the length of the hall while the morning comes in"; + let (_, cx) = captioning_at(280.0, long, cx); + let host = cx.debug_bounds("subtitle-host").expect("the editor draws"); + let words = cx + .debug_bounds("inline-edit subs-b-words") + .expect("the words draw"); + assert!(words.right() <= host.right(), "{words:?} in {host:?}"); +} diff --git a/src/overlays/lightbox.rs b/src/overlays/lightbox.rs index b65bb62..42133f9 100644 --- a/src/overlays/lightbox.rs +++ b/src/overlays/lightbox.rs @@ -68,6 +68,11 @@ pub(crate) fn media_button( .tab_index(0) .focus_ring(cx) .hover(|style| style.bg(light.opacity(0.12))) + .on_key_down(|event, _, cx| { + if event.keystroke.key == "space" && !event.keystroke.modifiers.modified() { + cx.stop_propagation(); + } + }) .on_click(move |_, window, cx| run(window, cx)) }) }