Skip to content

[Bug Fix] DropdownMenu: fix z-index stacking and default placement (#439) #440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cirdes merged 2 commits into from
Jun 24, 2026
Merged

[Bug Fix] DropdownMenu: fix z-index stacking and default placement (#439) #440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6b6b5b
[Bug Fix] DropdownMenu: fix z-index stacking and default placement (#…
djalmaaraujo Jun 23, 2026
9c4be6c
[Bug Fix] Rebuild MCP registry for DropdownMenu changes (#439)
djalmaaraujo Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/app/javascript/controllers/ruby_ui/dropdown_menu_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default class extends Controller {

#computeTooltip() {
computePosition(this.triggerTarget, this.contentTarget, {
placement: this.optionsValue.placement || "top",
placement: this.optionsValue.placement || "bottom",
middleware: [flip(), shift(), offset(8)],
strategy: this.optionsValue.strategy || "absolute",
}).then(({ x, y }) => {
Expand Down Expand Up @@ -73,12 +73,16 @@ export default class extends Controller {
this.#deselectAll();
this.#addEventListeners();
this.#computeTooltip();
// Lift the open menu above sibling dropdowns/elements. The container has no
// static z-index, so closed siblings stack in normal flow and never cover it.
this.element.style.zIndex = "50";
this.contentTarget.classList.remove("hidden");
}

close() {
this.openValue = false;
this.#removeEventListeners();
this.element.style.zIndex = "";
this.contentTarget.classList.add("hidden");
}

Expand Down
4 changes: 2 additions & 2 deletions docs/app/views/docs/dropdown_menu.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def view_template
render Docs::VisualCodeExample.new(title: "Example", context: self) do
<<~RUBY
DropdownMenu do
DropdownMenuTrigger(class: 'w-full') do
DropdownMenuTrigger do
Button(variant: :outline) { "Open" }
end
DropdownMenuContent do
Expand All @@ -29,7 +29,7 @@ def view_template
render Docs::VisualCodeExample.new(title: "Non-navigational item", description: "Use as: :div when the item hosts its own interactive element (e.g. a dialog or form trigger). This avoids nesting a <button>/<form> inside the item's <a> while keeping the menu-item styling, role and keyboard behavior.", context: self) do
<<~RUBY
DropdownMenu do
DropdownMenuTrigger(class: 'w-full') do
DropdownMenuTrigger do
Button(variant: :outline) { "Open" }
end
DropdownMenuContent do
Expand Down
1 change: 0 additions & 1 deletion gem/lib/ruby_ui/dropdown_menu/dropdown_menu.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ def view_template(&)
def default_attrs
{
class: [
"z-50",
"group/dropdown-menu",
(strategy == "absolute") ? "is-absolute" : "is-fixed"
],
Expand Down
6 changes: 5 additions & 1 deletion gem/lib/ruby_ui/dropdown_menu/dropdown_menu_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default class extends Controller {

#computeTooltip() {
computePosition(this.triggerTarget, this.contentTarget, {
placement: this.optionsValue.placement || "top",
placement: this.optionsValue.placement || "bottom",
middleware: [flip(), shift(), offset(8)],
strategy: this.optionsValue.strategy || "absolute",
}).then(({ x, y }) => {
Expand Down Expand Up @@ -73,12 +73,16 @@ export default class extends Controller {
this.#deselectAll();
this.#addEventListeners();
this.#computeTooltip();
// Lift the open menu above sibling dropdowns/elements. The container has no
// static z-index, so closed siblings stack in normal flow and never cover it.
this.element.style.zIndex = "50";
this.contentTarget.classList.remove("hidden");
}

close() {
this.openValue = false;
this.#removeEventListeners();
this.element.style.zIndex = "";

@cubic-dev-ai cubic-dev-ai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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>
Suggested change
this.element.style.zIndex = "";
this.element.style.zIndex = "";
this.element.setAttribute("data-state", "closed");

@djalmaaraujo djalmaaraujo Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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:

  1. Pre-existing, not introduced here. The controller has never synced data-state; my diff only touched close() to reset the inline zIndex.
  2. Suggested target is wrong. The data-[state=open|closed]:* animation utilities live on the inner content <div> in DropdownMenuContent (the attrs div), not on this.element (the DropdownMenu container). Setting data-state on the container won't trigger any animation.
  3. hidden blocks the close animation regardless. close() adds hidden (display:none) to the content wrapper, so the exit animation can't play unless the hide is deferred until animationend.

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.

@cubic-dev-ai cubic-dev-ai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@djalmaaraujo djalmaaraujo Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Open a new issue with the of of scope for me.

this.contentTarget.classList.add("hidden");
}

Expand Down
2 changes: 1 addition & 1 deletion gem/lib/ruby_ui/dropdown_menu/dropdown_menu_docs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def view_template
render Docs::VisualCodeExample.new(title: "Example", context: self) do
<<~RUBY
DropdownMenu do
DropdownMenuTrigger(class: 'w-full') do
DropdownMenuTrigger do
Button(variant: :outline) { "Open" }
end
DropdownMenuContent do
Expand Down
Loading