From ca355ea19df81671356f1d33e426afee331ef306 Mon Sep 17 00:00:00 2001 From: Ole Bohmbach Date: Sun, 1 Nov 2026 22:16:17 +0100 Subject: [PATCH] Give every Settings tab the same width, with air before the scroll bar (#151) The Settings scroll area no longer shrinks to each tab's content, so every tab has the same width and the scroll bar stays in one place; settings keep 14 points clear of it. --- crates/ui-egui/src/settings_ui.rs | 23 +++++++++++++------- crates/ui-egui/tests/smoke.rs | 35 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/settings_ui.rs b/crates/ui-egui/src/settings_ui.rs index 0f5b55c6..abe72c27 100644 --- a/crates/ui-egui/src/settings_ui.rs +++ b/crates/ui-egui/src/settings_ui.rs @@ -10,6 +10,7 @@ use craft_anything_engine::Mode; use egui::{RichText, Ui, Vec2}; use std::path::PathBuf; +/// Draws the Settings modal and the content for its selected tab. pub fn show(app: &mut CraftApp, ctx: &egui::Context) { if !app.state.settings_open { return; @@ -93,13 +94,21 @@ pub fn show(app: &mut CraftApp, ctx: &egui::Context) { let content_w = w - ui.spacing().scroll.allocated_width(); // As tall as the window allows, so a tab's settings aren't cut off at the fold. let h = (ctx.content_rect().height() - 220.0).clamp(280.0, 560.0); - egui::ScrollArea::vertical().min_scrolled_height(h).max_height(h).max_width(content_w).show(ui, |ui| match app.state.settings_tab { - SettingsTab::General => general(app, ui), - SettingsTab::Engines => engines(app, ui), - SettingsTab::Ai => ai(app, ui), - SettingsTab::Builds => builds(app, ui), - SettingsTab::Plugins => plugins(app, ui), - SettingsTab::Safety => safety(app, ui), + // Not shrunk to the tab's content: that gave each tab its own width and put the + // scroll bar in a different place on every page. + let tab = app.state.settings_tab; + egui::ScrollArea::vertical().auto_shrink([false, false]).min_scrolled_height(h).max_height(h).max_width(w).show(ui, |ui| { + // The bar sits at the edge of the area, so the air between it and the + // settings is taken from the settings' side. + ui.set_max_width(content_w - 14.0); + match tab { + SettingsTab::General => general(app, ui), + SettingsTab::Engines => engines(app, ui), + SettingsTab::Ai => ai(app, ui), + SettingsTab::Builds => builds(app, ui), + SettingsTab::Plugins => plugins(app, ui), + SettingsTab::Safety => safety(app, ui), + } }); }); }); diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index 3bb153ed..23dd300b 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -1027,6 +1027,41 @@ fn settings_window_tabs_draw() { h.get_by_label("Studio Light").click(); h.run_steps(3); assert_eq!(h.state().settings.theme, "studio-light"); + + // At the same window size, the same tab keeps its content bounds with and without scrolling. + let content_bounds = |overflow: bool| { + let mut h = harness_sized(egui::vec2(1000.0, 700.0), move || { + let mut a = demo::app(demo::engine(Mode::Beginner, true)); + a.state.settings_open = true; + a.state.settings_tab = craft_anything_ui_egui::state::SettingsTab::Safety; + a + }); + h.run_steps(3); + if overflow { + h.state_mut().view.project = Some("Test project".into()); + h.state_mut().view.project_allows = (0..20).map(|n| format!("extra tool {n}")).collect(); + h.run_steps(2); + } + let content = h.get_by_label("Run to study").rect(); + let close_right = edge(&h); + let scrollbars: Vec<_> = h.query_all_by_role(egui::accesskit::Role::ScrollBar).map(|node| node.rect()).collect(); + (content, close_right, scrollbars) + }; + let (short_content, short_window_right, short_scrollbars) = content_bounds(false); + let (long_content, long_window_right, long_scrollbars) = content_bounds(true); + let scrollbar = long_scrollbars + .iter() + .copied() + .filter(|r| r.left() < long_window_right) + .min_by(|a, b| a.left().total_cmp(&b.left())) + .expect("the overflowing Settings tab should show a scrollbar"); + assert!((short_window_right - long_window_right).abs() < 0.1, "the windows have different widths"); + assert!(short_scrollbars.iter().all(|r| r.left() > scrollbar.left()), "the short tab unexpectedly scrolls"); + assert_eq!(short_content, long_content, "the Settings content bounds changed when scrolling"); + assert!( + long_content.right() <= scrollbar.left() - 14.0, + "the content should stay at least 14 points before the scrollbar: content={long_content:?}, scrollbar={scrollbar:?}" + ); } #[test]