From bf1036a2737d5032c863ff98eea3852c06a09154 Mon Sep 17 00:00:00 2001 From: David Mann Date: Sat, 10 Oct 2026 07:51:26 +0200 Subject: [PATCH] Layers panel: Opacity and Fill share one right-aligned column (#2085) (#2173) In the pro theme the Fill row zeroed the horizontal item spacing for the lock icons on the whole row, so the "Fill:" label touched its field while "Opacity:" kept the normal gap and sat 8 pt further left. Scope the zero spacing to the lock icons only, so both labels end the same gap before fields that share one right edge, as in Photoshop. The narrow-panel check that drops the "Lock:" text now counts the restored gap. Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: Brandon Thomas --- crates/ui-egui/src/panels.rs | 30 +++++++++++---------- crates/ui-egui/src/props_layout_tests.rs | 33 +++++++++++++++++++++++- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index f1cf04c2..939bab5b 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -1776,25 +1776,29 @@ fn layers(app: &mut PhotocraftApp, ui: &mut egui::Ui) { // In a narrow panel with long translated labels, drop the "Lock:" text (the icons keep // their tooltips) rather than let the Fill label run over the lock icons. let gap = ui.spacing().item_spacing.x; - let icons_w = if t.pro { 5.0 * 20.0 } else { 22.0 + gap }; + let icons_w = if t.pro { 5.0 * 20.0 + gap } else { 22.0 + gap }; let fits = body_text_width(ui, lock_label) + icons_w + body_text_width(ui, fill_label) + LAYER_PCT_W + 2.0 * gap <= ui.available_width(); if fits { label(ui, lock_label); } if t.pro { - ui.spacing_mut().item_spacing.x = 0.0; - let lk = l.locks; - for (key, icon, tip, on) in [ - ("transparency", "grid-3x3", tl!("Lock transparent pixels"), lk.transparency), - ("pixels", "brush", tl!("Lock image pixels"), lk.pixels), - ("position", "move", tl!("Lock position"), lk.position), - ("artboard", "scan", tl!("Prevent auto-nesting in and out of Artboards and Frames"), lk.artboard), - ("all", "lock", tl!("Lock all"), lk.all), - ] { - if icons::button(ui, icon, 20.0, on, tip).clicked() { - actions.push(("layer.setProps".into(), json!({"layer": l.id.0, "locks": { key: !on }}))); + // The lock icons touch, but only they: the Fill label keeps the same gap before its + // field as Opacity's, so both stack in one right-aligned column (#2085). + ui.scope(|ui| { + ui.spacing_mut().item_spacing.x = 0.0; + let lk = l.locks; + for (key, icon, tip, on) in [ + ("transparency", "grid-3x3", tl!("Lock transparent pixels"), lk.transparency), + ("pixels", "brush", tl!("Lock image pixels"), lk.pixels), + ("position", "move", tl!("Lock position"), lk.position), + ("artboard", "scan", tl!("Prevent auto-nesting in and out of Artboards and Frames"), lk.artboard), + ("all", "lock", tl!("Lock all"), lk.all), + ] { + if icons::button(ui, icon, 20.0, on, tip).clicked() { + actions.push(("layer.setProps".into(), json!({"layer": l.id.0, "locks": { key: !on }}))); + } } - } + }); } if !t.pro { let locked = l.locks.transparency || l.locks.position || l.locks.all; diff --git a/crates/ui-egui/src/props_layout_tests.rs b/crates/ui-egui/src/props_layout_tests.rs index 2dd641eb..fd41fce3 100644 --- a/crates/ui-egui/src/props_layout_tests.rs +++ b/crates/ui-egui/src/props_layout_tests.rs @@ -3,7 +3,7 @@ use egui::{Rect, vec2}; use egui_kittest::Harness; -use egui_kittest::kittest::Queryable; +use egui_kittest::kittest::{NodeT, Queryable}; use photocraft_doc::{Layer, LayerContent}; use serde_json::json; @@ -232,3 +232,34 @@ fn character_and_paragraph_dock_panels_share_the_headers_without_type_options() assert!(headers(&h).is_empty()); assert!(h.query_by_value("Select a type layer to edit its character and paragraph settings.").is_some()); } + +/// #2085: like Photoshop, Opacity and Fill stack in one right-aligned column: both fields end at +/// the same edge, and both labels end the same gap before them (Fill's label used to touch its field). +#[test] +fn layers_opacity_and_fill_share_one_right_column() { + for theme in ["pro", "studioLight"] { + let (s, layers) = kinds(); + let mut h = Harness::builder().with_size(vec2(1440.0, 1000.0)).with_max_steps(64).build_eframe(move |cc| { + PhotocraftApp::setup_context(&cc.egui_ctx, Default::default()); + PhotocraftApp::new(s, crate::Services::default()) + }); + set(&mut h, json!({"theme": theme})); + select(&mut h, layers[0].1); + // The popup-slider arrow is named after the field; its label is the text left of it on its row. + let row = |name: &str| { + let arrow = h.query_all_by_label(name).find(|n| n.accesskit_node().role() == egui::accesskit::Role::Button).unwrap().rect(); + let text = if theme == "pro" { format!("{name}:") } else { name.to_string() }; + let label = h + .query_all_by_label(&text) + .filter(|n| n.accesskit_node().role() == egui::accesskit::Role::Label) + .map(|n| n.rect()) + .find(|r| (r.center().y - arrow.center().y).abs() < 12.0 && r.right() <= arrow.left()) + .unwrap(); + (label, arrow) + }; + let (ol, oa) = row("Opacity"); + let (fl, fa) = row("Fill"); + assert!((oa.right() - fa.right()).abs() < 0.5, "{theme}: fields end at {} and {}", oa.right(), fa.right()); + assert!((ol.right() - fl.right()).abs() < 0.5, "{theme}: Opacity label ends at {}, Fill label at {}", ol.right(), fl.right()); + } +}