Fix button hover border layout shift

2026-06-02 22:19:09 -04:00
parent 62f3af2300
commit d52b69c49b
2 changed files with 142 additions and 5 deletions
@@ -67,18 +67,44 @@ BorderSurface {
  Keys.onEnterPressed: if (focusable) root.clicked()
  Keys.onSpacePressed: if (focusable) root.clicked()

-  implicitWidth: row.implicitWidth + horizontalPadding * 2 + borderLeft + borderRight
-  implicitHeight: row.implicitHeight + verticalPadding * 2 + borderTop + borderBottom
+  // Reserve the largest border any visual state can paint. Otherwise a
+  // borderless idle button grows by a pixel per side on hover/focus and
+  // relayouts neighboring controls.
+  implicitWidth: row.implicitWidth + horizontalPadding * 2 + _reservedBorderLeft + _reservedBorderRight
+  implicitHeight: row.implicitHeight + verticalPadding * 2 + _reservedBorderTop + _reservedBorderBottom
  radius: Style.cornerRadius

  readonly property bool hot: mouseArea.containsMouse || hasCursor
  readonly property bool _showFocusRing: focusable && activeFocus
  readonly property color _selectedColor: Style.selectedStateColor(root.foreground, root.accent)
  readonly property var _tooltipBorderSpec: Border.localOrSurfaceSpec("tooltip", "border", root.tooltipBorder, Color.tooltip.border, Math.max(1, Style.normalBorderWidth))
+  readonly property var _focusBorderSpec: Border.controlSpec("focus", root.foreground, root.accent)
+  readonly property var _hoverBorderSpec: Border.controlSpec("hover-cursor", root.foreground, root.accent)
  readonly property var _selectedBorderSpec: Border.controlSpec("selected", root.foreground, root.accent)
  readonly property var _normalBorderSpec: Border.controlSpec("normal", root.foreground, root.accent)
-  readonly property var _borderSpec: _showFocusRing ? Border.controlSpec("focus", root.foreground, root.accent)
-    : hot                      ? Border.controlSpec("hover-cursor", root.foreground, root.accent)
+  readonly property real _reservedBorderTop: Math.max(
+    focusable ? Border.top(_focusBorderSpec) : 0,
+    Border.top(_hoverBorderSpec),
+    Border.top(_selectedBorderSpec),
+    bordered ? Border.top(_normalBorderSpec) : 0)
+  readonly property real _reservedBorderRight: Math.max(
+    focusable ? Border.right(_focusBorderSpec) : 0,
+    Border.right(_hoverBorderSpec),
+    Border.right(_selectedBorderSpec),
+    bordered ? Border.right(_normalBorderSpec) : 0)
+  readonly property real _reservedBorderBottom: Math.max(
+    focusable ? Border.bottom(_focusBorderSpec) : 0,
+    Border.bottom(_hoverBorderSpec),
+    Border.bottom(_selectedBorderSpec),
+    bordered ? Border.bottom(_normalBorderSpec) : 0)
+  readonly property real _reservedBorderLeft: Math.max(
+    focusable ? Border.left(_focusBorderSpec) : 0,
+    Border.left(_hoverBorderSpec),
+    Border.left(_selectedBorderSpec),
+    bordered ? Border.left(_normalBorderSpec) : 0)
+  readonly property real _reservedContentLeftInset: _reservedBorderLeft + leftPadding
+  readonly property var _borderSpec: _showFocusRing ? _focusBorderSpec
+    : hot                      ? _hoverBorderSpec
    : selected                 ? (Border.controlHasWidth("selected") ? _selectedBorderSpec : (bordered ? _normalBorderSpec : Border.none()))
    : bordered                 ? _normalBorderSpec
    : Border.none()
@@ -127,7 +153,7 @@ BorderSurface {
    id: row
    anchors.verticalCenter: parent.verticalCenter
    anchors.left: root.leftAlign ? parent.left : undefined
-    anchors.leftMargin: root.leftAlign ? root.contentLeftInset : 0
+    anchors.leftMargin: root.leftAlign ? root._reservedContentLeftInset : 0
    anchors.horizontalCenter: root.leftAlign ? undefined : parent.horizontalCenter
    spacing: Style.spacing.controlGap

@@ -0,0 +1,111 @@
+#!/bin/bash
+source "$(dirname "$0")/base-test.sh"
+
+run_node_test <<'JS'
+const fs = require('fs')
+
+const buttonQml = fs.readFileSync(path.join(root, 'shell/Ui/Button.qml'), 'utf8')
+
+assert(
+  /anchors\.leftMargin:\s*root\.leftAlign \? root\._reservedContentLeftInset : 0/.test(buttonQml),
+  'Button left-aligned content uses reserved border inset'
+)
+
+assert(
+  !/implicitWidth:[^\n]*\bborderLeft\b/.test(buttonQml) && !/implicitHeight:[^\n]*\bborderTop\b/.test(buttonQml),
+  'Button implicit size does not depend on current hover/focus border'
+)
+JS
+
+if ! command -v quickshell >/dev/null 2>&1; then
+  pass "quickshell not installed; skipping Button hover geometry runtime test"
+  exit 0
+fi
+
+TMPDIR=$(mktemp -d)
+cleanup() {
+  [[ -d $TMPDIR ]] && rm -rf "$TMPDIR"
+}
+trap cleanup EXIT
+
+ln -s "$ROOT/shell/Ui" "$TMPDIR/Ui"
+ln -s "$ROOT/shell/Commons" "$TMPDIR/Commons"
+
+cat >"$TMPDIR/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()
+  }
+
+  function checkStable(button, width, height, label, next) {
+    Qt.callLater(function() {
+      if (button.implicitWidth !== width || button.implicitHeight !== height) {
+        root.fail(label + " changed from " + width + "x" + height + " to " + button.implicitWidth + "x" + button.implicitHeight)
+        return
+      }
+      next()
+    })
+  }
+
+  function runChecks() {
+    var plainWidth = plainButton.implicitWidth
+    var plainHeight = plainButton.implicitHeight
+    plainButton.hasCursor = true
+    checkStable(plainButton, plainWidth, plainHeight, "hover-cursor", function() {
+      plainButton.hasCursor = false
+      plainButton.selected = true
+      checkStable(plainButton, plainWidth, plainHeight, "selected", function() {
+        var focusWidth = focusableButton.implicitWidth
+        var focusHeight = focusableButton.implicitHeight
+        focusableButton.hasCursor = true
+        checkStable(focusableButton, focusWidth, focusHeight, "focusable hover-cursor", function() {
+          console.log("RESULT pass")
+          Qt.quit()
+        })
+      })
+    })
+  }
+
+  Component.onCompleted: {
+    Style.styleOverrides = ({
+      "hover-cursor-border-width": 3,
+      "selected-border-width": 5,
+      "focus-border-width": 7
+    })
+    Qt.callLater(runChecks)
+  }
+
+  Item {
+    Button {
+      id: plainButton
+      text: "Refresh"
+    }
+
+    Button {
+      id: focusableButton
+      text: "Save"
+      focusable: true
+    }
+  }
+}
+QML
+
+output=$(timeout 15 quickshell -p "$TMPDIR" --no-color 2>&1) || {
+  printf '%s\n' "$output" >&2
+  fail "Button hover geometry runtime fixture exits cleanly"
+}
+
+if ! grep -q "RESULT pass" <<<"$output"; then
+  printf '%s\n' "$output" >&2
+  fail "Button implicit geometry is stable across hover/selected states"
+fi
+
+pass "Button implicit geometry is stable across hover/selected states"