Skip to content

Keep menu empty-state text inside the card - #7653

Open
benwillems wants to merge 1 commit into
basecamp:quattrofrom
benwillems:fix/7510-menu-empty-state-overflow
Open

Keep menu empty-state text inside the card#7653
benwillems wants to merge 1 commit into
basecamp:quattrofrom
benwillems:fix/7510-menu-empty-state-overflow

Conversation

@benwillems

@benwillems benwillems commented Aug 21, 2026

Copy link
Copy Markdown

This is my first PR here so please let me know if there is anything I can improve upon or do better to make reviewing easier. I'm looking to learn and get better and will be happy to make changes moving forward.
Thanks!

Ben

Summary

  • Constrain the menu empty-state column to the card width so a long unmatched query cannot paint past the rounded edges.
  • Elide the "No matches for …" line, matching the search field.
  • Apply the same empty-state constraint and elide to the clipboard and emoji pickers so they cannot overflow the same way.

The empty-state Column only had anchors.centerIn. Its message Text used a fixed width: Style.space(320) with no elide, so once the query was wider than 320px the line was centered on that box and painted outside the card. Giving the column width: parent.width (the already-sized list item) and eliding the message keeps it inside.

Fixes #7510

Testing

  • Super+Space, typed a long unmatched query (holas como esatas cjaksjdwdawdjwdj): the message stays inside the card, elided, still centered.
  • Short unmatched query still reads in full inside the card.
  • ./test/all — CLI suite passed. Shell suite had three environment-only failures (omarchy-pkgs checkout not present); nothing related to this QML change.

Before / after

Before — empty-state text paints past the rounded card:
7510-before

After — same query stays inside, with an ellipsis:
7510-after

Clipboard picker/Emoji
7510-emojis-after
7510-clipboard-after

Copilot AI balanced review requested due to automatic review settings August 21, 2026 02:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Constrains empty-state messages to picker card widths and elides long unmatched queries.

Changes:

  • Constrains menu, emoji, and clipboard empty-state columns.
  • Adds right elision to empty-state messages.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
shell/plugins/menu/Menu.qml Constrains and elides menu empty-state text.
shell/plugins/emojis/Emojis.qml Applies equivalent emoji-picker constraints.
shell/plugins/clipboard/Clipboard.qml Applies equivalent clipboard-picker constraints.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread shell/plugins/clipboard/Clipboard.qml
Comment thread shell/plugins/emojis/Emojis.qml
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed on a disposable VM running this branch's shell, with an A/B against quattro. Nothing found — clean, nothing pushed.

It fixes #7510 as described. On quattro a long unmatched query paints the "No matches for …" line straight out past both rounded edges of the card and over whatever is behind it; on this branch the same query elides inside the card. Checked by screenshot on both trees in a real compositor rather than by reading the diff.

It also fixes a bug nobody reported. On quattro the clipboard and emoji empty states render nothing at all — no icon, no message, just a blank card. Their Column had no explicit width while both children bound width: parent.width, which is circular; Qt's positioner skips zero-width children when computing implicit size, so it settles at zero with no binding-loop warning to give it away. Adding width: parent.width to the Column is what makes those two empty states appear for the first time, so it is load-bearing rather than the cosmetic tidy-up the PR body implies. Worth saying in the description — it is a bigger win than the title suggests.

No wrap-to-elide regression. The risk with adding elide is turning previously wrapped multi-line text into one truncated line. None of the five touched Text elements has a wrapMode, so every one was already Text.NoWrap on a single line; the elide replaces overflow with an ellipsis and no message loses lines it used to show.

anchors.centerIn: parent alongside width: parent.width is fine. centerIn sets only the two centre anchors and does not size the item, so there is no conflict and no binding loop, and both parent references resolve to the same item in all three files. The running shell logged no QML warnings on load or on opening any of the three panels.

parent.width is bounded in all three, despite the three cards having different widths: each empty-state Column's parent is the result viewport, which anchors inside a card of fixed cardWidth capped at the panel width.

Tests: menu, menu-plugin, menu-guards, menu-images, clipboard and emojis suites — 235 assertions, all passing — run through the shell-test wrapper on a worker whose compositor was confirmed serving this branch (quickshell -n -p …/omarchy-pr-7653/shell), so the QML was actually loaded and not merely grepped.

Second opinion: codex at xhigh reviewed independently and agreed on all of the above; its independence is not currently guaranteed, since it can read this session's own transcript. It did contribute the mechanism for the zero-width Column — that Qt's positioner excludes zero-width children and so stabilises at zero instead of emitting the usual binding-loop warning — which I had observed as a symptom without explaining. It also flagged one defect I could not reproduce: that the clipboard empty state now sits centred over the pane divider at Clipboard.qml:540. That Rectangle does lack a visibility guard, so the reading is reasonable, but no divider is painted there in the running shell — I measured a 21×515 pixel strip across that position with the clipboard both populated and filtered to no matches, and all 10,815 pixels are the card background. No artifact, so nothing to fix.

One thing out of scope, aimed at the maintainer rather than at you: shell/Ui/MultiSelect.qml:488 and shell/Ui/SearchableDropdown.qml:248 centre an empty-state Text with no width, elide or wrap, so a long caller-supplied emptyText or error string could overflow the same way. Neither echoes a typed query and MultiSelect has no current instantiation, so it is latent rather than live — and hardening it here would widen this PR past what it closes.

No collision with #7657, which also edits Menu.qml: its hunks are at lines 726 and 747, well clear of the empty-state block at 1438-1467, so the two merge in either order.

Next: waiting on the maintainer to merge. Nothing needed from you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu empty-state text overflows the container on long queries

3 participants