From 452749f58c9a1620d90a0a89d6ee2bf76f7384db Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Sat, 29 Aug 2026 11:35:06 +0100 Subject: [PATCH] fix(ribbon): align combo panel dropdowns to a shared baseline Style selectors (Standard, layer, Color/Linetype/Lineweight) sat at different heights across ribbon panels because each combo panel vertically centered its content: panels with fewer icon rows (Multiline Text, Table) centered their dropdown differently than panels with two rows (Dimensions, Multileader). - Pin all combo panels (Layers, Properties, style combos) to the top of the fixed 3-row panel with a shared COMBO_PANEL_PAD, so every primary dropdown shares one baseline regardless of icon-row count - Extract the duplicated tool-row and panel-container code into tool_row() / combo_panel_col() helpers + named spacing constants - Bundle the per-view render state into RenderCtx, shrinking render_large/render_large_dropdown from 12-14 positional args to (item, &ctx) - print_to_printer: silence the Windows-only dead-code warning on PrintOptions::quality with the platform split documented (the field is read by the CUPS `lp -o print-quality` path) --- src/io/print_to_printer.rs | 5 +- src/ui/ribbon/mod.rs | 93 +++++++------- src/ui/ribbon/widgets.rs | 245 +++++++++++++++++-------------------- 3 files changed, 166 insertions(+), 177 deletions(-) diff --git a/src/io/print_to_printer.rs b/src/io/print_to_printer.rs index 7e3128101..6f4d3a680 100644 --- a/src/io/print_to_printer.rs +++ b/src/io/print_to_printer.rs @@ -45,7 +45,10 @@ pub struct PrintOptions { pub printer: Option, /// Number of copies (treated as at least 1). pub copies: u32, - /// Print quality label selected in the plot dialog. + /// Print quality label selected in the plot dialog. Read only on the CUPS + /// path (`lp -o print-quality=…`); on Windows the driver's own quality + /// setting wins, so the field is legitimately unread there. + #[cfg_attr(target_os = "windows", allow(dead_code))] pub quality: Option, /// Controls applied while building the intermediate PDF. pub render: crate::io::pdf_export::PdfPlotOptions, diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index 96815f642..5b3308611 100644 --- a/src/ui/ribbon/mod.rs +++ b/src/ui/ribbon/mod.rs @@ -1303,6 +1303,20 @@ fn render_group<'a>( let mut items_row: Vec> = Vec::new(); let mut small_buf: Vec> = Vec::new(); + let ctx = widgets::RenderCtx { + active_tool, + open_dd, + last_cmd, + state, + layer_infos, + active_layer, + active_color, + active_linetype, + active_lineweight, + style_ctx, + compact, + }; + for item in &group.tools { let is_large = match item { RibbonItem::LargeTool(_) | RibbonItem::LargeDropdown { .. } => !compact, @@ -1314,20 +1328,7 @@ fn render_group<'a>( if is_large { flush_small_col(&mut small_buf, &mut items_row); - items_row.push(render_large( - item, - active_tool, - open_dd, - last_cmd, - state, - layer_infos, - active_layer, - active_color, - active_linetype, - active_lineweight, - style_ctx, - compact, - )); + items_row.push(render_large(item, &ctx)); } else { small_buf.push(render_small( item, @@ -1467,34 +1468,42 @@ fn collapse_button<'a>( // For a Properties panel the representative is its Match button. let rep = representative(group, last_used); let face: Element<'_, Message> = match rep { - Some(RibbonItem::PropertiesGroup { match_prop }) => render_large( - &RibbonItem::LargeTool(match_prop.clone()), - active_tool, - open_dd, - last_cmd, - state, - layer_infos, - active_layer, - active_color, - active_linetype, - active_lineweight, - style_ctx, - false, - ), - Some(item) => render_large( - item, - active_tool, - open_dd, - last_cmd, - state, - layer_infos, - active_layer, - active_color, - active_linetype, - active_lineweight, - style_ctx, - false, - ), + Some(RibbonItem::PropertiesGroup { match_prop }) => { + render_large( + &RibbonItem::LargeTool(match_prop.clone()), + &widgets::RenderCtx { + active_tool, + open_dd, + last_cmd, + state, + layer_infos, + active_layer, + active_color, + active_linetype, + active_lineweight, + style_ctx, + compact: false, + }, + ) + } + Some(item) => { + render_large( + item, + &widgets::RenderCtx { + active_tool, + open_dd, + last_cmd, + state, + layer_infos, + active_layer, + active_color, + active_linetype, + active_lineweight, + style_ctx, + compact: false, + }, + ) + } None => text("").into(), }; diff --git a/src/ui/ribbon/widgets.rs b/src/ui/ribbon/widgets.rs index 29e9f44a3..edbc651d0 100644 --- a/src/ui/ribbon/widgets.rs +++ b/src/ui/ribbon/widgets.rs @@ -70,6 +70,18 @@ pub(super) const TOOL_BAR_H: f32 = 3.0 * ROW_H + 18.0; /// shorter than a full 3-row panel — the ribbon height follows it down. pub(super) const COLLAPSED_FACE_H: f32 = LARGE_ICON + 20.0; +/// Gap between the combo dropdown and the tool rows beneath it. +const COMBO_ROW_SPACING: f32 = 3.0; +/// Shared padding of combo-style panels (Layers, Properties, style selectors). +/// The top inset pins every primary dropdown to the same baseline regardless +/// of how many icon rows sit underneath it. +const COMBO_PANEL_PAD: Padding = Padding { + top: 6.0, + bottom: 4.0, + left: 4.0, + right: 4.0, +}; + // ── Automatic large-button sizing ──────────────────────────────────────── thread_local! { @@ -604,21 +616,40 @@ pub(super) fn render_small<'a>( // ── Large item renderer ──────────────────────────────────────────────────── +/// Shared render context threaded through the large-item render path. Bundling +/// the per-view state keeps new fields from rippling through every render +/// signature and call site (same idea as `ToggleState`). +#[derive(Clone, Copy)] +pub(super) struct RenderCtx<'a> { + pub active_tool: &'a Option, + pub open_dd: &'a Option, + pub last_cmd: &'a HashMap<&'static str, &'static str>, + pub state: ToggleState, + pub layer_infos: &'a [LayerInfo], + pub active_layer: &'a str, + pub active_color: AcadColor, + pub active_linetype: &'a str, + pub active_lineweight: LineWeight, + pub style_ctx: &'a StyleContext, + /// When compact, the Properties panel's Match button shrinks to a small icon. + pub compact: bool, +} + /// A large dropdown button: the current icon on top, its ▾ directly beneath the /// icon, then the label at the bottom. Shared by `LargeDropdown` / `Dropdown` in /// the full ribbon and by a collapsed panel whose representative is a dropdown. /// `explicit_label` overrides the derived (current-item) label when given. -#[allow(clippy::too_many_arguments)] pub(super) fn render_large_dropdown<'a>( id: &'static str, icon: IconKind, explicit_label: Option<&'a str>, items: &[(&'static str, &'static str, IconKind)], default: &'static str, - active_tool: &Option, - open_dd: &Option, - last_cmd: &HashMap<&'static str, &'static str>, + ctx: &RenderCtx<'_>, ) -> Element<'a, Message> { + let active_tool = ctx.active_tool; + let open_dd = ctx.open_dd; + let last_cmd = ctx.last_cmd; let active = active_tool.as_deref() == Some(id) || items .iter() @@ -708,22 +739,65 @@ pub(super) fn render_large_dropdown<'a>( .into() } +/// A row of small tool buttons beneath a combo dropdown. Shared by the +/// Layers, Properties and style-combo panels so their icon rows render +/// identically. +fn tool_row<'a>(tools: &[ToolDef], active_tool: &Option) -> Element<'a, Message> { + let btns: Vec> = tools + .iter() + .map(|t| { + let is_active = active_tool.as_deref() == Some(t.id); + let tip = t!(t.label); + let event = t.event.clone(); + let icon_el: Element = make_icon(t.icon, 16.0); + let msg = module_event_to_message(event); + tooltip( + button(icon_el) + .on_press(msg) + .style(move |theme: &Theme, status| { + tool_btn_style(theme, is_active, status) + }) + .padding([2, 5]), + make_tip(tip.to_string()), + TipPos::Right, + ) + .gap(4.0) + .delay(Duration::from_millis(400)) + .style(tip_style) + .into() + }) + .collect(); + row(btns).spacing(2).align_y(iced::Center).into() +} + +/// Fixed-height wrapper for a combo-style panel: dropdown on top, icon rows +/// below, all pinned to the panel's top edge so every panel's primary +/// dropdown shares one baseline. +fn combo_panel_col(width: f32, items: Vec>) -> Element<'_, Message> { + container(column(items).spacing(COMBO_ROW_SPACING).align_x(iced::Left)) + .width(Length::Fixed(width)) + .height(Fill) + .align_y(iced::Top) + .padding(COMBO_PANEL_PAD) + .into() +} + /// Render a full-height large button (LargeTool, LargeDropdown, LayerCombo, StyleCombo). pub(super) fn render_large<'a>( item: &RibbonItem, - active_tool: &Option, - open_dd: &Option, - last_cmd: &HashMap<&'static str, &'static str>, - state: ToggleState, - layer_infos: &'a [LayerInfo], - active_layer: &'a str, - active_color: AcadColor, - active_linetype: &'a str, - active_lineweight: LineWeight, - style_ctx: &StyleContext, - // When compact, the Properties panel's Match button shrinks to a small icon. - compact: bool, + ctx: &RenderCtx<'_>, ) -> Element<'a, Message> { + let active_tool = ctx.active_tool; + let open_dd = ctx.open_dd; + let last_cmd = ctx.last_cmd; + let state = ctx.state; + let layer_infos = ctx.layer_infos; + let active_layer = ctx.active_layer; + let active_color = ctx.active_color; + let active_linetype = ctx.active_linetype; + let active_lineweight = ctx.active_lineweight; + let style_ctx = ctx.style_ctx; + let compact = ctx.compact; match item { // A plain Tool renders large too, so a collapsed panel can show its // representative tool as a big icon. @@ -779,16 +853,7 @@ pub(super) fn render_large<'a>( items, default, } => { - render_large_dropdown( - *id, - *icon, - Some(*label), - items, - *default, - active_tool, - open_dd, - last_cmd, - ) + render_large_dropdown(*id, *icon, Some(*label), items, *default, ctx) } // A plain Dropdown renders large too (used by a collapsed panel whose @@ -799,9 +864,7 @@ pub(super) fn render_large<'a>( items, default, } => { - render_large_dropdown( - *id, *icon, None, items, *default, active_tool, open_dd, last_cmd, - ) + render_large_dropdown(*id, *icon, None, items, *default, ctx) } RibbonItem::LayerComboGroup { row2, row3 } => { @@ -868,56 +931,14 @@ pub(super) fn render_large<'a>( .padding([3, 8]) .width(Fill); - let make_tool_row = |tools: &[ToolDef]| -> Element { - let btns: Vec> = tools - .iter() - .map(|t| { - let is_active = active_tool.as_deref() == Some(t.id); - let tip = t!(t.label); - let event = t.event.clone(); - let icon_el: Element = make_icon(t.icon, 16.0); - let msg = module_event_to_message(event); - tooltip( - button(icon_el) - .on_press(msg) - .style(move |theme: &Theme, status| { - tool_btn_style(theme, is_active, status) - }) - .padding([2, 5]), - make_tip(tip.to_string()), - TipPos::Right, - ) - .gap(4.0) - .delay(Duration::from_millis(400)) - .style(tip_style) - .into() - }) - .collect(); - row(btns).spacing(2).align_y(iced::Center).into() - }; - - let tools_row2 = make_tool_row(row2); - let tools_row3 = make_tool_row(row3); - - container( - column![ - PosReport::new(LAYER_COMBO_ID, combo_btn), - tools_row2, - tools_row3 - ] - .spacing(3) - .align_x(iced::Left), + combo_panel_col( + combo_w, + vec![ + container(PosReport::new(LAYER_COMBO_ID, combo_btn)).width(Fill).into(), + tool_row(row2, active_tool), + tool_row(row3, active_tool), + ], ) - .width(Length::Fixed(combo_w)) - .height(Fill) - .align_y(iced::Center) - .padding(Padding { - top: 4.0, - bottom: 4.0, - left: 4.0, - right: 4.0, - }) - .into() } RibbonItem::PropertiesGroup { match_prop } => { @@ -934,17 +955,10 @@ pub(super) fn render_large<'a>( } else { render_large( &RibbonItem::LargeTool(match_prop.clone()), - active_tool, - open_dd, - last_cmd, - state, - layer_infos, - active_layer, - active_color, - active_linetype, - active_lineweight, - style_ctx, - false, + &RenderCtx { + compact: false, + ..*ctx + }, ) }; @@ -1005,6 +1019,8 @@ pub(super) fn render_large<'a>( ); let lw_row = prop_row(LwItem(active_lineweight).to_string(), PROP_LW_ID, None); + // Top-pinned like every combo panel, so the Color dropdown shares + // the same baseline as the Layers / style selectors. let combos = container( column![ PosReport::new(PROP_COLOR_ID, color_row), @@ -1015,12 +1031,12 @@ pub(super) fn render_large<'a>( .align_x(iced::Left), ) .height(Fill) - .align_y(iced::Center) + .align_y(iced::Top) .padding(Padding { - top: 4.0, - bottom: 4.0, + top: COMBO_PANEL_PAD.top, + bottom: COMBO_PANEL_PAD.bottom, left: 0.0, - right: 4.0, + right: COMBO_PANEL_PAD.right, }); row![mp_el, combos] @@ -1068,54 +1084,15 @@ pub(super) fn render_large<'a>( let items_panel: Element = iced::widget::Space::new().width(0).height(0).into(); - // ── tool rows below combo ── - let make_tool_row = |tools: &[ToolDef]| -> Element { - let btns: Vec> = tools - .iter() - .map(|t| { - let is_active = active_tool.as_deref() == Some(t.id); - let tip = t!(t.label); - let event = t.event.clone(); - let icon_el: Element = make_icon(t.icon, 16.0); - let msg = module_event_to_message(event); - tooltip( - button(icon_el) - .on_press(msg) - .style(move |theme: &Theme, status| { - tool_btn_style(theme, is_active, status) - }) - .padding([2, 5]), - make_tip(tip.to_string()), - TipPos::Right, - ) - .gap(4.0) - .delay(Duration::from_millis(400)) - .style(tip_style) - .into() - }) - .collect(); - row(btns).spacing(2).align_y(iced::Center).into() - }; - let mut col_items: Vec> = vec![container(row![PosReport::new(*combo_id, combo_btn), items_panel].spacing(0)) .width(Fill) .into()]; for row_tools in rows { - col_items.push(make_tool_row(row_tools)); + col_items.push(tool_row(row_tools, active_tool)); } - container(column(col_items).spacing(3).align_x(iced::Left)) - .width(Length::Fixed(STYLE_COMBO_W)) - .height(Fill) - .align_y(iced::Center) - .padding(Padding { - top: 4.0, - bottom: 4.0, - left: 4.0, - right: 4.0, - }) - .into() + combo_panel_col(STYLE_COMBO_W, col_items) } } }