diff --git a/crates/ui-egui/src/dialogs/adjust_editors/ui_tests.rs b/crates/ui-egui/src/dialogs/adjust_editors/ui_tests.rs index bb0f737265..625039a692 100644 --- a/crates/ui-egui/src/dialogs/adjust_editors/ui_tests.rs +++ b/crates/ui-egui/src/dialogs/adjust_editors/ui_tests.rs @@ -570,6 +570,7 @@ fn gradient_map_stop_dragged_down_off_the_bar_is_removed_and_dragging_back_cance h.state_mut().run("layer.setAdjustment", json!({"layer": id.0, "stops": [[0, "#000000"], [0.5, "#ff0000"], [1, "#ffffff"]]})).unwrap(); h.run_steps(3); let three = layer(&h).1; + let before_cancel = h.state().session.active().unwrap().history.past_len(); let live_stops = |h: &Harness<'_, PhotocraftApp>| h.state().live_adjust.as_ref().and_then(|(_, v)| v["stops"].as_array().map(Vec::len)); // Off and back: nothing removed. let start = map_handle(&h, 0.5); @@ -589,6 +590,7 @@ fn gradient_map_stop_dragged_down_off_the_bar_is_removed_and_dragging_back_cance h.drop_at(start); h.run_steps(3); assert_eq!(map_stops(&layer(&h).1), map_stops(&three), "dragging back cancels the removal"); + assert_eq!(h.state().session.active().unwrap().history.past_len(), before_cancel, "a cancelled drag adds no history step"); // Off and released: removed in one step. let steps = h.state().session.active().unwrap().history.past_len(); drag(&mut h, start, away); diff --git a/crates/ui-egui/src/dialogs/layer_style/gradient_field.rs b/crates/ui-egui/src/dialogs/layer_style/gradient_field.rs index c59b666b3f..e5e253caab 100644 --- a/crates/ui-egui/src/dialogs/layer_style/gradient_field.rs +++ b/crates/ui-egui/src/dialogs/layer_style/gradient_field.rs @@ -235,10 +235,9 @@ pub(super) fn row(app: &mut PhotocraftApp, ui: &mut egui::Ui, f: &mut Map(drag_key)); - } else if resp.dragged() { + // The release frame applies the same rule (its last move may cross the edge), then ends + // the drag. + if resp.dragged() || resp.drag_stopped() { match drag.removed { None if off && stops.len() > 2 => { if let Some(stop) = stops.get(drag.index).copied() @@ -271,7 +270,11 @@ pub(super) fn row(app: &mut PhotocraftApp, ui: &mut egui::Ui, f: &mut Map(drag_key)); + } else { + ui.data_mut(|d| d.insert_temp(drag_key, drag)); + } } } else if resp.drag_stopped() { ui.data_mut(|d| d.remove::(drag_key)); @@ -455,6 +458,37 @@ mod tests { assert_eq!(stops.iter().map(|s| super::super::hex(&s.1)).collect::>(), ["#000000", "#ffffff"], "{stops:?}"); } + /// The release frame counts: a stop that leaves the strip in the frame it is released is + /// removed, and one brought back onto the strip in that frame is kept. + #[test] + fn the_release_frame_decides_whether_a_stop_comes_off_the_ramp() { + for back in [true, false] { + let (mut app, parent) = dialog_app(); + let effect = crate::layer_style::effects_of(&app.ui.dialogs[0].fields)[0]["id"].as_str().unwrap().to_string(); + set_param(&mut app.ui.dialog_mut(parent).unwrap().fields, &effect, KEY, json!([[0.0, "#000000"], [0.5, "#ff0000"], [1.0, "#ffffff"]])); + let mut h = harness(app, parent); + let (_, bar): (String, Rect) = h.ctx.data(|d| d.get_temp(bar_id())).expect("the ramp was drawn"); + let mid = pos2(bar.center().x, bar.bottom() + 9.0); + let away = pos2(mid.x, mid.y + 30.0); + press(&mut h, mid); + h.event(egui::Event::PointerMoved(pos2(mid.x, mid.y + 11.0))); + h.run_steps(1); + if back { + h.event(egui::Event::PointerMoved(away)); + h.run_steps(1); + assert_eq!(stops_in(h.state(), parent).len(), 2, "off the strip the stop is gone"); + } + let to = if back { mid } else { away }; + // The last move and the release in one frame (`Harness::event` gives each its own). + let input = &mut h.input_mut().events; + input.push(egui::Event::PointerMoved(to)); + input.push(egui::Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + let stops = stops_in(h.state(), parent); + assert_eq!(stops.len(), if back { 3 } else { 2 }, "back {back}: {stops:?}"); + } + } + /// A preset from the Gradients panel becomes the page's stops; the Delete button and the /// Location field work on the selected stop. #[test] diff --git a/crates/ui-egui/src/tools/gradient_ui.rs b/crates/ui-egui/src/tools/gradient_ui.rs index 12d2faeedd..daa9a9344f 100644 --- a/crates/ui-egui/src/tools/gradient_ui.rs +++ b/crates/ui-egui/src/tools/gradient_ui.rs @@ -903,7 +903,10 @@ fn selection_after_edit(f: &Fill, p: &Value, previous: Option) -> Option } fn stop_strip(ui: &mut egui::Ui, key: egui::Id, f: &Fill, midpoints: bool, pending: Option<&Value>, colors: ([f32; 4], [f32; 4])) -> Option { - let Fill::Gradient { stops, .. } = f else { return None }; + // The stops as committed: a drag edit (and the two-stop minimum) addresses these, not the + // pending preview, which may already leave the dragged stop out. + let Fill::Gradient { stops, opacity_stops, .. } = f else { return None }; + let opacity_count = if opacity_stops.is_empty() { 2 } else { opacity_stops.len() }; let t = Tokens::get(ui.ctx()); let w = ui.available_width().max(120.0); let (outer, resp) = ui.allocate_exact_size(vec2(w, 58.0), Sense::click_and_drag()); @@ -1015,14 +1018,15 @@ fn stop_strip(ui: &mut egui::Ui, key: egui::Id, f: &Fill, midpoints: bool, pendi let edit = match m { Marker::Color(i) if off && stops.len() > 2 => json!({"action": "delete", "index": i}), Marker::Color(i) => json!({"action": "move", "index": i, "location": loc}), - Marker::Opacity(i) if off && ops.len() > 2 => json!({"action": "delete", "kind": "opacity", "index": i}), + Marker::Opacity(i) if off && opacity_count > 2 => json!({"action": "delete", "kind": "opacity", "index": i}), Marker::Opacity(i) => json!({"action": "move", "kind": "opacity", "index": i, "location": loc}), Marker::Mid(i) => { let (a, b) = (sorted.get(i).map_or(0.0, |s| s.0), sorted.get(i + 1).map_or(1.0, |s| s.0)); json!({"action": "midpoint", "index": i, "location": ((loc - a) / (b - a).max(1e-4)).clamp(0.05, 0.95)}) } }; - if resp.dragged() { + // The release frame counts too: its own last move decides whether the stop comes off. + if resp.dragged() || resp.drag_stopped() { preview = Some(edit); } } diff --git a/crates/ui-egui/src/tools/gradient_ui/editor_tests.rs b/crates/ui-egui/src/tools/gradient_ui/editor_tests.rs index c761838f36..34636bd376 100644 --- a/crates/ui-egui/src/tools/gradient_ui/editor_tests.rs +++ b/crates/ui-egui/src/tools/gradient_ui/editor_tests.rs @@ -260,7 +260,7 @@ fn the_last_two_color_stops_cannot_be_dragged_off() { } } -/// An opacity stop dragged up off the bar is removed; dragging it down over the ramp is not. +/// An opacity stop dragged up off the bar is removed while two remain. #[test] fn an_opacity_stop_dragged_up_off_the_bar_is_removed() { for layer in [true, false] { @@ -277,3 +277,55 @@ fn an_opacity_stop_dragged_up_off_the_bar_is_removed() { assert_eq!(sink.fill(h.state()).unwrap(), before, "layer {layer}: two opacity stops always remain"); } } + +/// Only the stop's own direction takes it off: an opacity stop dragged down over the ramp and a +/// colour stop dragged up over it are kept, however far they go. +#[test] +fn a_stop_dragged_the_other_way_is_kept() { + for layer in [true, false] { + let (mut h, sink) = editor(layer); + for opacity in [true, false] { + let from = marker(&h, 0.5, opacity); + let to = from + vec2(0.0, if opacity { 70.0 } else { -70.0 }); + drag_through(&mut h, &[from, to], |h| assert_eq!(shown(h, sink), (3, 3), "layer {layer} opacity {opacity}: still shown")); + assert_eq!(counts(&h, sink), (3, 3), "layer {layer} opacity {opacity}: the stop is kept"); + } + } +} + +/// Press on a stop, drag a little along the bar, then move to each of `then` in its own frame; +/// the last move and the release arrive in the same frame, as a quick flick does. +fn flick(h: &mut Harness, from: Pos2, then: &[Pos2]) { + h.event(egui::Event::PointerMoved(from)); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + h.event(egui::Event::PointerMoved(from + vec2(10.0, 0.0))); + h.run_steps(1); + let Some((&last, moves)) = then.split_last() else { return }; + for &p in moves { + h.event(egui::Event::PointerMoved(p)); + h.run_steps(1); + } + // `Harness::event` runs one frame per event, so both go straight into the next frame. + let input = &mut h.input_mut().events; + input.push(egui::Event::PointerMoved(last)); + input.push(egui::Event::PointerButton { pos: last, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(3); +} + +/// The release frame counts: a stop flicked off the bar and released in the same frame is +/// removed, and one brought back onto the bar in the release frame is kept. +#[test] +fn the_release_frame_decides_whether_the_stop_comes_off() { + for layer in [true, false] { + let (mut h, sink) = editor(layer); + let from = marker(&h, 0.5, false); + let away = from + vec2(10.0, 40.0); + flick(&mut h, from, &[away, from]); + assert_eq!(counts(&h, sink), (3, 3), "layer {layer}: back on the bar at release, the stop is kept"); + let from = marker(&h, 0.5, false); + flick(&mut h, from, &[from + vec2(10.0, 40.0)]); + assert_eq!(counts(&h, sink), (2, 3), "layer {layer}: released off the bar in the same frame, the stop is removed"); + } +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 8bc4974646..abf9431ef4 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -240,7 +240,7 @@ Paragraph; the rest open as in-app floating windows. | Color panel modes from its panel menu (Hue Cube, Brightness Cube, Color Wheel, Grayscale/RGB/HSB/CMYK/Lab/Web Color sliders), colour ramp (RGB/CMYK Spectrum, Grayscale Ramp, Current Colors, Make Ramp Web Safe; Shift-click cycles, Alt-click picks the other colour), Copy Color as HTML / Hex Code; the out-of-gamut ⚠ with the nearest printable colour under the chips (sliders always; the cubes when the field is at least 53 pt tall); the same in every theme | done | `color_panel_ui.rs`, `color_panel_tests.rs`; UI-217-17, #2569, #6000, #8672 | | A colour picked in the Color or Swatches panel or the Color Picker recolours the selection: selected characters while editing; every selected type layer with a Type tool; every selected shape's fill with a shape, Pen or path selection tool; only the foreground with other tools, the Move tool too; a swatch click recolours the selected type layers and shape fills whatever the tool (#9238); one history step (a Color panel drag too) | done: `apply_color.rs`, `apply_color_tests.rs` | | | The Type tool's Color Picker (Text Color) and a gradient fill's Color Picker (Stop Color) preview the picked colour on the canvas before OK (the selected characters, every selected type layer, or the stop); Cancel restores the document with no history step, OK commits the previewed edit in one step (#5530) | done: `dialogs/color_picker_preview.rs`, `dialogs/color_picker_preview/tests.rs` | | -| Gradient stops come off the bar: a colour stop dragged down, or an opacity stop dragged up, by about a marker's height is removed on release while two of its kind remain; while it is off the ramp and canvas preview show it gone, and dragging it back onto the bar cancels; the selected stop's Delete button does the same. Gradient Editor, Properties, the shape gradient popover, Gradient Map and Layer Style (Gradient Overlay) share one rule (#10742) | done: `tools/gradient_ui.rs` `stop_dragged_off`, `tools/gradient_ui/editor_tests.rs`, `adjust_editors/ui_tests.rs`, `layer_style/gradient_field.rs`. Differs: Gradient Map and Layer Style show colour stops only; no Delete-key shortcut for the selected stop | | +| Gradient stops come off the bar: a colour stop dragged down, or an opacity stop dragged up, by about a marker's height is removed on release while two of its kind remain; while it is off the ramp and canvas preview show it gone, and dragging it back onto the bar cancels; the selected stop's Delete button (Remove stop, the × beside the colour, in Gradient Map) does the same. Gradient Editor, Properties, the shape gradient popover, Gradient Map and Layer Style (Gradient Overlay) share one rule (#10742) | done: `tools/gradient_ui.rs` `stop_dragged_off`, `tools/gradient_ui/editor_tests.rs`, `adjust_editors/ui_tests.rs`, `layer_style/gradient_field.rs`. Differs: Gradient Map and Layer Style show colour stops only; no Delete-key shortcut for the selected stop | | | Window size and dock width remembered | done | UI-217-2 | | Histogram panel: Compact (the default, of the entire image), Expanded and All Channels views (the channel kept across them); RGB/CMYK/Lab/Gray channels, Luminosity and Colors (overlaid, mixed where they overlap); Show Channels in Color and Show Statistics; Source (Entire Image, Selected Layer, Adjustment Composite) for multilayer documents; Mean, Std Dev, Median, Pixels, Level, Count, Percentile and Cache Level for the pointer level or a dragged range; the cached-data warning, Uncached Refresh (menu, button, warning click, double-click); the adjusted histogram over the original while an adjustment previews; Cache Levels 1–8 in Preferences › Performance; Color Sampler levels marked on the graph | partial | `histogram_panel.rs`, `histogram_panel_tests.rs`, `tone.rs`; differs: filter dialogs that preview on the CPU proxy aren't counted live, a selection doesn't limit the histogram yet, with Source Selected Layer an adjustment dialog's preview isn't counted, and new strings are English-only so far | | Command palette with recents | done | UI-217-7 |