Fix unclickable tray submenus by drilling down inside the popup (#6703)

* Fix unclickable tray submenus by drilling down inside the popup

Clicking a tray menu entry that has children was a silent no-op: the
row called QsMenuEntry.display(), which renders a *platform* menu, and
Quickshell refuses that unless the shell root sets `//@ pragma
UseQApplication` -- shell.qml does not. The log shows "Cannot display
PlatformMenuEntry as quickshell was not started in QApplication mode"
and nothing opens. Apps whose whole menu is submenus, like
radiotray-ng's station list, were unusable.

Adding the pragma would be the wrong fix: it switches the entire shell
from QGuiApplication to QApplication, dragging QtWidgets into the
process and changing application-class behavior for the sake of one
popup -- which would then render as an unstyled platform menu beside
omarchy's own popup styling anyway.

Instead, submenus drill down inside the existing popup. A child
QsMenuEntry inherits QsMenuHandle, so it can feed a nested QsMenuOpener
and render through the same row delegate. Each level keeps its own live
opener on a stack -- a child entry is owned by its parent opener's
model, so collapsing to a single reassigned opener would destroy the
very entry being displayed. A back header row walks out one level; at
the root the menu renders exactly as before, and items without a
DBusMenu still use the platform fallback.

* Destroy submenu openers deepest-first and reset before switching items

resetTrayMenu() destroyed openers front-to-back and only cleared
submenuStack afterward. A deeper opener's menu entry is owned by its
parent's children model, so destroying the parent first could
invalidate an entry a still-live child opener referenced. Clear the
stack before tearing anything down, then destroy deepest-first so a
child is always gone before the parent whose model owns its entry.

openTrayMenu() reassigned activeTrayItem before calling resetTrayMenu().
trayMenuOpener.menu binds to activeTrayItem.menu, so that reassignment
immediately swaps what the root opener's children expose -- invalidating
entries any live submenu opener still referenced, before resetTrayMenu()
got a chance to tear them down. Reset first, then switch items.

Thanks @Copilot for catching both.

* Defer submenu reset until the popup's fade-out actually finishes

onTrayMenuOpenChanged reset the submenu stack the instant trayMenuOpen
went false, but the popup stays visible for the whole 140ms opacity
fade (PopupCard's own visible: open || card.opacity > 0) -- dismissing
from a submenu flashed the root menu mid-fade, and could resize or
reposition the fading popup if the two have different geometry.

Moved the reset to trayMenuPopup's own onVisibleChanged, which only
fires once the fade has genuinely completed. Switching to a different
tray item is unaffected: openTrayMenu() already resets explicitly
before assigning the new item, independent of whether the popup ever
dips to invisible (rapid reopen mid-fade never does).

Thanks @Copilot for catching this.

* Ignore tray menu clicks for a beat after changing submenu level

Changing level swaps the Repeater's model, which rebuilds the row
delegates synchronously -- a fresh row lands under a cursor that hasn't
moved. Submenu clicks used to be silent no-ops, which trained users to
click them twice, so that second click now fires whatever entry took
the spot. On radiotray-ng that means an accidental station switch.

Gate row and back-header clicks for 250ms after each level change. A
deliberate follow-up click is slower than that; a double-click is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Pin the submenu back header above the scrolling menu rows

The back header lived inside the Flickable's Column, so in a submenu
taller than the 420px cap -- exactly the long station list this
drill-down exists for -- scrolling down pushed the only way back off
screen, with no Escape or right-click alternative.

Move it into a pinned Column above the Flickable and account for its
height in the popup's contentHeight.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Reset the tray menu scroll offset when the drill-down is torn down

Flickable keeps its contentY across a model swap whenever the new
content is still tall enough to hold it. A menu dismissed while
scrolled therefore reopened part-way down with its first entries off
screen: reproducible on any tray app whose root menu outgrows the
420px cap, and now reachable on every app once a long submenu has
been scrolled.

Zero the offset in resetTrayMenu(), which runs both on teardown and
before switching items.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Toni Nowak <t.nowak@ai-flow.no>
Co-authored-by: David Heinemeier Hansson <david@hey.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Toni Nowak
2026-08-11 17:48:27 +02:00
committed by GitHub
co-authored by Claude Opus 5 Toni Nowak David Heinemeier Hansson
parent 05bb82b34e
commit 5b2c02dee3
+289 -107
View File
@@ -33,6 +33,82 @@ BarWidget {
property real revealProgress: expanded ? 1 : 0 property real revealProgress: expanded ? 1 : 0
readonly property real revealExtent: drawerExtent * revealProgress readonly property real revealExtent: drawerExtent * revealProgress
// Submenu drill-down state. QsMenuEntry.display() renders a *platform* menu,
// which Quickshell refuses unless the shell root sets `//@ pragma
// UseQApplication` - omarchy's shell.qml does not, so every submenu click was
// a silent no-op ("Cannot display PlatformMenuEntry as quickshell was not
// started in QApplication mode" in the shell log) and apps whose whole UI is
// submenus, e.g. radiotray-ng's station list, were unusable. QsMenuEntry
// inherits QsMenuHandle, so a child entry can feed a nested QsMenuOpener and
// render inside this popup instead of going through the platform. Each level
// keeps its own live opener: a child entry is owned by its parent opener's
// model, so collapsing the stack to a single opener would destroy the very
// entry being displayed (submenu turns up empty).
property var submenuStack: []
readonly property int submenuDepth: submenuStack.length
readonly property string currentTitle: submenuDepth > 0 ? submenuStack[submenuDepth - 1].title : ""
readonly property var currentChildren: submenuDepth > 0
? submenuStack[submenuDepth - 1].opener.children
: trayMenuOpener.children
// Changing level rebuilds the row delegates synchronously, so the next
// row lands under a cursor that hasn't moved. Submenu clicks used to be
// silent no-ops, which trained users to click them twice, and that second
// click would now fire whatever entry took the spot. Ignore row clicks for
// a beat after each level change; a deliberate follow-up click is slower.
property bool menuLevelSettling: false
Component {
id: submenuOpenerComponent
QsMenuOpener {}
}
Timer {
id: menuLevelSettleTimer
interval: 250
onTriggered: root.menuLevelSettling = false
}
function settleMenuLevel() {
menuLevelSettling = true
menuLevelSettleTimer.restart()
}
function resetTrayMenu() {
menuLevelSettling = false
menuLevelSettleTimer.stop()
// Flickable keeps its offset across a model swap whenever the new content
// is still tall enough to hold it, so a menu dismissed while scrolled
// would otherwise reopen part-way down with its first entries off screen.
trayMenuFlick.contentY = 0
// Clear the reactive stack before tearing anything down, so no binding can
// read a partially-destroyed opener while this runs. Then destroy deepest
// first: an inner opener's menu entry is owned by its parent's children
// model, so destroying a parent first would invalidate an entry a still-
// live child opener references.
var openers = submenuStack
submenuStack = []
for (var i = openers.length - 1; i >= 0; i--) openers[i].opener.destroy()
}
function enterSubmenu(entry, title) {
var opener = submenuOpenerComponent.createObject(root, { menu: entry })
if (!opener) return
var stack = submenuStack.slice()
stack.push({ opener: opener, title: title })
submenuStack = stack
settleMenuLevel()
}
function leaveSubmenu() {
if (submenuStack.length === 0) return
var stack = submenuStack.slice()
var top = stack.pop()
submenuStack = stack
top.opener.destroy()
settleMenuLevel()
}
function close() { function close() {
managePopupOpen = false managePopupOpen = false
trayMenuOpen = false trayMenuOpen = false
@@ -45,6 +121,11 @@ BarWidget {
return return
} }
// Reset before switching items: trayMenuOpener.menu binds to
// activeTrayItem.menu, so assigning a new item invalidates the old root's
// children immediately, before any nested opener referencing them would
// otherwise get torn down.
resetTrayMenu()
activeTrayItem = item activeTrayItem = item
activeTrayAnchor = anchorItem activeTrayAnchor = anchorItem
trayMenuOpen = true trayMenuOpen = true
@@ -442,133 +523,234 @@ BarWidget {
owner: root owner: root
bar: root.bar bar: root.bar
open: root.trayMenuOpen open: root.trayMenuOpen
// The card fades out over 140ms (visible stays true for that whole time --
// see PopupCard's own visible: open || card.opacity > 0), so resetting on
// "open" would swap a live submenu for the root menu mid-fade: a visible
// flash, and a resize/reposition if the two have different geometry. Wait
// for the fade to actually finish. Switching to a different tray item
// still resets immediately, from openTrayMenu() itself.
onVisibleChanged: if (!visible) root.resetTrayMenu()
padding: Style.space(8) padding: Style.space(8)
borderColor: Qt.rgba(root.foreground.r, root.foreground.g, root.foreground.b, 0.45) borderColor: Qt.rgba(root.foreground.r, root.foreground.g, root.foreground.b, 0.45)
contentWidth: trayMenuPopup.fittedContentWidth(Style.space(232)) contentWidth: trayMenuPopup.fittedContentWidth(Style.space(232))
contentHeight: trayMenuPopup.fittedContentHeight(trayMenuColumn.implicitHeight, Style.space(420)) contentHeight: trayMenuPopup.fittedContentHeight(menuHeaderHeight + trayMenuColumn.implicitHeight, Style.space(420))
Flickable { // Column skips invisible children but keeps reporting their height, so
id: trayMenuFlick // read the header's extent through its own visibility.
readonly property int menuHeaderHeight: menuHeader.visible ? menuHeader.implicitHeight : 0
Column {
id: trayMenuLayout
anchors.fill: parent anchors.fill: parent
contentWidth: width spacing: 0
contentHeight: trayMenuColumn.implicitHeight
clip: true
boundsBehavior: Flickable.StopAtBounds
flickableDirection: Flickable.VerticalFlick
interactive: contentHeight > height
ScrollBar.vertical: ScrollBar { policy: ScrollBar.AsNeeded }
// Header for a drilled-into submenu: names where we are and walks back
// out. Pinned above the Flickable rather than scrolling with the rows,
// so the way back stays reachable in a submenu taller than the card.
// Only present below the root level, so the root menu is unchanged.
Column { Column {
id: trayMenuColumn id: menuHeader
width: trayMenuFlick.width visible: root.submenuDepth > 0
width: trayMenuLayout.width
spacing: 0 spacing: 0
Repeater { Item {
model: trayMenuOpener.children id: menuBackRow
width: menuHeader.width
implicitHeight: Style.space(30)
delegate: Item { Rectangle {
id: menuRow anchors.fill: parent
required property var modelData radius: Math.max(2, Style.cornerRadius)
required property int index color: backMouse.containsMouse ? Style.hoverFillFor(root.foreground, root.foreground) : "transparent"
}
readonly property string rowText: String(modelData.text || "") Text {
readonly property string activeTitle: root.activeTrayItem ? String(root.activeTrayItem.title || root.activeTrayItem.id || "") : "" anchors.verticalCenter: parent.verticalCenter
readonly property bool rootTitleEntry: index === 0 && modelData.hasChildren && rowText.toLowerCase() === activeTitle.toLowerCase() anchors.left: parent.left
readonly property bool leadingSeparator: modelData.isSeparator && index <= 1 width: Style.space(22)
readonly property bool hiddenRow: rootTitleEntry || leadingSeparator horizontalAlignment: Text.AlignHCenter
text: "\u2039"
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
}
visible: !hiddenRow Text {
width: trayMenuColumn.width anchors.verticalCenter: parent.verticalCenter
implicitHeight: hiddenRow ? 0 : (modelData.isSeparator ? Style.space(11) : Style.space(30)) anchors.left: parent.left
opacity: modelData.enabled ? 1.0 : 0.45 anchors.leftMargin: Style.space(28)
anchors.right: parent.right
anchors.rightMargin: Style.space(10)
text: root.currentTitle
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
elide: Text.ElideRight
}
Rectangle { MouseArea {
visible: menuRow.modelData.isSeparator id: backMouse
anchors.left: parent.left anchors.fill: parent
anchors.leftMargin: Style.space(10) hoverEnabled: true
anchors.right: parent.right cursorShape: Qt.PointingHandCursor
anchors.rightMargin: Style.space(10) onClicked: {
anchors.verticalCenter: parent.verticalCenter if (root.menuLevelSettling) return
height: 1 // Reset before the model swap so the parent level shows from
color: Color.popups.border // the top (same ordering as the row delegate below).
opacity: 0.45 trayMenuFlick.contentY = 0
root.leaveSubmenu()
} }
}
}
Rectangle { Item {
visible: !menuRow.modelData.isSeparator width: menuHeader.width
anchors.fill: parent implicitHeight: Style.space(11)
radius: Math.max(2, Style.cornerRadius)
color: rowMouse.containsMouse && menuRow.modelData.enabled ? Style.hoverFillFor(root.foreground, root.foreground) : "transparent"
}
Text { Rectangle {
visible: !menuRow.modelData.isSeparator && menuRow.modelData.buttonType !== QsMenuButtonType.None anchors.left: parent.left
anchors.verticalCenter: parent.verticalCenter anchors.leftMargin: Style.space(10)
anchors.left: parent.left anchors.right: parent.right
width: Style.space(22) anchors.rightMargin: Style.space(10)
horizontalAlignment: Text.AlignHCenter anchors.verticalCenter: parent.verticalCenter
text: menuRow.modelData.checkState === Qt.Checked ? "\uf00c" : "" height: 1
color: root.foreground color: Color.popups.border
font.family: root.fontFamily opacity: 0.45
font.pixelSize: Style.font.bodySmall }
} }
}
Image { Flickable {
id: menuIcon id: trayMenuFlick
visible: !menuRow.modelData.isSeparator && String(menuRow.modelData.icon || "") !== "" width: trayMenuLayout.width
anchors.verticalCenter: parent.verticalCenter height: trayMenuLayout.height - trayMenuPopup.menuHeaderHeight
anchors.left: parent.left contentWidth: width
anchors.leftMargin: Style.space(24) contentHeight: trayMenuColumn.implicitHeight
width: Style.space(16) clip: true
height: Style.space(16) boundsBehavior: Flickable.StopAtBounds
fillMode: Image.PreserveAspectFit flickableDirection: Flickable.VerticalFlick
// Decode at physical pixels: IconImage uses the logical size, interactive: contentHeight > height
// which leaves PNG icons upscaled and blurry on HiDPI displays.
sourceSize.width: width * Screen.devicePixelRatio
sourceSize.height: height * Screen.devicePixelRatio
source: menuRow.modelData.icon
}
Text { ScrollBar.vertical: ScrollBar { policy: ScrollBar.AsNeeded }
visible: !menuRow.modelData.isSeparator
anchors.verticalCenter: parent.verticalCenter
anchors.left: parent.left
anchors.leftMargin: menuIcon.visible ? Style.space(46) : Style.space(28)
anchors.right: submenuGlyph.left
anchors.rightMargin: Style.space(8)
text: menuRow.rowText
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
elide: Text.ElideRight
}
Text { Column {
id: submenuGlyph id: trayMenuColumn
visible: !menuRow.modelData.isSeparator && menuRow.modelData.hasChildren width: trayMenuFlick.width
anchors.verticalCenter: parent.verticalCenter spacing: 0
anchors.right: parent.right
anchors.rightMargin: Style.space(10)
text: "\u203a"
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
}
MouseArea { Repeater {
id: rowMouse model: root.currentChildren
anchors.fill: parent
hoverEnabled: true delegate: Item {
enabled: !menuRow.modelData.isSeparator && menuRow.modelData.enabled id: menuRow
cursorShape: enabled ? Qt.PointingHandCursor : Qt.ArrowCursor required property var modelData
onClicked: { required property int index
if (menuRow.modelData.hasChildren) {
var point = menuRow.QsWindow.contentItem.mapFromItem(menuRow, menuRow.width, menuRow.height / 2) readonly property string rowText: String(modelData.text || "")
menuRow.modelData.display(menuRow.QsWindow.window, point.x, point.y) readonly property string activeTitle: root.activeTrayItem ? String(root.activeTrayItem.title || root.activeTrayItem.id || "") : ""
} else { // Both only ever describe the root menu; inside a submenu the first
menuRow.modelData.triggered() // rows are real entries and must not be swallowed.
root.close() readonly property bool atRoot: root.submenuDepth === 0
readonly property bool rootTitleEntry: atRoot && index === 0 && modelData.hasChildren && rowText.toLowerCase() === activeTitle.toLowerCase()
readonly property bool leadingSeparator: atRoot && modelData.isSeparator && index <= 1
readonly property bool hiddenRow: rootTitleEntry || leadingSeparator
visible: !hiddenRow
width: trayMenuColumn.width
implicitHeight: hiddenRow ? 0 : (modelData.isSeparator ? Style.space(11) : Style.space(30))
opacity: modelData.enabled ? 1.0 : 0.45
Rectangle {
visible: menuRow.modelData.isSeparator
anchors.left: parent.left
anchors.leftMargin: Style.space(10)
anchors.right: parent.right
anchors.rightMargin: Style.space(10)
anchors.verticalCenter: parent.verticalCenter
height: 1
color: Color.popups.border
opacity: 0.45
}
Rectangle {
visible: !menuRow.modelData.isSeparator
anchors.fill: parent
radius: Math.max(2, Style.cornerRadius)
color: rowMouse.containsMouse && menuRow.modelData.enabled ? Style.hoverFillFor(root.foreground, root.foreground) : "transparent"
}
Text {
visible: !menuRow.modelData.isSeparator && menuRow.modelData.buttonType !== QsMenuButtonType.None
anchors.verticalCenter: parent.verticalCenter
anchors.left: parent.left
width: Style.space(22)
horizontalAlignment: Text.AlignHCenter
text: menuRow.modelData.checkState === Qt.Checked ? "\uf00c" : ""
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
}
Image {
id: menuIcon
visible: !menuRow.modelData.isSeparator && String(menuRow.modelData.icon || "") !== ""
anchors.verticalCenter: parent.verticalCenter
anchors.left: parent.left
anchors.leftMargin: Style.space(24)
width: Style.space(16)
height: Style.space(16)
fillMode: Image.PreserveAspectFit
// Decode at physical pixels: IconImage uses the logical size,
// which leaves PNG icons upscaled and blurry on HiDPI displays.
sourceSize.width: width * Screen.devicePixelRatio
sourceSize.height: height * Screen.devicePixelRatio
source: menuRow.modelData.icon
}
Text {
visible: !menuRow.modelData.isSeparator
anchors.verticalCenter: parent.verticalCenter
anchors.left: parent.left
anchors.leftMargin: menuIcon.visible ? Style.space(46) : Style.space(28)
anchors.right: submenuGlyph.left
anchors.rightMargin: Style.space(8)
text: menuRow.rowText
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
elide: Text.ElideRight
}
Text {
id: submenuGlyph
visible: !menuRow.modelData.isSeparator && menuRow.modelData.hasChildren
anchors.verticalCenter: parent.verticalCenter
anchors.right: parent.right
anchors.rightMargin: Style.space(10)
text: "\u203a"
color: root.foreground
font.family: root.fontFamily
font.pixelSize: Style.font.bodySmall
}
MouseArea {
id: rowMouse
anchors.fill: parent
hoverEnabled: true
enabled: !menuRow.modelData.isSeparator && menuRow.modelData.enabled
cursorShape: enabled ? Qt.PointingHandCursor : Qt.ArrowCursor
onClicked: {
if (root.menuLevelSettling) return
if (menuRow.modelData.hasChildren) {
// Reset scroll BEFORE swapping the model: the swap destroys
// this delegate synchronously and ids stop resolving after.
trayMenuFlick.contentY = 0
root.enterSubmenu(menuRow.modelData, menuRow.rowText)
} else {
menuRow.modelData.triggered()
root.close()
}
} }
} }
} }