diff --git a/shell/Ui/BarIconButton.qml b/shell/Ui/BarIconButton.qml index e2af8808a76..8629003c4dd 100644 --- a/shell/Ui/BarIconButton.qml +++ b/shell/Ui/BarIconButton.qml @@ -11,6 +11,10 @@ WidgetButton { property bool debugOpticalBounds: Quickshell.env("OMARCHY_DEBUG_BAR_ICONS") === "1" readonly property real opticalCenterErrorX: glyph.visible ? glyph.paintedCenterX - opticalCanvas.width / 2 : 0 readonly property real glyphPaintedWidth: glyph.visible ? glyph.tightWidth : 0 + // Forwards the loaded vector icon, if any. Loader.item is statically + // QObject (reading .implicitWidth off it trips missing-property), so the + // bar measures through this untyped alias instead. Null on the glyph path. + readonly property var iconContentItem: iconLoader.item readonly property real glyphBaselineY: glyph.visible ? glyph.baselineY : 0 readonly property int glyphFontSize: glyph.visible ? glyph.renderedFontSize : 0 @@ -39,6 +43,7 @@ WidgetButton { } Loader { + id: iconLoader anchors.fill: parent visible: root.iconComponent !== null sourceComponent: root.iconComponent diff --git a/shell/Ui/WidgetButton.qml b/shell/Ui/WidgetButton.qml index 87d18050cf2..a8e316fbe64 100644 --- a/shell/Ui/WidgetButton.qml +++ b/shell/Ui/WidgetButton.qml @@ -62,6 +62,10 @@ Item { // Width of the painted label, for bar chrome that wants to line up with the // text rather than with the slot it sits in. Zero on icon-only buttons. readonly property real labelWidth: label.visible ? label.implicitWidth : 0 + // Tight painted width of the label: implicitWidth above includes the + // font's side bearings, which would pad pills a pixel or two wider per + // side than tight-measured icon glyphs. Zero on icon-only buttons. + readonly property real labelTightWidth: label.visible ? Math.max(0, labelMetrics.tightBoundingRect.width) : 0 visible: hasVisualContent || keepSpace opacity: !hasVisualContent || concealed ? 0 : (dimmed ? 0.45 : 1) @@ -72,6 +76,14 @@ Item { NumberAnimation { duration: 140; easing.type: Easing.OutCubic } } + TextMetrics { + id: labelMetrics + // Same font the label paints with, so tight bounds match the ink. + font.family: root.fontFamily + font.pixelSize: root.fontSize + text: root.text + } + Text { id: label textFormat: Text.PlainText diff --git a/shell/plugins/bar/Bar.qml b/shell/plugins/bar/Bar.qml index be9fece40f4..da0d163dfd0 100644 --- a/shell/plugins/bar/Bar.qml +++ b/shell/plugins/bar/Bar.qml @@ -1737,6 +1737,10 @@ Item { id: horizontalModuleList Row { + // No spacing here: every ModuleSlot pads itself from its own + // painted width (see slotPad), so ink-to-ink stays uniform whatever + // each widget paints. A fixed spacing would stack on top of the + // widest bearings instead of absorbing them. spacing: 0 Repeater { @@ -1755,6 +1759,7 @@ Item { id: verticalModuleList Column { + // As above: per-slot padding carries the gaps, not the positioner. spacing: 0 Repeater { @@ -1809,10 +1814,75 @@ Item { var key = root.vertical ? "openPanelIndicatorHeight" : "openPanelIndicatorWidth" var hint = activeItem && key in activeItem ? activeItem[key] : undefined if (hint !== undefined && hint !== null && hint > 0) return Math.round(hint) - return Math.max(Style.space(10), Math.round((root.vertical ? slot.height : slot.width) * 0.55)) - } - implicitWidth: activeItem && activeItem.visible ? (root.vertical ? root.barSize : activeItem.implicitWidth) : 0 - implicitHeight: activeItem && activeItem.visible ? activeItem.implicitHeight : 0 + return Math.max(Style.space(10), Math.round((root.vertical ? slot.contentHeight : slot.contentWidth) * 0.55)) + } + // Painted half-gap every slot holds its content away from the slot edge. + // Adjacent slots then land exactly 2*paintHalfGap ink-to-ink, whatever + // each widget paints โ€” icon slots, text pills, and paint that overflows + // its slot all end up on the same rhythm. + readonly property int paintHalfGap: Style.space(6) + // How far slot padding may intrude into a widget's own empty margins to + // enforce the gap above when a widget demands wider bearings. Never + // reaches paint, but neighbouring hit areas overlap by up to this much. + // Sized to cover the widest production bearing spread: a text pill at a + // scaled bar font carries ~halfGap + 4.5px of bearing per side against an + // icon's ~halfGap, so the cap must clear that or the pair keeps a + // subpixel residual (16.34px vs 16px at font 16). + readonly property int paintIntrude: Style.space(4) + // Size the slot lays out for its content (what implicitWidth used to be). + readonly property real contentWidth: activeItem && activeItem.visible + ? (root.vertical ? root.barSize : activeItem.implicitWidth) : 0 + readonly property real contentHeight: activeItem && activeItem.visible + ? activeItem.implicitHeight : 0 + // Tight painted extent along the layout axis, best effort, measured on + // the bar button: widgets keep paint metrics on the button inside the + // root, never on the root itself. Popup buttons nest deeper and must + // never be measured, so only the root and its direct children qualify. + // Tray is exempt: its chevron is a direct child but does not represent + // the drawer it opens. + readonly property var paintItem: { + if (!activeItem) return null + var id = root.canonicalWidgetId(moduleName) + if (id === "omarchy.spacer" || id === "omarchy.tray") return activeItem + return BarModel.paintChild(activeItem) || activeItem + } + // BarIconButton glyphs (which also covers text painted wider than its + // slot), WidgetButton labels, vector icon content, icon canvases. + // Opaque customs fall back to full-bleed โ€” extra air, never overlap. + readonly property real paintedExtent: { + var item = paintItem + if (!item) return 0 + if (root.vertical) { + if ("opticalSize" in item && item.opticalSize > 0) return item.opticalSize + return contentHeight + } + if ("glyphPaintedWidth" in item && item.glyphPaintedWidth > 0) return item.glyphPaintedWidth + if ("labelTightWidth" in item && item.labelTightWidth > 0) return item.labelTightWidth + if ("labelWidth" in item && item.labelWidth > 0) return item.labelWidth + // Vector icons size themselves under the canvas (usually to the icon + // font); measure the loaded item instead of assuming a full canvas, + // capped at the canvas so an over-reporting component cannot shrink + // its padding. + if ("iconContentItem" in item && item.iconContentItem + && item.iconContentItem.implicitWidth > 0) { + if ("opticalSize" in item && item.opticalSize > 0) + return Math.min(item.iconContentItem.implicitWidth, item.opticalSize) + return item.iconContentItem.implicitWidth + } + if ("opticalSize" in item && item.opticalSize > 0) return item.opticalSize + return contentWidth + } + // Symmetric compensation for this slot's own bearing. Negative bearings + // (paint wider than the slot) pad extra; the pure-gap spacer keeps its + // authored span and stays out of this. + readonly property real slotPad: { + var span = root.vertical ? contentHeight : contentWidth + if (!(span > 0)) return 0 + if (root.canonicalWidgetId(moduleName) === "omarchy.spacer") return 0 + return BarModel.slotPad(span, paintedExtent, paintHalfGap, paintIntrude) + } + implicitWidth: contentWidth + (root.vertical ? 0 : 2 * slotPad) + implicitHeight: contentHeight + (root.vertical ? 2 * slotPad : 0) width: implicitWidth height: implicitHeight z: modulePointer.dragging ? 100 : 0 @@ -1839,7 +1909,9 @@ Item { id: componentLoader active: !slot.qmlCustom && !slot.registered sourceComponent: slot.commandCustom ? customCommandModuleComponent : emptyModuleComponent - anchors.fill: parent + width: root.vertical ? parent.width : slot.contentWidth + height: root.vertical ? slot.contentHeight : parent.height + anchors.centerIn: parent opacity: slot.dragSource ? 0.22 : 1.0 onLoaded: { slot.injectProps() @@ -1851,7 +1923,9 @@ Item { id: registryLoader active: slot.registered sourceComponent: slot.registered ? slot.registryComponent : null - anchors.fill: parent + width: root.vertical ? parent.width : slot.contentWidth + height: root.vertical ? slot.contentHeight : parent.height + anchors.centerIn: parent opacity: slot.dragSource ? 0.22 : 1.0 onLoaded: { slot.injectProps() @@ -1863,7 +1937,9 @@ Item { id: qmlLoader active: slot.qmlCustom source: slot.qmlCustom ? root.customModuleSource(slot.entry) : "" - anchors.fill: parent + width: root.vertical ? parent.width : slot.contentWidth + height: root.vertical ? slot.contentHeight : parent.height + anchors.centerIn: parent opacity: slot.dragSource ? 0.22 : 1.0 onLoaded: { slot.injectProps() diff --git a/shell/plugins/bar/BarModel.js b/shell/plugins/bar/BarModel.js index 36900e5b482..cf149d15ea4 100644 --- a/shell/plugins/bar/BarModel.js +++ b/shell/plugins/bar/BarModel.js @@ -208,8 +208,54 @@ function nearestDropTarget(candidates, point, vertical) { return best } +// Symmetric slot padding that normalizes ink-to-ink gaps: a slot whose +// content paints `paintedExtent` wide inside `contentSpan` holds its paint +// `halfGap` from each slot edge, so neighbours always land 2*halfGap apart. +// Negative bearings (paint wider than the slot) pad extra instead of +// touching the neighbour. Zero spans stay collapsed so hidden widgets keep +// contributing no gap. `maxIntrude` lets the padding go negative into the +// widget's own empty margins to enforce gaps smaller than the widest +// bearing; it never reaches paint, but neighbouring hit areas overlap by +// that much, so keep it small. +function slotPad(contentSpan, paintedExtent, halfGap, maxIntrude) { + var span = Number(contentSpan) + if (!isFinite(span) || span <= 0) return 0 + var half = Number(halfGap) + if (!isFinite(half) || half <= 0) return 0 + var painted = Number(paintedExtent) + if (!isFinite(painted) || painted < 0) painted = span + var cap = Number(maxIntrude) + if (!isFinite(cap) || cap < 0) cap = 0 + return Math.max(-cap, half - (span - painted) / 2) +} + +// Bar buttons are always the widget root itself or a direct child of it; +// buttons inside the popup nest deeper and must never be measured. Returns +// the first object exposing bar paint metrics, or null. Duck-typed so the +// same function runs against live QObjects and plain test fixtures. +function hasPaintMetrics(value) { + if (!value) return false + return "glyphPaintedWidth" in value || "labelTightWidth" in value + || "labelWidth" in value || "iconContentItem" in value + || "opticalSize" in value +} + +function paintChild(item) { + if (!item) return null + if (hasPaintMetrics(item)) return item + var kids = item.children + if (!kids || typeof kids.length !== "number") return null + for (var i = 0; i < kids.length; i++) { + if (hasPaintMetrics(kids[i])) return kids[i] + } + return null +} + if (typeof module !== "undefined") { module.exports = { + hasPaintMetrics: hasPaintMetrics, + paintChild: paintChild, + slotPad: slotPad, isDrawnSlot: isDrawnSlot, pickDrawnSlot: pickDrawnSlot, pickPanelSlot: pickPanelSlot, diff --git a/shell/plugins/bar/README.md b/shell/plugins/bar/README.md index 5741c5f3114..a4d4e1216ab 100644 --- a/shell/plugins/bar/README.md +++ b/shell/plugins/bar/README.md @@ -178,3 +178,30 @@ declaring `kinds: ["bar-widget"]` and a `barWidget` entry point. See [../../README.md](../../README.md) for the manifest schema. Rescan, enable, and place third-party plugins with `omarchy-shell shell rescanPlugins`, `omarchy plugin enable`, and `omarchy bar move`. + +## Widget spacing contract + +Bar sections keep ink-to-ink gaps uniform at 12px: every module slot +measures its own painted width (icon glyphs, button labels, icon +canvases) and pads itself symmetrically so its paint sits 6px from each +slot edge. Paint that overflows its slot gets extra compensation instead +of touching its neighbour; padding may intrude up to 4px into a widget's +own empty margins to enforce the gap against wider widget bearings, never +into paint (all values scale with the bar font). Hidden widgets collapse to zero and contribute no gap. The +Row/Column itself uses no spacing โ€” the slots carry it all, and +`omarchy.spacer` keeps its authored span exempt. + +Widget authors should still follow the shared geometry so the +compensation stays small (custom `Item` modules expose no paint metrics, +so the bar treats them as full-bleed): + +- Icon widgets: extend `BarIconButton` from `qs.Ui` (default `slotSize` + `Style.bar.iconSlot`, 16px optical canvas). Compact status icons use + `slotSize: Style.bar.statusSlot`. +- Text pills: extend `WidgetButton` with the default `horizontalMargin` + (8.5). Custom widths should keep equivalent side bearings. +- Fully custom `Item` modules expose no paint metrics, so the bar pads + them as full-bleed: include side padding in the item itself to stay + compact. +- Composite widgets (tray, indicators, workspaces) normalize at their + outer boundary only; gaps between items inside them are the widget's own. diff --git a/test/shell.d/bar-module-spacing-test.sh b/test/shell.d/bar-module-spacing-test.sh new file mode 100755 index 00000000000..9ca8ac14da7 --- /dev/null +++ b/test/shell.d/bar-module-spacing-test.sh @@ -0,0 +1,207 @@ +#!/bin/bash + +set -euo pipefail + +source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" + +# Bar sections keep ink-to-ink gaps uniform by padding every module slot +# from its own painted width, instead of a fixed positioner spacing that +# stacks on top of the widest widget bearings. Lock the mechanism in: +# per-slot compensation driven by paint metrics, zero positioner spacing, +# and the pure-gap spacer exempt. +gutter_count=$(rg -c 'spacing: 0' "$ROOT/shell/plugins/bar/Bar.qml" || true) +[[ $gutter_count == "2" ]] || fail "bar module lists leave spacing to the slots" "spacing: 0 occurrences: $gutter_count" +pass "bar module lists leave spacing to the slots" + +for anchor in 'paintHalfGap' 'paintIntrude' 'slotPad' 'paintedExtent' 'paintChild' 'paintItem' 'labelTightWidth' 'iconContentItem' 'BarModel\.slotPad\(' 'BarModel\.paintChild\(activeItem\)' 'omarchy\.spacer'; do + rg -q "$anchor" "$ROOT/shell/plugins/bar/Bar.qml" || fail "bar normalizes slot spacing from painted widths" "$anchor" +done +pass "bar normalizes slot spacing from painted widths" + +run_node_test <<'JS' +const bar = requireFromRoot('shell/plugins/bar/BarModel.js') + +// Standard icon: 27px slot, ~11px of tight glyph paint. The padding goes +// slightly negative here: it intrudes into the widget's own empty margin +// to enforce the gap, never into paint. +assertEqual(bar.slotPad(27, 11, 6, 3), -2, 'an icon slot enforces the half gap') +// Text pill: 30px slot, ~13px label. +assertEqual(bar.slotPad(30, 13, 6, 3), -2.5, 'a text pill enforces the half gap') +// Overflowing paint (icon + percentage in an icon slot) pads extra +// instead of touching its neighbour. +assertEqual(bar.slotPad(27, 43, 6, 3), 14, 'overflowing paint is compensated, not clipped') +// Intrusion is bounded: even a wildly over-reported bearing only ever +// overlaps neighbouring hit areas by the cap, never paint. +assertEqual(bar.slotPad(60, 10, 6, 3), -3, 'intrusion stops at the cap') +assertEqual(bar.slotPad(100, 4, 6, 3), -3, 'a lying bearing still stops at the cap') +// Without the cap the padding stays outward-only, as before. +assertEqual(bar.slotPad(30, 13, 6), 0, 'no cap means outward-only padding') +// Hidden widgets stay collapsed and contribute no gap. +assertEqual(bar.slotPad(0, 0, 6, 3), 0, 'a zero span stays collapsed') +assertEqual(bar.slotPad(-4, 0, 6, 3), 0, 'a negative span stays collapsed') +assertEqual(bar.slotPad(27, 11, 0, 3), 0, 'a zero half gap pads nothing') + +// The identity the bar relies on: pad + own bearing on both sides of a +// pair always sums to the full uniform gap. +function pairGap(spanA, paintedA, spanB, paintedB, half, cap) { + const bearing = (span, painted) => (span - painted) / 2 + return bar.slotPad(spanA, paintedA, half, cap) + bearing(spanA, paintedA) + + bar.slotPad(spanB, paintedB, half, cap) + bearing(spanB, paintedB) +} +assertEqual(pairGap(27, 11, 27, 11, 6, 3), 12, 'two icons land on the uniform gap') +assertEqual(pairGap(27, 11, 30, 13, 6, 3), 12, 'icon and pill land on the uniform gap') +assertEqual(pairGap(27, 43, 27, 11, 6, 3), 12, 'overflowing paint and icon land on the uniform gap') +// Scaled bar font (half gap 8, intrude 5): a wide-bearing text pill next to +// a status icon. The pill needs 4.5px of intrusion; a narrower cap would +// clamp and leave a 16.5px residual instead of the uniform 16px. +assertEqual(bar.slotPad(37, 12, 8, 5), -4.5, 'a scaled pill intrudes past the old cap') +assertEqual(bar.slotPad(37, 12, 8, 4), -4, 'a narrower cap clamps instead of equalizing') +assertEqual(pairGap(37, 12, 28, 11, 8, 5), 16, 'scaled pill and icon land on the uniform gap') + +// The bar measures the button inside the widget root: paint metrics live +// on the button, never on the root, while popup buttons nest deeper. +const bareButton = { labelWidth: 40, children: [] } +assertEqual(bar.paintChild(bareButton), bareButton, 'a root that is the button measures itself') +const service = { refresh: function() {} } +const button = { glyphPaintedWidth: 43 } +const widget = { children: [service, button] } +assertEqual(bar.paintChild(widget), button, 'a button child of the root is measured') +assertEqual(bar.paintChild({ children: [{ children: [button] }] }), null, 'popup-depth buttons are never measured') +assertEqual(bar.paintChild({ children: [service] }), null, 'a root without metrics measures nothing') +assertEqual(bar.paintChild(null), null, 'a missing widget measures nothing') +assertEqual(bar.hasPaintMetrics(button), true, 'glyph paint is a metric') +assertEqual(bar.hasPaintMetrics(service), false, 'service objects carry no metrics') +JS + +if ! command -v quickshell >/dev/null 2>&1; then + pass "quickshell not installed; skipping bar module spacing runtime test" + exit 0 +fi + +# The compensation above only works when the kit reports truthful paint +# metrics, including text painted wider than its slot. Exercise the real +# buttons the bar measures. Positioner reflow needs a rendered scene, so +# the fixture opens a window on the offscreen platform: no compositor +# required, nothing maps on screen. +test_tmp=$(mktemp -d) +trap 'rm -rf "$test_tmp"' EXIT + +ln -s "$ROOT/shell/Ui" "$test_tmp/Ui" +ln -s "$ROOT/shell/Commons" "$test_tmp/Commons" + +cat >"$test_tmp/shell.qml" <<'QML' +import QtQuick +import Quickshell +import qs.Commons +import qs.Ui + +ShellRoot { + id: root + + function fail(message) { + console.log("RESULT fail " + message) + Qt.quit() + } + + Component.onCompleted: Qt.callLater(function() { + narrowWidth = pill.labelWidth + narrowTight = pill.labelTightWidth + pill.text = "OpenCode ยท 82%" + overflow.text = "X 100%" + settle.restart() + }) + + property real narrowWidth: 0 + property real narrowTight: 0 + + Timer { + id: settle + interval: 300 + onTriggered: { + if (!(pill.labelWidth > narrowWidth)) { + fail("pill paint width does not track content") + return + } + if (!(pill.labelTightWidth > narrowTight)) { + fail("pill tight width does not track content") + return + } + // Tight bounds exclude the font side bearings, so pills pad from ink + // like tight-measured icon glyphs. Ink may overshoot the advance + // slightly, so allow a small tolerance around the label width. + if (!(pill.labelTightWidth > 0 && Math.abs(pill.labelTightWidth - pill.labelWidth) <= 5)) { + fail("pill tight width is not a sane measure of its label") + return + } + if (!(glyph.glyphPaintedWidth > 0 && glyph.glyphPaintedWidth < Style.bar.iconSlot)) { + fail("icon paint width is not inside its slot") + return + } + if (!(overflow.glyphPaintedWidth > Style.bar.iconSlot)) { + fail("overflowing paint is not visible past its slot") + return + } + if (overflow.opticalSize !== Style.bar.iconCanvas) { + fail("icon canvas does not match the shared canvas") + return + } + // Vector icons size themselves under the canvas (like the font-sized + // status icons); the bar must see that size, not a full canvas. + if (!(vector.iconContentItem && vector.iconContentItem.implicitWidth === 12)) { + fail("vector icon content is not measurable through the button") + return + } + console.log("RESULT pass") + Qt.quit() + } + } + + QtObject { + id: testBar + property bool vertical: false + property int barSize: Style.bar.sizeHorizontal + property string fontFamily: Style.font.family + property color barForeground: "white" + property color urgent: "red" + property bool foregroundAnimationEnabled: false + function registerClickTarget(target) {} + function unregisterClickTarget(target) {} + function hideTooltip(target) {} + function showTooltip(target, text) {} + } + + Window { + visible: true + width: 400 + height: 60 + + Row { + WidgetButton { id: pill; bar: testBar; text: "X" } + BarIconButton { id: glyph; bar: testBar; text: "x" } + BarIconButton { id: overflow; bar: testBar; text: "x" } + BarIconButton { + id: vector + bar: testBar + iconComponent: Component { + Rectangle { implicitWidth: 12; implicitHeight: 12 } + } + } + } + } +} +QML + +output=$(QT_QPA_PLATFORM=offscreen timeout 15 env \ + QML2_IMPORT_PATH="$ROOT/shell${QML2_IMPORT_PATH:+:$QML2_IMPORT_PATH}" \ + QML_IMPORT_PATH="$ROOT/shell${QML_IMPORT_PATH:+:$QML_IMPORT_PATH}" \ + quickshell -p "$test_tmp" --no-color 2>&1) || { + printf '%s\n' "$output" >&2 + fail "bar module spacing fixture exits cleanly" +} + +if ! grep -q 'RESULT pass' <<<"$output"; then + printf '%s\n' "$output" >&2 + fail "bar paint metrics track content for slot compensation" +fi + +pass "bar paint metrics track content for slot compensation"