[Bug Fix] DropdownMenu: fix z-index stacking and default placement (#439)#440
Conversation
) - Default placement changed from `top` to `bottom` so menus open below their trigger (matches dropdown convention). - Removed the static `z-50` from the DropdownMenu container; the controller now sets z-index only while open, so closed sibling dropdowns no longer cover an open menu (they share the same z-50 and DOM order decided paint order). - Dropped `w-full` from the Usage demo trigger so the floating reference matches the visible button instead of a full-width wrapper.
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="gem/lib/ruby_ui/dropdown_menu/dropdown_menu_controller.js">
<violation number="1" location="gem/lib/ruby_ui/dropdown_menu/dropdown_menu_controller.js:85">
P2: `close()` does not sync `data-state="closed"` — close animations never trigger</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| close() { | ||
| this.openValue = false; | ||
| this.#removeEventListeners(); | ||
| this.element.style.zIndex = ""; |
There was a problem hiding this comment.
P2: close() does not sync data-state="closed" — close animations never trigger
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At gem/lib/ruby_ui/dropdown_menu/dropdown_menu_controller.js, line 85:
<comment>`close()` does not sync `data-state="closed"` — close animations never trigger</comment>
<file context>
@@ -73,12 +73,16 @@ export default class extends Controller {
close() {
this.openValue = false;
this.#removeEventListeners();
+ this.element.style.zIndex = "";
this.contentTarget.classList.add("hidden");
}
</file context>
| this.element.style.zIndex = ""; | |
| this.element.style.zIndex = ""; | |
| this.element.setAttribute("data-state", "closed"); |
There was a problem hiding this comment.
Thanks — flagging this as out of scope for this PR (which only covers the z-index stacking and default placement). A few points on the finding:
- Pre-existing, not introduced here. The controller has never synced
data-state; my diff only touchedclose()to reset the inlinezIndex. - Suggested target is wrong. The
data-[state=open|closed]:*animation utilities live on the inner content<div>inDropdownMenuContent(theattrsdiv), not onthis.element(theDropdownMenucontainer). Settingdata-stateon the container won't trigger any animation. hiddenblocks the close animation regardless.close()addshidden(display:none) to the content wrapper, so the exit animation can't play unless the hide is deferred untilanimationend.
Proper close-animation handling is being tracked separately (see the data-state/hidden sync work on the Floating UI port branch). Leaving it out to keep this PR focused.
There was a problem hiding this comment.
You're right: this PR only changes z-index/default placement, so the data-state/close-animation issue is out of scope here. Also, the animation utilities apply to the inner content wrapper, and hidden prevents the exit animation unless hide is deferred.
This comment was influenced by this learning. Open the link to edit it, or reply here to edit or delete it.
There was a problem hiding this comment.
Open a new issue with the of of scope for me.
Fixes #439
Problems
After the Floating UI port (#158), the DropdownMenu had two issues (reproducible on https://www.rubyui.com/docs/dropdown_menu):
z-50, and so did every sibling dropdown — equal z-index, so paint order fell back to DOM order and later siblings covered an open menu.top(unusual for a dropdown — convention isbottom), and the demo wrapped the trigger inclass: 'w-full', so Floating UI used the full-width wrapper as the reference and centred the menu over it, far from the small button.Changes
dropdown_menu_controller.js(gem + docs copy):top→bottom.element.style.zIndexonly while the menu is open ("50"on open, cleared on close).dropdown_menu.rb: removed the staticz-50from the container. Closed dropdowns now stack in normal flow (no stacking context), so the one open menu — elevated by the controller — always paints above its neighbours.dropdown_menu_docs.rb+docs/app/views/docs/dropdown_menu.rb: droppedw-fullfrom the Usage trigger so the floating reference matches the visible button.Testing
cd gem && bundle exec rake→ 237 runs, 0 failures, StandardRB clean.node --checkon both controllers.Screenshots
Before/after to be added.
Summary by cubic
Fixes DropdownMenu stacking so open menus render above siblings, and sets the default placement to open below the trigger. Updates docs and MCP registry so the menu positions relative to the button, not a full-width wrapper.
z-50; controller now setszIndex = "50"only while open and clears on close.toptobottomin both controllers.w-fullfromDropdownMenuTrigger; rebuilt MCPregistry.jsonto sync examples.Written for commit 9c4be6c. Summary will update on new commits.