From 4010f4a466ef16957c548c2bcadf4fa6d0f3f090 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 15:00:40 -0400 Subject: [PATCH] LRC-LIB-KEYWORDS: a keyword drag ends with Esc, or wherever it is released The Keyword List ended a keyword drag only while it was drawn: with another panel opened mid-drag, the drag lingered until the list came back. `keyword_list::drag_feedback` now runs every frame, like the album drag's: the drag ends with the button's release wherever it is, or with Esc. A press on a tick box that moves no longer picks the keyword up. The test that a keyword can't be dropped inside itself now checks that no move was even tried. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/panels/keyword_list.rs | 30 ++++++------ crates/ui-egui/src/tests_keyword_list.rs | 57 +++++++++++++++++++++++ 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index eebffbc76..4169af149 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -969,6 +969,7 @@ impl LightcraftApp { pick::poll(self, &ctx); panels::grid::drag_feedback(self, &ctx); panels::left::album_drag_feedback(self, &ctx); + panels::keyword_list::drag_feedback(self, &ctx); panels::toast(self, &ctx); self.widgets = widgets::take_registry(&ctx); self.end_frame(&ctx, t0); diff --git a/crates/ui-egui/src/panels/keyword_list.rs b/crates/ui-egui/src/panels/keyword_list.rs index 2e5274dc5..8eab45029 100644 --- a/crates/ui-egui/src/panels/keyword_list.rs +++ b/crates/ui-egui/src/panels/keyword_list.rs @@ -78,15 +78,6 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { row(app, ui, r, &selection, &ticks, filter.trim().is_empty()); } ui.add_space(12.0); - // the drag ends with the button's release, wherever it is - if app.ui.dragging_keyword.is_some() { - let (released, down) = ui.input(|i| (i.pointer.any_released(), i.pointer.any_down())); - if released || !down { - app.ui.dragging_keyword = None; - } else if let (Some(k), Some(p)) = (app.ui.dragging_keyword.clone(), pointer) { - drag_label(ui.ctx(), &k, p); - } - } } /// After the keyword `from` became `to` (renamed, moved or merged), the list's pick and open @@ -119,11 +110,21 @@ fn shown_under(ui: &egui::Ui, rect: Rect, pointer: Option) -> bool { pointer.is_some_and(|p| rect.contains(p) && ui.clip_rect().contains(p)) } -/// The dragged keyword's name, following the pointer. -fn drag_label(ctx: &egui::Context, keyword: &str, at: egui::Pos2) { +/// A keyword being dragged, each frame (after the panels, which take the drop): its name follows +/// the pointer, and the drag ends with the button's release wherever it is (the list may be gone +/// by then) or with Esc. +pub fn drag_feedback(app: &mut LightcraftApp, ctx: &egui::Context) { + let Some(keyword) = app.ui.dragging_keyword.clone() else { return }; + let (released, down, at, esc) = + ctx.input(|i| (i.pointer.any_released(), i.pointer.any_down(), i.pointer.latest_pos(), i.key_pressed(egui::Key::Escape))); + if released || !down || esc { + app.ui.dragging_keyword = None; + return; + } + let Some(at) = at else { return }; let t = Tokens::get(ctx); ctx.set_cursor_icon(egui::CursorIcon::Grabbing); - let name = keyword.rsplit('|').next().unwrap_or(keyword).to_string(); + let name = keyword.rsplit('|').next().unwrap_or(&keyword).to_string(); egui::Area::new(egui::Id::new("drag-keyword")).order(egui::Order::Tooltip).interactable(false).fixed_pos(at + vec2(14.0, 10.0)).show(ctx, |ui| { egui::Frame::NONE.fill(t.accent).corner_radius(10.0).inner_margin(egui::Margin::symmetric(9, 3)).show(ui, |ui| { ui.label(egui::RichText::new(name).color(Color32::WHITE).font(t.semibold(12.0))); @@ -199,8 +200,9 @@ fn row(app: &mut LightcraftApp, ui: &mut egui::Ui, r: &Row, selection: &[PhotoId let state = ticks.tick(&r.path); let boxr = Rect::from_center_size(pos2(x + 22.0, cy), vec2(13.0, 13.0)); let on = !selection.is_empty(); - let tb = - ui.interact(boxr.expand(3.0), egui::Id::new(("keyword-list-tick", r.path.to_lowercase())), if on { Sense::click() } else { Sense::hover() }); + // (it senses drags too, so a press there that moves doesn't pick the keyword up) + let sense = if on { Sense::click_and_drag() } else { Sense::hover() }; + let tb = ui.interact(boxr.expand(3.0), egui::Id::new(("keyword-list-tick", r.path.to_lowercase())), sense); register(ui.ctx(), format!("keywordCheck:{}", r.path), boxr); let edge = if on { t.text_label } else { t.text_dim.gamma_multiply(0.5) }; match state { diff --git a/crates/ui-egui/src/tests_keyword_list.rs b/crates/ui-egui/src/tests_keyword_list.rs index 6790ccff1..6c638562a 100644 --- a/crates/ui-egui/src/tests_keyword_list.rs +++ b/crates/ui-egui/src/tests_keyword_list.rs @@ -245,6 +245,7 @@ fn a_keyword_cant_be_dropped_inside_itself() { drag(&mut h, "keywordRow:Portugal", "keywordRow:Portugal|Lisbon"); assert_eq!((keywords_of(&h, ids[0]), h.app.session.undo.len()), (vec!["Portugal|Lisbon".to_string()], undo)); assert_eq!(h.app.ui.dialog, None); + assert_eq!(h.app.ui.toast, None, "not even tried: no error to show"); } /// Dropping a keyword where one of that name is already (Italy has a Rome) asks before merging @@ -343,3 +344,59 @@ fn the_list_follows_its_keywords() { assert!(has(&h, "keywordRow:Portugal|Lisbon"), "its new parent opened"); assert_eq!(h.app.ui.keyword_list_selected.as_deref(), Some("Portugal|Lisbon")); } + +/// Press at `from`, move in steps to `to` (each a frame), without releasing. +fn press_and_move(h: &mut Headless, from: egui::Pos2, to: egui::Pos2) { + let press = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }; + h.events.extend([egui::Event::PointerMoved(from), press(from, true)]); + h.step(); + for i in 1..=8 { + h.events.push(egui::Event::PointerMoved(from + (to - from) * (i as f32 / 8.0))); + h.step(); + } +} + +fn release(h: &mut Headless, at: egui::Pos2) { + h.events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.settle(SETTLE); +} + +/// Esc gives up a keyword drag: released on another keyword afterwards, it stays where it is. +#[test] +fn escape_gives_up_a_keyword_drag() { + let (mut h, ids) = keywords_panel_at([1400.0, 2400.0]); + run(&mut h, "photo.setMeta", json!({"ids": [ids[0]], "keywords": ["Lisbon"]})); + run(&mut h, "keyword.create", json!({"name": "Portugal", "parent": null})); + let (from, to) = (center(&h, "keywordRow:Lisbon"), center(&h, "keywordRow:Portugal")); + press_and_move(&mut h, from, to); + assert_eq!(h.app.ui.dragging_keyword.as_deref(), Some("Lisbon"), "dragging"); + h.events.push(egui::Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE }); + h.step(); + assert_eq!(h.app.ui.dragging_keyword, None, "given up"); + release(&mut h, to); + assert_eq!(keywords_of(&h, ids[0]), ["Lisbon"]); +} + +/// A drag ends with the button's release even when the Keyword List went away meanwhile (another +/// panel opened): it doesn't linger to the next time the list is shown. +#[test] +fn a_keyword_drag_ends_without_the_list() { + let (mut h, _) = keywords_panel_at([1400.0, 2400.0]); + let from = center(&h, "keywordRow:mountains"); + press_and_move(&mut h, from, from + egui::vec2(-300.0, 0.0)); + assert_eq!(h.app.ui.dragging_keyword.as_deref(), Some("mountains")); + h.app.ui.right = crate::state::RightPanel::Info; + h.step(); + release(&mut h, from + egui::vec2(-300.0, 0.0)); + assert_eq!(h.app.ui.dragging_keyword, None); +} + +/// Pressing on a tick box and moving doesn't pick the keyword up. +#[test] +fn a_tick_box_doesnt_start_a_drag() { + let (mut h, _) = keywords_panel_at([1400.0, 2400.0]); + let from = center(&h, "keywordCheck:mountains"); + press_and_move(&mut h, from, from + egui::vec2(0.0, 60.0)); + assert_eq!(h.app.ui.dragging_keyword, None); + release(&mut h, from + egui::vec2(0.0, 60.0)); +}