Skip to content

[Bug Fix] Overlays: release Command scroll lock on teardown, settle Tooltip exit on cancel #535

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

Open
djalmaaraujo wants to merge 2 commits into
base: main
Choose a base branch
from
+14 −5
Open

[Bug Fix] Overlays: release Command scroll lock on teardown, settle Tooltip exit on cancel #535

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
4af5eb6
fix(overlay): release Command scroll lock on teardown and settle Tool…
djalmaaraujo Sep 15, 2026
93f5af5
fix(command): release scroll lock only when torn down mid-open, not o…
djalmaaraujo Sep 15, 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: 6 additions & 0 deletions gem/lib/ruby_ui/command/command_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ export default class extends Controller {
}

disconnect() {
// Torn down mid-open (Turbo nav/morph) never runs afterExit, the only place the
// <body> scroll lock is released. Release it here for that path only; on a normal
// close afterExit already handled it.
if (this.hasPanelTarget && this.panelTarget.dataset.state !== "closed") {
document.body.classList.remove("overflow-hidden");
}
// Nothing is left to wait for the exit animation, so apply the pending removal now.
if (this.hasPanelTarget) this.settleExit(this.panelTarget);
}
Expand Down
9 changes: 6 additions & 3 deletions gem/lib/ruby_ui/tooltip/tooltip_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ export default class extends Controller {
document.body.appendChild(element);

this.triggerTarget.setAttribute("aria-describedby", element.id);
element.addEventListener("animationend", (event) => this.animationEnd(event));
// animationcancel covers an exit run cut short (backgrounded tab, interrupting
// style change) — without it the cloned node is never removed from <body>.
element.addEventListener("animationend", this.handleExitAnimationEnd);
element.addEventListener("animationcancel", this.handleExitAnimationEnd);

const onBeforeCache = () => this.unmount();
document.addEventListener("turbo:before-cache", onBeforeCache);
Expand Down Expand Up @@ -50,12 +53,12 @@ export default class extends Controller {
this.mounted?.element.setAttribute("data-state", "closed");
}

animationEnd(event) {
handleExitAnimationEnd = (event) => {
if (event.animationName !== "exit") return;
if (this.mounted?.element.getAttribute("data-state") !== "closed") return;

this.unmount();
}
};

cloneTemplate() {
return this.contentTarget.content.firstElementChild.cloneNode(true);
Expand Down
4 changes: 2 additions & 2 deletions mcp/data/registry.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@
},
{
"path": "command_controller.js",
"content": "import { Controller } from \"@hotwired/stimulus\";\nimport Fuse from \"fuse.js\";\n\n// Connects to data-controller=\"ruby-ui--command\"\nexport default class extends Controller {\n static targets = [\"input\", \"group\", \"item\", \"empty\", \"backdrop\", \"panel\"];\n\n connect() {\n this.selectedIndex = -1;\n\n if (!this.hasInputTarget) {\n return;\n }\n\n this.inputTarget.focus();\n this.searchIndex = this.buildSearchIndex();\n this.toggleVisibility(this.emptyTargets, false);\n }\n\n disconnect() {\n // Nothing is left to wait for the exit animation, so apply the pending removal now.\n if (this.hasPanelTarget) this.settleExit(this.panelTarget);\n }\n\n dismiss() {\n this.backdropTarget.dataset.state = \"closed\";\n this.panelTarget.dataset.state = \"closed\";\n this.hideAfterExitAnimation(this.panelTarget);\n }\n\n // Opened again while dismissing: bring this instance back instead of stacking a new one.\n show() {\n this.backdropTarget.dataset.state = \"open\";\n this.panelTarget.dataset.state = \"open\";\n document.body.classList.add(\"overflow-hidden\");\n this.focusInput();\n }\n\n afterExit() {\n document.body.classList.remove(\"overflow-hidden\");\n this.element.remove();\n }\n\n // Overlay exit — the same block in every overlay controller, so keep them in sync.\n exitAnimationNames = new WeakMap();\n\n hideAfterExitAnimation(animated) {\n const exitAnimations = animated\n .getAnimations()\n .filter((animation) => animation instanceof CSSAnimation);\n\n // No exit animation, or no box to run it in: animationend would never fire.\n if (exitAnimations.length === 0) {\n this.settleExit(animated);\n return;\n }\n\n this.exitAnimationNames.set(animated, exitAnimations.map((animation) => animation.animationName));\n animated.addEventListener(\"animationend\", this.handleExitAnimationEnd);\n animated.addEventListener(\"animationcancel\", this.handleExitAnimationEnd);\n }\n\n handleExitAnimationEnd = (event) => {\n // animationend bubbles — an animated child must not hide its container.\n if (event.target !== event.currentTarget) return;\n // Closing mid-open cancels the enter animation; only the exit run settles this.\n if (!this.exitAnimationNames.get(event.currentTarget)?.includes(event.animationName)) return;\n\n this.settleExit(event.currentTarget);\n };\n\n settleExit(animated) {\n animated.removeEventListener(\"animationend\", this.handleExitAnimationEnd);\n animated.removeEventListener(\"animationcancel\", this.handleExitAnimationEnd);\n // Reopened mid-exit: it is on its way back in, leave it visible.\n if (animated.dataset.state !== \"closed\") return;\n\n this.afterExit(animated);\n }\n\n focusInput() {\n this.inputTarget?.focus();\n }\n\n filter(e) {\n // Deselect any previously selected item\n this.deselectAll();\n\n const query = e.target.value.toLowerCase();\n if (query.length === 0) {\n this.resetVisibility();\n return;\n }\n\n this.toggleVisibility(this.itemTargets, false);\n\n const results = this.searchIndex.search(query);\n results.forEach((result) =>\n this.toggleVisibility([result.item.element], true),\n );\n\n this.toggleVisibility(this.emptyTargets, results.length === 0);\n this.updateGroupVisibility();\n }\n\n toggleVisibility(elements, isVisible) {\n elements.forEach((el) => el.classList.toggle(\"hidden\", !isVisible));\n }\n\n updateGroupVisibility() {\n this.groupTargets.forEach((group) => {\n const hasVisibleItems =\n group.querySelectorAll(\n \"[data-ruby-ui--command-target='item']:not(.hidden)\",\n ).length > 0;\n this.toggleVisibility([group], hasVisibleItems);\n });\n }\n\n resetVisibility() {\n this.toggleVisibility(this.itemTargets, true);\n this.toggleVisibility(this.groupTargets, true);\n this.toggleVisibility(this.emptyTargets, false);\n }\n\n buildSearchIndex() {\n const options = {\n keys: [\"value\"],\n threshold: 0.2,\n includeMatches: true,\n };\n const items = this.itemTargets.map((el) => ({\n value: el.dataset.value,\n element: el,\n }));\n return new Fuse(items, options);\n }\n\n handleKeydown(e) {\n const visibleItems = this.itemTargets.filter(\n (item) => !item.classList.contains(\"hidden\"),\n );\n if (e.key === \"ArrowDown\") {\n e.preventDefault();\n this.updateSelectedItem(visibleItems, 1);\n } else if (e.key === \"ArrowUp\") {\n e.preventDefault();\n this.updateSelectedItem(visibleItems, -1);\n } else if (e.key === \"Enter\" && this.selectedIndex !== -1) {\n e.preventDefault();\n visibleItems[this.selectedIndex].click();\n }\n }\n\n updateSelectedItem(visibleItems, direction) {\n if (this.selectedIndex >= 0) {\n this.toggleAriaSelected(visibleItems[this.selectedIndex], false);\n }\n\n this.selectedIndex += direction;\n\n // Ensure the selected index is within the bounds of the visible items\n if (this.selectedIndex < 0) {\n this.selectedIndex = visibleItems.length - 1;\n } else if (this.selectedIndex >= visibleItems.length) {\n this.selectedIndex = 0;\n }\n\n this.toggleAriaSelected(visibleItems[this.selectedIndex], true);\n }\n\n toggleAriaSelected(element, isSelected) {\n element.setAttribute(\"aria-selected\", isSelected.toString());\n }\n\n deselectAll() {\n this.itemTargets.forEach((item) => this.toggleAriaSelected(item, false));\n this.selectedIndex = -1;\n }\n}\n"
"content": "import { Controller } from \"@hotwired/stimulus\";\nimport Fuse from \"fuse.js\";\n\n// Connects to data-controller=\"ruby-ui--command\"\nexport default class extends Controller {\n static targets = [\"input\", \"group\", \"item\", \"empty\", \"backdrop\", \"panel\"];\n\n connect() {\n this.selectedIndex = -1;\n\n if (!this.hasInputTarget) {\n return;\n }\n\n this.inputTarget.focus();\n this.searchIndex = this.buildSearchIndex();\n this.toggleVisibility(this.emptyTargets, false);\n }\n\n disconnect() {\n // Torn down mid-open (Turbo nav/morph) never runs afterExit, the only place the\n // <body> scroll lock is released. Release it here for that path only; on a normal\n // close afterExit already handled it.\n if (this.hasPanelTarget && this.panelTarget.dataset.state !== \"closed\") {\n document.body.classList.remove(\"overflow-hidden\");\n }\n // Nothing is left to wait for the exit animation, so apply the pending removal now.\n if (this.hasPanelTarget) this.settleExit(this.panelTarget);\n }\n\n dismiss() {\n this.backdropTarget.dataset.state = \"closed\";\n this.panelTarget.dataset.state = \"closed\";\n this.hideAfterExitAnimation(this.panelTarget);\n }\n\n // Opened again while dismissing: bring this instance back instead of stacking a new one.\n show() {\n this.backdropTarget.dataset.state = \"open\";\n this.panelTarget.dataset.state = \"open\";\n document.body.classList.add(\"overflow-hidden\");\n this.focusInput();\n }\n\n afterExit() {\n document.body.classList.remove(\"overflow-hidden\");\n this.element.remove();\n }\n\n // Overlay exit — the same block in every overlay controller, so keep them in sync.\n exitAnimationNames = new WeakMap();\n\n hideAfterExitAnimation(animated) {\n const exitAnimations = animated\n .getAnimations()\n .filter((animation) => animation instanceof CSSAnimation);\n\n // No exit animation, or no box to run it in: animationend would never fire.\n if (exitAnimations.length === 0) {\n this.settleExit(animated);\n return;\n }\n\n this.exitAnimationNames.set(animated, exitAnimations.map((animation) => animation.animationName));\n animated.addEventListener(\"animationend\", this.handleExitAnimationEnd);\n animated.addEventListener(\"animationcancel\", this.handleExitAnimationEnd);\n }\n\n handleExitAnimationEnd = (event) => {\n // animationend bubbles — an animated child must not hide its container.\n if (event.target !== event.currentTarget) return;\n // Closing mid-open cancels the enter animation; only the exit run settles this.\n if (!this.exitAnimationNames.get(event.currentTarget)?.includes(event.animationName)) return;\n\n this.settleExit(event.currentTarget);\n };\n\n settleExit(animated) {\n animated.removeEventListener(\"animationend\", this.handleExitAnimationEnd);\n animated.removeEventListener(\"animationcancel\", this.handleExitAnimationEnd);\n // Reopened mid-exit: it is on its way back in, leave it visible.\n if (animated.dataset.state !== \"closed\") return;\n\n this.afterExit(animated);\n }\n\n focusInput() {\n this.inputTarget?.focus();\n }\n\n filter(e) {\n // Deselect any previously selected item\n this.deselectAll();\n\n const query = e.target.value.toLowerCase();\n if (query.length === 0) {\n this.resetVisibility();\n return;\n }\n\n this.toggleVisibility(this.itemTargets, false);\n\n const results = this.searchIndex.search(query);\n results.forEach((result) =>\n this.toggleVisibility([result.item.element], true),\n );\n\n this.toggleVisibility(this.emptyTargets, results.length === 0);\n this.updateGroupVisibility();\n }\n\n toggleVisibility(elements, isVisible) {\n elements.forEach((el) => el.classList.toggle(\"hidden\", !isVisible));\n }\n\n updateGroupVisibility() {\n this.groupTargets.forEach((group) => {\n const hasVisibleItems =\n group.querySelectorAll(\n \"[data-ruby-ui--command-target='item']:not(.hidden)\",\n ).length > 0;\n this.toggleVisibility([group], hasVisibleItems);\n });\n }\n\n resetVisibility() {\n this.toggleVisibility(this.itemTargets, true);\n this.toggleVisibility(this.groupTargets, true);\n this.toggleVisibility(this.emptyTargets, false);\n }\n\n buildSearchIndex() {\n const options = {\n keys: [\"value\"],\n threshold: 0.2,\n includeMatches: true,\n };\n const items = this.itemTargets.map((el) => ({\n value: el.dataset.value,\n element: el,\n }));\n return new Fuse(items, options);\n }\n\n handleKeydown(e) {\n const visibleItems = this.itemTargets.filter(\n (item) => !item.classList.contains(\"hidden\"),\n );\n if (e.key === \"ArrowDown\") {\n e.preventDefault();\n this.updateSelectedItem(visibleItems, 1);\n } else if (e.key === \"ArrowUp\") {\n e.preventDefault();\n this.updateSelectedItem(visibleItems, -1);\n } else if (e.key === \"Enter\" && this.selectedIndex !== -1) {\n e.preventDefault();\n visibleItems[this.selectedIndex].click();\n }\n }\n\n updateSelectedItem(visibleItems, direction) {\n if (this.selectedIndex >= 0) {\n this.toggleAriaSelected(visibleItems[this.selectedIndex], false);\n }\n\n this.selectedIndex += direction;\n\n // Ensure the selected index is within the bounds of the visible items\n if (this.selectedIndex < 0) {\n this.selectedIndex = visibleItems.length - 1;\n } else if (this.selectedIndex >= visibleItems.length) {\n this.selectedIndex = 0;\n }\n\n this.toggleAriaSelected(visibleItems[this.selectedIndex], true);\n }\n\n toggleAriaSelected(element, isSelected) {\n element.setAttribute(\"aria-selected\", isSelected.toString());\n }\n\n deselectAll() {\n this.itemTargets.forEach((item) => this.toggleAriaSelected(item, false));\n this.selectedIndex = -1;\n }\n}\n"
},
{
"path": "command_dialog.rb",
Expand Down Expand Up @@ -2863,7 +2863,7 @@
},
{
"path": "tooltip_controller.js",
"content": "import { Controller } from \"@hotwired/stimulus\";\nimport { computePosition, autoUpdate, offset, shift } from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n\n static values = { placement: \"top\" };\n\n mount() {\n if (this.mounted) return;\n\n const element = this.cloneTemplate();\n element.setAttribute(\"data-placement\", this.placementValue);\n document.body.appendChild(element);\n\n this.triggerTarget.setAttribute(\"aria-describedby\", element.id);\n element.addEventListener(\"animationend\", (event) => this.animationEnd(event));\n\n const onBeforeCache = () => this.unmount();\n document.addEventListener(\"turbo:before-cache\", onBeforeCache);\n\n this.mounted = { element, onBeforeCache };\n this.mounted.stopAutoUpdate = autoUpdate(this.triggerTarget, element, () => this.reposition());\n }\n\n unmount() {\n if (!this.mounted) return;\n\n document.removeEventListener(\"turbo:before-cache\", this.mounted.onBeforeCache);\n\n this.mounted.stopAutoUpdate?.();\n this.mounted.element.remove();\n this.triggerTarget.removeAttribute(\"aria-describedby\");\n\n this.mounted = null;\n }\n\n disconnect() {\n this.unmount();\n }\n\n show() {\n if (!this.hasContentTarget) return;\n\n this.mount();\n this.mounted.element.setAttribute(\"data-state\", \"open\");\n }\n\n hide() {\n this.mounted?.element.setAttribute(\"data-state\", \"closed\");\n }\n\n animationEnd(event) {\n if (event.animationName !== \"exit\") return;\n if (this.mounted?.element.getAttribute(\"data-state\") !== \"closed\") return;\n\n this.unmount();\n }\n\n cloneTemplate() {\n return this.contentTarget.content.firstElementChild.cloneNode(true);\n }\n\n reposition() {\n if (!this.mounted) return;\n\n const position = { placement: this.placementValue, middleware: [offset(4), shift()] };\n\n computePosition(this.triggerTarget, this.mounted.element, position).then(({ x, y }) => {\n this.mounted?.element.style.setProperty(\"left\", `${x}px`);\n this.mounted?.element.style.setProperty(\"top\", `${y}px`);\n });\n }\n}\n"
"content": "import { Controller } from \"@hotwired/stimulus\";\nimport { computePosition, autoUpdate, offset, shift } from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n\n static values = { placement: \"top\" };\n\n mount() {\n if (this.mounted) return;\n\n const element = this.cloneTemplate();\n element.setAttribute(\"data-placement\", this.placementValue);\n document.body.appendChild(element);\n\n this.triggerTarget.setAttribute(\"aria-describedby\", element.id);\n // animationcancel covers an exit run cut short (backgrounded tab, interrupting\n // style change) — without it the cloned node is never removed from <body>.\n element.addEventListener(\"animationend\", this.handleExitAnimationEnd);\n element.addEventListener(\"animationcancel\", this.handleExitAnimationEnd);\n\n const onBeforeCache = () => this.unmount();\n document.addEventListener(\"turbo:before-cache\", onBeforeCache);\n\n this.mounted = { element, onBeforeCache };\n this.mounted.stopAutoUpdate = autoUpdate(this.triggerTarget, element, () => this.reposition());\n }\n\n unmount() {\n if (!this.mounted) return;\n\n document.removeEventListener(\"turbo:before-cache\", this.mounted.onBeforeCache);\n\n this.mounted.stopAutoUpdate?.();\n this.mounted.element.remove();\n this.triggerTarget.removeAttribute(\"aria-describedby\");\n\n this.mounted = null;\n }\n\n disconnect() {\n this.unmount();\n }\n\n show() {\n if (!this.hasContentTarget) return;\n\n this.mount();\n this.mounted.element.setAttribute(\"data-state\", \"open\");\n }\n\n hide() {\n this.mounted?.element.setAttribute(\"data-state\", \"closed\");\n }\n\n handleExitAnimationEnd = (event) => {\n if (event.animationName !== \"exit\") return;\n if (this.mounted?.element.getAttribute(\"data-state\") !== \"closed\") return;\n\n this.unmount();\n };\n\n cloneTemplate() {\n return this.contentTarget.content.firstElementChild.cloneNode(true);\n }\n\n reposition() {\n if (!this.mounted) return;\n\n const position = { placement: this.placementValue, middleware: [offset(4), shift()] };\n\n computePosition(this.triggerTarget, this.mounted.element, position).then(({ x, y }) => {\n this.mounted?.element.style.setProperty(\"left\", `${x}px`);\n this.mounted?.element.style.setProperty(\"top\", `${y}px`);\n });\n }\n}\n"
},
{
"path": "tooltip_trigger.rb",
Expand Down