From d52b69c49b3501c0a70feaebb254ff381701470d Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Tue, 2 Jun 2026 22:19:09 -0400 Subject: [PATCH] Fix button hover border layout shift --- shell/Ui/Button.qml | 36 +++++- test/shell.d/button-border-stability-test.sh | 111 +++++++++++++++++++ 2 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 test/shell.d/button-border-stability-test.sh diff --git a/shell/Ui/Button.qml b/shell/Ui/Button.qml index c88521f2..2c093b4b 100644 --- a/shell/Ui/Button.qml +++ b/shell/Ui/Button.qml @@ -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 diff --git a/test/shell.d/button-border-stability-test.sh b/test/shell.d/button-border-stability-test.sh new file mode 100644 index 00000000..8f5b4ce1 --- /dev/null +++ b/test/shell.d/button-border-stability-test.sh @@ -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"