From e329f1cc6f9a2fe00314cef9f608ab2c21efaf7f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 6 May 2026 15:45:07 +0200 Subject: [PATCH] codify UI spacing constants (T-86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds lib/widgets/src/spacing.dart with three categories of named constants — insets (clideInsetHairline / Tight / Icon / Standard / Text), gaps (clideGapTight / Standard / Section / SectionLarge / Major / Column), and sizes (clideIconMicro / Caption / Standard / HitTarget / Emphatic, clideControlHeight). Migrates MultitabPane to consume the constants and updates the ui-design geometry reference to point at them. Inline pixel literals in widget code were drifting (12 here, 6 there, 28 elsewhere) — pulling them through named symbols makes the "uniform inner spacing" rule enforceable instead of eyeballed. Co-Authored-By: Claude --- .../skills/ui-design/references/geometry.md | 21 +++-- .pql/pql-plan.json | 24 +++-- CHANGELOG.md | 4 + lib/widgets/src/multitab_pane.dart | 27 +++--- lib/widgets/src/spacing.dart | 89 +++++++++++++++++++ lib/widgets/widgets.dart | 1 + test/widgets/multitab_pane_test.dart | 11 ++- 7 files changed, 151 insertions(+), 26 deletions(-) create mode 100644 lib/widgets/src/spacing.dart diff --git a/.claude/skills/ui-design/references/geometry.md b/.claude/skills/ui-design/references/geometry.md index 2d7bf0d1..5d5452c7 100644 --- a/.claude/skills/ui-design/references/geometry.md +++ b/.claude/skills/ui-design/references/geometry.md @@ -4,11 +4,22 @@ Principles for placing icons, buttons, and text inside controls. Apply when building tab strips, list items, buttons with affordances, or anything where actions sit next to content. -> Numeric values in this doc will move to `ClideSpacing` constants -> (`lib/widgets/src/spacing.dart`) — see T-86. Until then, the -> constants used in the codebase: `12` (text inset), `6` (icon -> breathing), `8` (standard gap), `4` (tight gap), `16` (icon hit -> area), `28` (button / row height). +> Constants live in `lib/widgets/src/spacing.dart` — pull from there +> instead of inlining literals: +> +> | Concept | Constant | +> |-------------------------|-------------------------| +> | Hairline (2px) | `clideInsetHairline` | +> | Tight inset (4px) | `clideInsetTight` | +> | Uniform icon margin (6) | `clideInsetIcon` | +> | Standard inset (8px) | `clideInsetStandard` | +> | Text-content inset (12) | `clideInsetText` | +> | Tight / standard gap | `clideGapTight` / `clideGapStandard` | +> | Section / major gap | `clideGapSection` / `clideGapMajor` | +> | Micro icon (10) | `clideIconMicro` | +> | Standard icon (14) | `clideIconStandard` | +> | Hit-target (16) | `clideIconHitTarget` | +> | Control height (28) | `clideControlHeight` | ## Uniform inner spacing rule diff --git a/.pql/pql-plan.json b/.pql/pql-plan.json index 6d67b6df..995f0359 100644 --- a/.pql/pql-plan.json +++ b/.pql/pql-plan.json @@ -1,5 +1,5 @@ { - "exported_at": "2026-05-06T13:38:19Z", + "exported_at": "2026-05-06T13:45:07Z", "decisions": [ { "id": "D-1", @@ -2696,19 +2696,19 @@ "id": "T-85", "type": "task", "title": "consolidate theme-ui into ui-design skill with theme/surface/geometry/icons references", - "status": "in_progress", + "status": "done", "priority": "medium", "created_at": "2026-05-06 13:28:58", - "updated_at": "2026-05-06 13:29:05" + "updated_at": "2026-05-06 13:38:38" }, { "id": "T-86", "type": "task", "title": "codify UI spacing constants (gaps, paddings, control sizes)", - "status": "backlog", + "status": "in_progress", "priority": "medium", "created_at": "2026-05-06 13:28:58", - "updated_at": "2026-05-06 13:28:58" + "updated_at": "2026-05-06 13:38:44" } ], "ticket_deps": null, @@ -4097,6 +4097,20 @@ "old_value": "backlog", "new_value": "in_progress", "changed_at": "2026-05-06 13:29:05" + }, + { + "ticket_id": "T-85", + "field": "status", + "old_value": "in_progress", + "new_value": "done", + "changed_at": "2026-05-06 13:38:38" + }, + { + "ticket_id": "T-86", + "field": "status", + "old_value": "backlog", + "new_value": "in_progress", + "changed_at": "2026-05-06 13:38:44" } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a322e29..136498c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. - Claude pane uses `MultitabPane` for primary + secondaries — drops ~100 lines of bespoke tab-strip code, gains drag-to-reorder. +- UI spacing constants live in `lib/widgets/src/spacing.dart` — + `clideInset*` for paddings, `clideGap*` for sibling distances, + `clideIcon*` / `clideControlHeight` for control sizes. Inline + pixel literals replaced where they were repeated. ### Changed diff --git a/lib/widgets/src/multitab_pane.dart b/lib/widgets/src/multitab_pane.dart index c0a24b2a..b655348b 100644 --- a/lib/widgets/src/multitab_pane.dart +++ b/lib/widgets/src/multitab_pane.dart @@ -4,6 +4,7 @@ import 'package:clide/widgets/src/clide_tappable.dart'; import 'package:clide/widgets/src/clide_text.dart'; import 'package:clide/widgets/src/icons/x.dart'; import 'package:clide/widgets/src/multitab_controller.dart'; +import 'package:clide/widgets/src/spacing.dart'; import 'package:flutter/widgets.dart'; typedef MultitabBuilder = Widget Function(BuildContext context, MultitabEntry entry); @@ -27,7 +28,7 @@ class MultitabPane extends StatelessWidget { this.onAddRequested, this.allowReorder = true, this.keepAlive = false, - this.tabHeight = 28, + this.tabHeight = clideControlHeight, }); final MultitabController controller; @@ -321,13 +322,13 @@ class _TabState extends State<_Tab> { builder: (context, _, __) => Container( constraints: BoxConstraints(minWidth: 96, maxWidth: 200), height: widget.tabHeight, - // Left margin stays at 12 (text breathing room). - // Right margin matches the close button's vertical - // breathing room ((tabHeight − iconSize) / 2 ≈ 6) so the - // gap around the icon is uniform on top, bottom, and right. + // Left: text inset (title breathing room). + // Right: when there's a close button, match its uniform + // icon margin so top/bottom/right are equal — see the + // ui-design `geometry.md` "no double-edge padding" rule. padding: EdgeInsets.only( - left: 12, - right: widget.onClose != null ? 6 : 12, + left: clideInsetText, + right: widget.onClose != null ? clideInsetIcon : clideInsetText, ), decoration: BoxDecoration( color: bg, @@ -351,14 +352,14 @@ class _TabState extends State<_Tab> { ), // Right column: close icon, fixed natural width. if (widget.onClose != null) ...[ - const SizedBox(width: 8), + const SizedBox(width: clideGapStandard), Opacity( opacity: _hovered || widget.active ? 1.0 : 0.0, child: ClideTappable( onTap: widget.onClose, builder: (context, hovered, _) => Container( - width: 16, - height: 16, + width: clideIconHitTarget, + height: clideIconHitTarget, alignment: Alignment.center, decoration: BoxDecoration( color: hovered ? tokens.listItemHoverBackground : null, @@ -366,7 +367,7 @@ class _TabState extends State<_Tab> { ), child: ClideIcon( const CloseIcon(), - size: 10, + size: clideIconMicro, color: hovered ? tokens.globalForeground : tokens.globalTextMuted, ), ), @@ -397,14 +398,14 @@ class _AddButton extends StatelessWidget { child: ClideTappable( onTap: onTap, builder: (context, hovered, _) => Container( - width: 28, + width: clideControlHeight, height: tabHeight, alignment: Alignment.center, decoration: BoxDecoration( color: hovered ? tokens.listItemHoverBackground : null, ), child: ClideText('+', - fontSize: 14, + fontSize: clideIconStandard, color: hovered ? tokens.globalForeground : tokens.globalTextMuted), ), ), diff --git a/lib/widgets/src/spacing.dart b/lib/widgets/src/spacing.dart new file mode 100644 index 00000000..306735d6 --- /dev/null +++ b/lib/widgets/src/spacing.dart @@ -0,0 +1,89 @@ +/// Spacing constants shared across widgets. +/// +/// Three categories — every widget that picks a literal pixel value +/// for layout should pull it from one of these instead: +/// +/// 1. **Insets** — padding inside containers around content +/// (text breathing room, icon micro-margin). +/// 2. **Gaps** — distance between siblings in a Row or Column. +/// 3. **Sizes** — control dimensions (icon glyph size, hit-target +/// outer size, button/row/tab height). +/// +/// Values are paired with the geometry rules under +/// `.claude/skills/ui-design/references/geometry.md` (uniform inner +/// spacing, no double-edge padding, two-column control pattern). +/// Adjust here only — never inline literals. +library; + +// --------------------------------------------------------------------------- +// Insets — padding inside containers +// --------------------------------------------------------------------------- + +/// Hairline. Used for divider thickness and the terminal cell padding +/// where 1px would clip glyphs. +const double clideInsetHairline = 2; + +/// Tight inset for compact controls (sidebar item gutter). +const double clideInsetTight = 4; + +/// Uniform breathing room around an icon inside a control. Pair with +/// a 16x16 hit target inside a 28-tall control: top/bottom auto-fall +/// to (28 − 16) / 2 = 6, so a matching right padding of 6 yields a +/// uniform border on the constrained sides. +const double clideInsetIcon = 6; + +/// Standard gap between adjacent inline elements in a row (icon → text, +/// text → action). Used as the in-row SizedBox between a title and its +/// trailing close button. +const double clideInsetStandard = 8; + +/// Text-content padding. Used as the leading horizontal padding of +/// text-bearing controls (tab title left padding, list-item gutter). +const double clideInsetText = 12; + +// --------------------------------------------------------------------------- +// Gaps — distance between siblings +// --------------------------------------------------------------------------- + +/// Tight gap between row segments inside a control. +const double clideGapTight = 4; + +/// Standard gap between sibling controls (between a title and an +/// action icon, between two list items in a flex row). +const double clideGapStandard = 8; + +/// Section internal gap (between a section header and its first row). +const double clideGapSection = 14; + +/// Section gap (between two sections in a stack). +const double clideGapSectionLarge = 20; + +/// Major gap between large blocks (between two columns in a row, +/// between header and content card). +const double clideGapMajor = 24; + +/// Welcome-screen-style gap between the two centered columns. +const double clideGapColumn = 56; + +// --------------------------------------------------------------------------- +// Sizes — control dimensions +// --------------------------------------------------------------------------- + +/// Micro icon glyph (close ×, dropdown chevron when paired with text). +const double clideIconMicro = 10; + +/// Caption-row icon (status bar, sidebar inline icons). +const double clideIconCaption = 13; + +/// Standard inline icon (icon rail, action buttons). +const double clideIconStandard = 14; + +/// Hit-target outer container around a micro icon. Provides hover +/// background and a comfortable click area; the icon centers inside. +const double clideIconHitTarget = 16; + +/// Emphatic icon (standalone affordances, primary action glyphs). +const double clideIconEmphatic = 18; + +/// Standard control height (tab, button, list row). +const double clideControlHeight = 28; diff --git a/lib/widgets/widgets.dart b/lib/widgets/widgets.dart index bb936971..83cb1a98 100644 --- a/lib/widgets/widgets.dart +++ b/lib/widgets/widgets.dart @@ -28,6 +28,7 @@ export 'src/multitab_pane.dart'; export 'src/clide_tappable.dart'; export 'src/clide_text.dart'; export 'src/clide_tooltip.dart'; +export 'src/spacing.dart'; export 'src/typography.dart'; export 'src/icons/check.dart'; export 'src/icons/chevron.dart'; diff --git a/test/widgets/multitab_pane_test.dart b/test/widgets/multitab_pane_test.dart index cb9ddfcd..522ab37f 100644 --- a/test/widgets/multitab_pane_test.dart +++ b/test/widgets/multitab_pane_test.dart @@ -1,3 +1,4 @@ +import 'package:clide/widgets/src/icons/x.dart'; import 'package:clide/widgets/widgets.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -5,6 +6,10 @@ import 'package:flutter_test/flutter_test.dart'; import '../helpers/kernel_fixture.dart'; import '../helpers/widget_harness.dart'; +Finder _closeIcons() => find.byWidgetPredicate( + (w) => w is ClideIcon && w.painter is CloseIcon, + ); + MultitabEntry entry(String id, {bool closeable = true, bool reorderable = true}) { return MultitabEntry( id: id, @@ -119,7 +124,7 @@ void main() { // A pinned tab has no close target; a closeable one does (it's // hidden via Opacity until hover, but still in the tree). // Two tabs total, one × glyph for 's'. - expect(find.text('×'), findsOneWidget); + expect(_closeIcons(), findsOneWidget); }); testWidgets('default close behavior removes the entry', (tester) async { @@ -130,7 +135,7 @@ void main() { harness(f, MultitabPane(controller: c, bodyBuilder: body)), ); - await tester.tap(find.text('×')); + await tester.tap(_closeIcons()); await tester.pumpAndSettle(); expect(c.entries.map((e) => e.id), ['p']); @@ -151,7 +156,7 @@ void main() { ); // Both tabs are closeable; tap the first × encountered. - await tester.tap(find.text('×').first); + await tester.tap(_closeIcons().first); await tester.pumpAndSettle(); expect(closed, isNotNull);