From 4ad80fa7bee3bfc6b3a5cd1ce99de69d347808c2 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Tue, 3 Nov 2026 09:57:59 -0500 Subject: [PATCH] M1: Layers panel width: measure every candidate as painted, cap painting, no drops past the list (#1904) Review: the widest name is found by laying rows out in descending order of a bound on their width until the bound falls under the widest found, with no cut at 48 rows; each name is laid out as it is painted (one line, cut at NAME_ROOM); the painted room is capped at NAME_ROOM too; and a drag over a wide row counts only inside the list's clip rect, so a layer released over the canvas drops nowhere. Tests: a_wide_name_among_many_long_narrow_ones_is_still_measured, a_name_with_line_breaks_is_measured_as_painted, a_drag_released_over_the_canvas_drops_nowhere. Co-Authored-By: Claude Fable 5.1 --- crates/ui-egui/src/layers_wide_names_tests.rs | 46 +++++++++++++++++++ crates/ui-egui/src/panels/layers.rs | 29 ++++++------ docs/ui-parity.md | 4 +- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/layers_wide_names_tests.rs b/crates/ui-egui/src/layers_wide_names_tests.rs index 2a4ae8999..eeb833e42 100644 --- a/crates/ui-egui/src/layers_wide_names_tests.rs +++ b/crates/ui-egui/src/layers_wide_names_tests.rs @@ -113,3 +113,49 @@ fn a_name_past_the_room_cap_still_elides() { assert!(elided, "a 3000-character name elides"); assert!(size.x <= crate::layers::NAME_ROOM + 1.0, "at the cap, not thousands of points wide: {}", size.x); } + +/// Review: the widest name isn't always the longest in bytes. Fifty rows of narrow letters outrank +/// one row of wide ones by the bound; it is laid out all the same, so it reads in full. +#[test] +fn a_wide_name_among_many_long_narrow_ones_is_still_measured() { + let mut s = Session::new(); + for _ in 0..50 { + named(&mut s, &"i".repeat(100), None); + } + named(&mut s, &"W".repeat(70), None); + let mut u = Ui::new(s); + let (elided, ..) = painted(&mut u, "WWWW"); + assert!(!elided, "the wide name reads in full"); +} + +/// Review: a name is laid out as it is painted, on one line, so line breaks in it don't make it +/// measure narrower than it paints. +#[test] +fn a_name_with_line_breaks_is_measured_as_painted() { + let mut s = Session::new(); + named(&mut s, "Size=small,\nType=primary,\nState=default,\nIcon=leading,\nWidth=full", None); + let mut u = Ui::new(s); + let (elided, ..) = painted(&mut u, "Size=small,"); + assert!(!elided, "the whole name on one line"); +} + +/// Review: a wide row runs on under the canvas; a dragged layer released there is no drop. +#[test] +fn a_drag_released_over_the_canvas_drops_nowhere() { + let mut s = Session::new(); + let wide = named(&mut s, &"Size=small, Type=primary, ".repeat(6), None); + let short = named(&mut s, "Short", None); + let page = s.page().unwrap(); + let order = s.doc().unwrap().node(page).unwrap().children.clone(); + let mut u = Ui::new(s); + let from = u.row_at(short); + let level = u.row_at(wide).y; + // Over the canvas, level with the wide row (which runs on under it). + let over_canvas = Pos2::new(800.0, level); + u.drag_to(from, over_canvas); + u.button(over_canvas, false); + for _ in 0..5 { + u.frame(vec![]); + } + assert_eq!(u.app.session.doc().unwrap().node(page).unwrap().children, order, "no reorder"); +} diff --git a/crates/ui-egui/src/panels/layers.rs b/crates/ui-egui/src/panels/layers.rs index a34b43e77..54682c683 100644 --- a/crates/ui-egui/src/panels/layers.rs +++ b/crates/ui-egui/src/panels/layers.rs @@ -540,25 +540,20 @@ fn name_font(doc: &Document, page: NodeId, n: &figcraft_doc::Node) -> egui::Font /// The width the widest of `rows` needs to show its whole name, from the panel's left edge: the /// row's margins, its indent, the disclosure triangle and icon, the name in its font (up to /// [`NAME_ROOM`]) and the eye and padlock's room. The list is made this wide, so it scrolls -/// sideways when a name doesn't fit the panel (#1904). Only the rows whose name could be the -/// widest are laid out: a name's bytes at the widest glyph bound its width, and the few rows with -/// the largest bounds are measured, largest first, until the bound falls under the widest found; -/// a 10 000-row list lays out a handful of names. +/// sideways when a name doesn't fit the panel (#1904). A name's bytes at the widest glyph bound +/// its width, and the rows are laid out in descending order of that bound until the bound falls +/// under the widest found, so no name that could be the widest is skipped: a list with one long +/// name lays out a handful, a list of names all alike lays them all out, through egui's galley +/// cache. A name is laid out as it is painted: on one line, cut at [`NAME_ROOM`]. fn widest_row(ui: &Ui, doc: &Document, page: NodeId, rows: &[Row]) -> f32 { // What sits left of the name (the row's margin, the indent's start, the triangle and icon) // and right of it (the eye and padlock, the margin): see the row loop in `layers`. const AROUND: f32 = 8.0 + 4.0 + 36.0 + 56.0 + 8.0; // No glyph of the 11 pt fonts is wider than this: a name's byte count bounds its width. const GLYPH: f32 = 12.0; - // The rows laid out at most: enough that the widest is among them whatever the letters. - const MEASURED: usize = 48; let indent = |r: &Row| r.depth as f32 * 16.0; let mut bounds: Vec<(f32, usize)> = rows.iter().enumerate().filter_map(|(i, r)| doc.node(r.id).map(|n| (indent(r) + row_name(n).len() as f32 * GLYPH, i))).collect(); - if bounds.len() > MEASURED { - bounds.select_nth_unstable_by(MEASURED, |a, b| b.0.total_cmp(&a.0)); - bounds.truncate(MEASURED); - } bounds.sort_by(|a, b| b.0.total_cmp(&a.0)); let mut widest = 0.0f32; for (bound, i) in bounds { @@ -566,8 +561,8 @@ fn widest_row(ui: &Ui, doc: &Document, page: NodeId, rows: &[Row]) -> f32 { break; } let Some((r, n)) = rows.get(i).and_then(|r| doc.node(r.id).map(|n| (r, n))) else { continue }; - let w = ui.painter().layout_no_wrap(row_name(n).to_string(), name_font(doc, page, n), Color32::WHITE).size().x; - widest = widest.max(indent(r) + w.min(NAME_ROOM)); + let w = crate::bidi::name_galley(ui.painter(), row_name(n), name_font(doc, page, n), Color32::WHITE, NAME_ROOM).size().x; + widest = widest.max(indent(r) + w); } widest + AROUND } @@ -1198,8 +1193,8 @@ fn layers(app: &mut FigApp, ui: &mut Ui) { let font = name_font(&doc, page, n); let name = row_name(n).to_string(); // Room left of the lock and eye buttons (the row is wide enough for the whole - // name, up to `NAME_ROOM`). - let max_w = (rr.max.x - 56.0 - name_x).max(20.0); + // name, up to `NAME_ROOM`: a row widened by a deeper name gives no more). + let max_w = (rr.max.x - 56.0 - name_x).clamp(20.0, NAME_ROOM); crate::bidi::paint_name(ui.painter(), pos2(name_x, r.center().y), &name, font, col, max_w); } let (v, l) = eye_lock(ui, &t, [r, rr], row, n, over && !dev, dev, &mut sweep); @@ -1223,7 +1218,10 @@ fn layers(app: &mut FigApp, ui: &mut Ui) { if resp.drag_started() && !crate::menus::dev_mode_refuses(app, "node.reparent") { ui.data_mut(|d| d.insert_temp(drag_id, row.id)); } - let over = dragging.is_some() && ui.input(|i| i.pointer.hover_pos()).is_some_and(|p| r.contains(p)); + // In the row and in the list's view: a wide row runs on under the canvas (#1904), and a + // drag released there lands nowhere. + let clip = ui.clip_rect(); + let over = dragging.is_some() && ui.input(|i| i.pointer.hover_pos()).is_some_and(|p| r.contains(p) && clip.contains(p)); // What is inside the dragged layers, and inside an instance, takes no drop. refused |= over && (doc.ancestors(row.id).iter().any(|a| moving.contains(a)) @@ -1232,6 +1230,7 @@ fn layers(app: &mut FigApp, ui: &mut Ui) { && !blocked(row.id) && let Some(p) = ui.input(|i| i.pointer.hover_pos()) && r.contains(p) + && clip.contains(p) { let collapsed = row.has_kids && !app.ui.expanded.contains(&row.id); let (zone, open, released) = drop_zone(ui, p, [r, rr], x0, row.id, collapsed, n); diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 71d32a14b..facf91b39 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -170,7 +170,7 @@ in the table. `crates/ui-egui/src/shortcut_keys_tests.rs` drives the keys a look | Screen-reader names, roles, values, states (AccessKit) | done on desktop | browser: eframe doesn't send the tree yet | | Keyboard navigation: F6 regions, Tab within, focus ring | done | | | Layers panel from the keyboard: arrows, F2 / Cmd+R rename (the row gets the keyboard back), Shift+F10 context menu; 10k-layer lists | done | revealing a layer, expanding, collapsing and arrow keys timed in `cargo xtask bench` | -| Layers panel: long or deeply nested names | done | the list is as wide as its widest name needs and scrolls sideways, so a name reads in full without widening the sidebar (the reference app's Layers panel has a horizontal scrollbar); a name past 1200 pt still elides (`layers_wide_names_tests`, #1904) | +| Layers panel: long or deeply nested names | done | the list is as wide as its widest name needs and scrolls sideways, so a name reads in full without widening the sidebar (the reference app's Layers panel has a horizontal scrollbar); a drag released over the canvas drops nowhere; a name past 1200 pt still elides (`layers_wide_names_tests`, #1904) | | Reduced motion | done | system setting or preference | | WCAG AA contrast of secondary and accent text | done | disabled and dim text at least 3:1 against panels, fields and selected rows, selected text at least 3:1 on its highlight, in both themes (`theme::tests`, #890) | | Tested with VoiceOver, NVDA, Narrator, Orca | missing | human QA | @@ -179,7 +179,7 @@ in the table. `crates/ui-egui/src/shortcut_keys_tests.rs` drives the keys a look | Date | Change | Summary | |---|---|---| -| 2026-11-03 | patch | M1, #1904: the Layers panel cut a name that didn't fit with an ellipsis and had no horizontal scrollbar, so the variants of an imported component set all read `Size=small, Type=prim…` and a deeply nested name lost its room to the indent. The list is now as wide as its widest name needs (the row cache measures the few names that could be the widest, so a 10 000-row list lays out a handful) and scrolls sideways, as the reference app's Layers panel does with its horizontal scrollbar; revealing a layer scrolls up or down only; the eye and padlock sit at the end of the row and scroll with it; a name past 1200 pt still elides (#1650). Tests: `layers_wide_names_tests`. Numbers unchanged. | +| 2026-11-03 | patch | M1, #1904: the Layers panel cut a name that didn't fit with an ellipsis and had no horizontal scrollbar, so the variants of an imported component set all read `Size=small, Type=prim…` and a deeply nested name lost its room to the indent. The list is now as wide as its widest name needs (the row cache lays names out in descending order of a bound on their width until the bound falls under the widest found: a list with one long name lays out a handful, names all alike all of them, through egui's galley cache) and scrolls sideways, as the reference app's Layers panel does with its horizontal scrollbar; revealing a layer scrolls up or down only; the eye and padlock sit at the end of the row and scroll with it; a drag released past the list, over the canvas, drops nowhere; a name past 1200 pt still elides (#1650). Tests: `layers_wide_names_tests`. Numbers unchanged. | | 2026-11-02 | patch | M8, #1875: a click with the Bend tool on a sharp point between two segments gives it handles along the line between its neighbours, a third of the way to each ("Click on a point or path where you want to add a curve", Edit vector layers); a click on a curved point still makes it sharp, a drag still sets the handles by hand, an endpoint is left alone. Between a handle's end and a point both within reach of the pointer, the nearer one on the page is hit (the handle on a tie), so a short path's point (its handles a few pixels long) can be clicked again. Before, only a drag curved a point, and a short one left two tiny handles in its direction. Tests: `vectoredit::tests::short_handles_leave_their_point_clickable`, `vectoredit::tests::a_bend_click_curves_a_sharp_point_between_its_neighbours`, `bend_tool_smooths_and_sharpens_points`. Numbers unchanged. | | 2026-11-02 | minor | M4, #1827: wrap style on a text layer (`WrapStyle`: Auto, Balance, Pretty): Balance re-breaks each paragraph at the narrowest width that keeps its line count (bisection over the greedy breaker, as browsers balance), Pretty narrows one of the last four lines just enough so the last line isn't one word; `text.wrapStyle`, `node.set wrapStyle` (`textWrap` too), inspect `wrapStyle`, Dev Mode CSS `text-wrap` (Tailwind `text-balance` / `text-pretty`), the Type settings control under Vertical trim. Per-paragraph settings, text-style support and the `.fig` import/export mapping stay open. Test: `text_behaviour_tests::balance_and_pretty_wrap_styles_break_lines_as_documented`. | | 2026-11-02 | patch | M4: Backspace or Delete at the start of a list item takes its marker and keeps its level ("Use Backspace or Delete at the beginning of a list item to delete the counter, but keep the same level of indentation", Create bulleted and numbered lists): a plain paragraph may now carry a list level, and the text layout starts it where that level's item text starts (the gutters stay even when no list is left). Backspace used to drop the level too, and Delete at an item's start deleted the next character. Tab on such a paragraph types a tab, as outside a list. Tests: `text_tail_tests::list_keys_at_list_items`, text crate `behaviour_tests::a_plain_paragraph_at_a_list_level_starts_where_the_items_text_does` |