From 96047cdad7e4e222e78a8d8fb0a20ea1bc0a294b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Djalma=20Ara=C3=BAjo?= Date: Thu, 30 Jul 2026 12:30:57 -0300 Subject: [PATCH 1/5] [Bug Fix] Popover: set data-state/data-side, clear closeTimeout on disconnect, close on Escape (#494) --- .../controllers/ruby_ui/popover_controller.js | 22 ++++++++++++++++++- gem/lib/ruby_ui/popover/popover_content.rb | 3 ++- gem/lib/ruby_ui/popover/popover_controller.js | 22 ++++++++++++++++++- gem/test/ruby_ui/popover_test.rb | 13 +++++++++++ mcp/data/registry.json | 4 ++-- 5 files changed, 59 insertions(+), 5 deletions(-) diff --git a/docs/app/javascript/controllers/ruby_ui/popover_controller.js b/docs/app/javascript/controllers/ruby_ui/popover_controller.js index 9b847764d..c5b0d0a55 100644 --- a/docs/app/javascript/controllers/ruby_ui/popover_controller.js +++ b/docs/app/javascript/controllers/ruby_ui/popover_controller.js @@ -23,8 +23,11 @@ export default class extends Controller { disconnect() { this.removeEventListeners(); + clearTimeout(this.closeTimeout); + document.removeEventListener("keydown", this.handleKeydown); if (this.cleanup) { this.cleanup(); + this.cleanup = null; } } @@ -75,15 +78,29 @@ export default class extends Controller { } }; + handleKeydown = (event) => { + if (event.key !== "Escape") return; + if (!this.openValue) return; + + clearTimeout(this.closeTimeout); + this.openValue = false; + this.hidePopover(); + }; + showPopover() { this.contentTarget.classList.remove("hidden"); + this.contentTarget.dataset.state = "open"; + document.addEventListener("keydown", this.handleKeydown); this.updatePosition(); } hidePopover() { this.contentTarget.classList.add("hidden"); + this.contentTarget.dataset.state = "closed"; + document.removeEventListener("keydown", this.handleKeydown); if (this.cleanup) { this.cleanup(); + this.cleanup = null; } } @@ -96,11 +113,14 @@ export default class extends Controller { computePosition(this.triggerTarget, this.contentTarget, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], - }).then(({ x, y }) => { + }).then(({ x, y, placement }) => { Object.assign(this.contentTarget.style, { left: `${x}px`, top: `${y}px`, }); + // flip() can resolve to the opposite side of the requested placement, + // so the directional slide-in classes must follow the resolved value. + this.contentTarget.dataset.side = placement.split("-")[0]; }); }); } diff --git a/gem/lib/ruby_ui/popover/popover_content.rb b/gem/lib/ruby_ui/popover/popover_content.rb index 8ba5aedac..1730dd9bb 100644 --- a/gem/lib/ruby_ui/popover/popover_content.rb +++ b/gem/lib/ruby_ui/popover/popover_content.rb @@ -11,7 +11,8 @@ def view_template(&) def default_attrs { data: { - ruby_ui__popover_target: "content" + ruby_ui__popover_target: "content", + state: :closed }, class: [ "hidden z-50 rounded-md border bg-background p-1 text-foreground shadow-md outline-none", diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index 9b847764d..c5b0d0a55 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -23,8 +23,11 @@ export default class extends Controller { disconnect() { this.removeEventListeners(); + clearTimeout(this.closeTimeout); + document.removeEventListener("keydown", this.handleKeydown); if (this.cleanup) { this.cleanup(); + this.cleanup = null; } } @@ -75,15 +78,29 @@ export default class extends Controller { } }; + handleKeydown = (event) => { + if (event.key !== "Escape") return; + if (!this.openValue) return; + + clearTimeout(this.closeTimeout); + this.openValue = false; + this.hidePopover(); + }; + showPopover() { this.contentTarget.classList.remove("hidden"); + this.contentTarget.dataset.state = "open"; + document.addEventListener("keydown", this.handleKeydown); this.updatePosition(); } hidePopover() { this.contentTarget.classList.add("hidden"); + this.contentTarget.dataset.state = "closed"; + document.removeEventListener("keydown", this.handleKeydown); if (this.cleanup) { this.cleanup(); + this.cleanup = null; } } @@ -96,11 +113,14 @@ export default class extends Controller { computePosition(this.triggerTarget, this.contentTarget, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], - }).then(({ x, y }) => { + }).then(({ x, y, placement }) => { Object.assign(this.contentTarget.style, { left: `${x}px`, top: `${y}px`, }); + // flip() can resolve to the opposite side of the requested placement, + // so the directional slide-in classes must follow the resolved value. + this.contentTarget.dataset.side = placement.split("-")[0]; }); }); } diff --git a/gem/test/ruby_ui/popover_test.rb b/gem/test/ruby_ui/popover_test.rb index cc5b86561..a4039c96c 100644 --- a/gem/test/ruby_ui/popover_test.rb +++ b/gem/test/ruby_ui/popover_test.rb @@ -25,4 +25,17 @@ def test_render_with_all_items assert_match(/Profile/, output) end + + # The animation classes on the content are keyed on data-state, so the + # attribute has to be present before the Stimulus controller ever runs. + def test_content_renders_closed_state_by_default + output = phlex do + RubyUI.PopoverContent { "popover body" } + end + + assert_match(/data-state="closed"/, output) + assert_match(/hidden/, output) + assert_match(/absolute/, output) + assert_match(/popover body/, output) + end end diff --git a/mcp/data/registry.json b/mcp/data/registry.json index 14ae2d2ec..9b0eecbd6 100644 --- a/mcp/data/registry.json +++ b/mcp/data/registry.json @@ -2053,11 +2053,11 @@ }, { "path": "popover_content.rb", - "content": "# frozen_string_literal: true\n\nmodule RubyUI\n class PopoverContent < Base\n def view_template(&)\n div(**attrs, &)\n end\n\n private\n\n def default_attrs\n {\n data: {\n ruby_ui__popover_target: \"content\"\n },\n class: [\n \"hidden z-50 rounded-md border bg-background p-1 text-foreground shadow-md outline-none\",\n \"data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0\",\n \"data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95\",\n \"data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2\",\n \"data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2\",\n \"absolute\"\n ]\n }\n end\n end\nend\n" + "content": "# frozen_string_literal: true\n\nmodule RubyUI\n class PopoverContent < Base\n def view_template(&)\n div(**attrs, &)\n end\n\n private\n\n def default_attrs\n {\n data: {\n ruby_ui__popover_target: \"content\",\n state: :closed\n },\n class: [\n \"hidden z-50 rounded-md border bg-background p-1 text-foreground shadow-md outline-none\",\n \"data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0\",\n \"data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95\",\n \"data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2\",\n \"data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2\",\n \"absolute\"\n ]\n }\n end\n end\nend\n" }, { "path": "popover_controller.js", - "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n }\n\n disconnect() {\n this.removeEventListeners();\n if (this.cleanup) {\n this.cleanup();\n }\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n removeEventListeners() {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n showPopover() {\n this.contentTarget.classList.remove(\"hidden\");\n this.updatePosition();\n }\n\n hidePopover() {\n this.contentTarget.classList.add(\"hidden\");\n if (this.cleanup) {\n this.cleanup();\n }\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n });\n });\n }\n}\n" + "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n }\n\n disconnect() {\n this.removeEventListeners();\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n removeEventListeners() {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.openValue = false;\n this.hidePopover();\n };\n\n showPopover() {\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n hidePopover() {\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n this.contentTarget.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n}\n" }, { "path": "popover_trigger.rb", From f73269c85f5c474300fb95e1b3ec6e77366c7b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Djalma=20Ara=C3=BAjo?= Date: Thu, 30 Jul 2026 12:39:38 -0300 Subject: [PATCH 2/5] [Bug Fix] Popover: order disconnect so a missing target cannot skip the teardown --- .../javascript/controllers/ruby_ui/popover_controller.js | 8 +++++--- gem/lib/ruby_ui/popover/popover_controller.js | 8 +++++--- mcp/data/registry.json | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/app/javascript/controllers/ruby_ui/popover_controller.js b/docs/app/javascript/controllers/ruby_ui/popover_controller.js index c5b0d0a55..3798a4672 100644 --- a/docs/app/javascript/controllers/ruby_ui/popover_controller.js +++ b/docs/app/javascript/controllers/ruby_ui/popover_controller.js @@ -21,14 +21,17 @@ export default class extends Controller { this.addEventListeners(); } + // Teardown that cannot fail comes first: resolving a target throws once the + // element is gone, and Stimulus swallows that, skipping the rest of disconnect. disconnect() { - this.removeEventListeners(); clearTimeout(this.closeTimeout); document.removeEventListener("keydown", this.handleKeydown); + document.removeEventListener("click", this.handleOutsideClick); if (this.cleanup) { this.cleanup(); this.cleanup = null; } + this.removeElementEventListeners(); } addEventListeners() { @@ -43,13 +46,12 @@ export default class extends Controller { } } - removeEventListeners() { + removeElementEventListeners() { this.triggerTarget.removeEventListener("mouseenter", this.handleMouseEnter); this.triggerTarget.removeEventListener("mouseleave", this.handleMouseLeave); this.contentTarget.removeEventListener("mouseenter", this.handleMouseEnter); this.contentTarget.removeEventListener("mouseleave", this.handleMouseLeave); this.triggerTarget.removeEventListener("click", this.handleClick); - document.removeEventListener("click", this.handleOutsideClick); } handleMouseEnter = () => { diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index c5b0d0a55..3798a4672 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -21,14 +21,17 @@ export default class extends Controller { this.addEventListeners(); } + // Teardown that cannot fail comes first: resolving a target throws once the + // element is gone, and Stimulus swallows that, skipping the rest of disconnect. disconnect() { - this.removeEventListeners(); clearTimeout(this.closeTimeout); document.removeEventListener("keydown", this.handleKeydown); + document.removeEventListener("click", this.handleOutsideClick); if (this.cleanup) { this.cleanup(); this.cleanup = null; } + this.removeElementEventListeners(); } addEventListeners() { @@ -43,13 +46,12 @@ export default class extends Controller { } } - removeEventListeners() { + removeElementEventListeners() { this.triggerTarget.removeEventListener("mouseenter", this.handleMouseEnter); this.triggerTarget.removeEventListener("mouseleave", this.handleMouseLeave); this.contentTarget.removeEventListener("mouseenter", this.handleMouseEnter); this.contentTarget.removeEventListener("mouseleave", this.handleMouseLeave); this.triggerTarget.removeEventListener("click", this.handleClick); - document.removeEventListener("click", this.handleOutsideClick); } handleMouseEnter = () => { diff --git a/mcp/data/registry.json b/mcp/data/registry.json index 9b0eecbd6..cb19f4534 100644 --- a/mcp/data/registry.json +++ b/mcp/data/registry.json @@ -2057,7 +2057,7 @@ }, { "path": "popover_controller.js", - "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n }\n\n disconnect() {\n this.removeEventListeners();\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n removeEventListeners() {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.openValue = false;\n this.hidePopover();\n };\n\n showPopover() {\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n hidePopover() {\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n this.contentTarget.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n}\n" + "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n removeElementEventListeners() {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.openValue = false;\n this.hidePopover();\n };\n\n showPopover() {\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n hidePopover() {\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n this.contentTarget.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n}\n" }, { "path": "popover_trigger.rb", From f91e8c93e44b2d9a197ec694837d91f524798652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Djalma=20Ara=C3=BAjo?= Date: Thu, 30 Jul 2026 14:17:08 -0300 Subject: [PATCH 3/5] [Bug Fix] Popover: guard targets in teardown and re-arm the open state on reconnect --- .../controllers/ruby_ui/popover_controller.js | 36 +++++++++++++++---- gem/lib/ruby_ui/popover/popover_controller.js | 36 +++++++++++++++---- mcp/data/registry.json | 2 +- 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/docs/app/javascript/controllers/ruby_ui/popover_controller.js b/docs/app/javascript/controllers/ruby_ui/popover_controller.js index 3798a4672..80b8b1a99 100644 --- a/docs/app/javascript/controllers/ruby_ui/popover_controller.js +++ b/docs/app/javascript/controllers/ruby_ui/popover_controller.js @@ -19,6 +19,10 @@ export default class extends Controller { this.closeTimeout = null; this.cleanup = null; this.addEventListeners(); + // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element) + // arrives already open. Re-arm the parts that live on the controller instead of + // the markup — the keydown listener and the autoUpdate positioning. + if (this.openValue) this.showPopover(); } // Teardown that cannot fail comes first: resolving a target throws once the @@ -46,12 +50,19 @@ export default class extends Controller { } } + // Each target is guarded on its own: losing one of them must not strand the + // listeners attached to the other. removeElementEventListeners() { - this.triggerTarget.removeEventListener("mouseenter", this.handleMouseEnter); - this.triggerTarget.removeEventListener("mouseleave", this.handleMouseLeave); - this.contentTarget.removeEventListener("mouseenter", this.handleMouseEnter); - this.contentTarget.removeEventListener("mouseleave", this.handleMouseLeave); - this.triggerTarget.removeEventListener("click", this.handleClick); + if (this.hasTriggerTarget) { + this.triggerTarget.removeEventListener("mouseenter", this.handleMouseEnter); + this.triggerTarget.removeEventListener("mouseleave", this.handleMouseLeave); + this.triggerTarget.removeEventListener("click", this.handleClick); + } + + if (this.hasContentTarget) { + this.contentTarget.removeEventListener("mouseenter", this.handleMouseEnter); + this.contentTarget.removeEventListener("mouseleave", this.handleMouseLeave); + } } handleMouseEnter = () => { @@ -90,20 +101,27 @@ export default class extends Controller { }; showPopover() { + if (!this.hasTriggerTarget || !this.hasContentTarget) return; + this.contentTarget.classList.remove("hidden"); this.contentTarget.dataset.state = "open"; document.addEventListener("keydown", this.handleKeydown); this.updatePosition(); } + // Same rule as disconnect(): release what is held outside the element first, so a + // missing content target cannot leave the keydown listener or autoUpdate running. hidePopover() { - this.contentTarget.classList.add("hidden"); - this.contentTarget.dataset.state = "closed"; document.removeEventListener("keydown", this.handleKeydown); if (this.cleanup) { this.cleanup(); this.cleanup = null; } + + if (!this.hasContentTarget) return; + + this.contentTarget.classList.add("hidden"); + this.contentTarget.dataset.state = "closed"; } updatePosition() { @@ -112,6 +130,10 @@ export default class extends Controller { } this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => { + // autoUpdate keeps firing on scroll/resize; bail out rather than throw once + // a target is gone and nothing has torn the popover down yet. + if (!this.hasTriggerTarget || !this.hasContentTarget) return; + computePosition(this.triggerTarget, this.contentTarget, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index 3798a4672..80b8b1a99 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -19,6 +19,10 @@ export default class extends Controller { this.closeTimeout = null; this.cleanup = null; this.addEventListeners(); + // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element) + // arrives already open. Re-arm the parts that live on the controller instead of + // the markup — the keydown listener and the autoUpdate positioning. + if (this.openValue) this.showPopover(); } // Teardown that cannot fail comes first: resolving a target throws once the @@ -46,12 +50,19 @@ export default class extends Controller { } } + // Each target is guarded on its own: losing one of them must not strand the + // listeners attached to the other. removeElementEventListeners() { - this.triggerTarget.removeEventListener("mouseenter", this.handleMouseEnter); - this.triggerTarget.removeEventListener("mouseleave", this.handleMouseLeave); - this.contentTarget.removeEventListener("mouseenter", this.handleMouseEnter); - this.contentTarget.removeEventListener("mouseleave", this.handleMouseLeave); - this.triggerTarget.removeEventListener("click", this.handleClick); + if (this.hasTriggerTarget) { + this.triggerTarget.removeEventListener("mouseenter", this.handleMouseEnter); + this.triggerTarget.removeEventListener("mouseleave", this.handleMouseLeave); + this.triggerTarget.removeEventListener("click", this.handleClick); + } + + if (this.hasContentTarget) { + this.contentTarget.removeEventListener("mouseenter", this.handleMouseEnter); + this.contentTarget.removeEventListener("mouseleave", this.handleMouseLeave); + } } handleMouseEnter = () => { @@ -90,20 +101,27 @@ export default class extends Controller { }; showPopover() { + if (!this.hasTriggerTarget || !this.hasContentTarget) return; + this.contentTarget.classList.remove("hidden"); this.contentTarget.dataset.state = "open"; document.addEventListener("keydown", this.handleKeydown); this.updatePosition(); } + // Same rule as disconnect(): release what is held outside the element first, so a + // missing content target cannot leave the keydown listener or autoUpdate running. hidePopover() { - this.contentTarget.classList.add("hidden"); - this.contentTarget.dataset.state = "closed"; document.removeEventListener("keydown", this.handleKeydown); if (this.cleanup) { this.cleanup(); this.cleanup = null; } + + if (!this.hasContentTarget) return; + + this.contentTarget.classList.add("hidden"); + this.contentTarget.dataset.state = "closed"; } updatePosition() { @@ -112,6 +130,10 @@ export default class extends Controller { } this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => { + // autoUpdate keeps firing on scroll/resize; bail out rather than throw once + // a target is gone and nothing has torn the popover down yet. + if (!this.hasTriggerTarget || !this.hasContentTarget) return; + computePosition(this.triggerTarget, this.contentTarget, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], diff --git a/mcp/data/registry.json b/mcp/data/registry.json index cb19f4534..5cb5bf502 100644 --- a/mcp/data/registry.json +++ b/mcp/data/registry.json @@ -2057,7 +2057,7 @@ }, { "path": "popover_controller.js", - "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n removeElementEventListeners() {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.openValue = false;\n this.hidePopover();\n };\n\n showPopover() {\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n hidePopover() {\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n this.contentTarget.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n}\n" + "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element)\n // arrives already open. Re-arm the parts that live on the controller instead of\n // the markup — the keydown listener and the autoUpdate positioning.\n if (this.openValue) this.showPopover();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n // Each target is guarded on its own: losing one of them must not strand the\n // listeners attached to the other.\n removeElementEventListeners() {\n if (this.hasTriggerTarget) {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n if (this.hasContentTarget) {\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n }\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.openValue = false;\n this.hidePopover();\n };\n\n showPopover() {\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n // Same rule as disconnect(): release what is held outside the element first, so a\n // missing content target cannot leave the keydown listener or autoUpdate running.\n hidePopover() {\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n\n if (!this.hasContentTarget) return;\n\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n // autoUpdate keeps firing on scroll/resize; bail out rather than throw once\n // a target is gone and nothing has torn the popover down yet.\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n this.contentTarget.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n}\n" }, { "path": "popover_trigger.rb", From c86a8d504730aae19e0216d565409e1c3c124784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Djalma=20Ara=C3=BAjo?= Date: Thu, 30 Jul 2026 14:29:48 -0300 Subject: [PATCH 4/5] [Bug Fix] Popover: release autoUpdate on detached targets and keep openValue in sync --- .../controllers/ruby_ui/popover_controller.js | 73 +++++++++++-------- gem/lib/ruby_ui/popover/popover_controller.js | 73 +++++++++++-------- mcp/data/registry.json | 2 +- 3 files changed, 83 insertions(+), 65 deletions(-) diff --git a/docs/app/javascript/controllers/ruby_ui/popover_controller.js b/docs/app/javascript/controllers/ruby_ui/popover_controller.js index 80b8b1a99..660f76910 100644 --- a/docs/app/javascript/controllers/ruby_ui/popover_controller.js +++ b/docs/app/javascript/controllers/ruby_ui/popover_controller.js @@ -31,10 +31,7 @@ export default class extends Controller { clearTimeout(this.closeTimeout); document.removeEventListener("keydown", this.handleKeydown); document.removeEventListener("click", this.handleOutsideClick); - if (this.cleanup) { - this.cleanup(); - this.cleanup = null; - } + this.stopAutoUpdate(); this.removeElementEventListeners(); } @@ -67,28 +64,23 @@ export default class extends Controller { handleMouseEnter = () => { clearTimeout(this.closeTimeout); - this.openValue = true; this.showPopover(); }; handleMouseLeave = () => { - this.closeTimeout = setTimeout(() => { - this.openValue = false; - this.hidePopover(); - }, 100); + this.closeTimeout = setTimeout(() => this.hidePopover(), 100); }; handleClick = (event) => { event.stopPropagation(); - this.openValue = !this.openValue; - this.openValue ? this.showPopover() : this.hidePopover(); + this.openValue ? this.hidePopover() : this.showPopover(); }; handleOutsideClick = (event) => { - if (!this.element.contains(event.target) && this.openValue) { - this.openValue = false; - this.hidePopover(); - } + if (this.element.contains(event.target)) return; + if (!this.openValue) return; + + this.hidePopover(); }; handleKeydown = (event) => { @@ -96,13 +88,15 @@ export default class extends Controller { if (!this.openValue) return; clearTimeout(this.closeTimeout); - this.openValue = false; this.hidePopover(); }; + // openValue is set here rather than by the callers, so a guarded early return can + // never leave the DOM claiming the popover is open while nothing is wired up. showPopover() { if (!this.hasTriggerTarget || !this.hasContentTarget) return; + this.openValue = true; this.contentTarget.classList.remove("hidden"); this.contentTarget.dataset.state = "open"; document.addEventListener("keydown", this.handleKeydown); @@ -112,11 +106,9 @@ export default class extends Controller { // Same rule as disconnect(): release what is held outside the element first, so a // missing content target cannot leave the keydown listener or autoUpdate running. hidePopover() { + this.openValue = false; document.removeEventListener("keydown", this.handleKeydown); - if (this.cleanup) { - this.cleanup(); - this.cleanup = null; - } + this.stopAutoUpdate(); if (!this.hasContentTarget) return; @@ -125,27 +117,44 @@ export default class extends Controller { } updatePosition() { - if (this.cleanup) { - this.cleanup(); - } - - this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => { - // autoUpdate keeps firing on scroll/resize; bail out rather than throw once - // a target is gone and nothing has torn the popover down yet. - if (!this.hasTriggerTarget || !this.hasContentTarget) return; - - computePosition(this.triggerTarget, this.contentTarget, { + this.stopAutoUpdate(); + + // Hold the exact pair this run positions. A target can be detached or swapped + // while the controller stays connected, and the stale element must not be + // written to by an observer callback or an in-flight computePosition. + const trigger = this.triggerTarget; + const content = this.contentTarget; + + this.cleanup = autoUpdate(trigger, content, () => { + if (!trigger.isConnected || !content.isConnected) { + // Release the observers instead of throwing on every scroll and resize. + // Deferred because autoUpdate runs this once synchronously, before the + // handle below has been assigned. + queueMicrotask(() => this.stopAutoUpdate()); + return; + } + + computePosition(trigger, content, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], }).then(({ x, y, placement }) => { - Object.assign(this.contentTarget.style, { + if (!content.isConnected) return; + + Object.assign(content.style, { left: `${x}px`, top: `${y}px`, }); // flip() can resolve to the opposite side of the requested placement, // so the directional slide-in classes must follow the resolved value. - this.contentTarget.dataset.side = placement.split("-")[0]; + content.dataset.side = placement.split("-")[0]; }); }); } + + stopAutoUpdate() { + if (!this.cleanup) return; + + this.cleanup(); + this.cleanup = null; + } } diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index 80b8b1a99..660f76910 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -31,10 +31,7 @@ export default class extends Controller { clearTimeout(this.closeTimeout); document.removeEventListener("keydown", this.handleKeydown); document.removeEventListener("click", this.handleOutsideClick); - if (this.cleanup) { - this.cleanup(); - this.cleanup = null; - } + this.stopAutoUpdate(); this.removeElementEventListeners(); } @@ -67,28 +64,23 @@ export default class extends Controller { handleMouseEnter = () => { clearTimeout(this.closeTimeout); - this.openValue = true; this.showPopover(); }; handleMouseLeave = () => { - this.closeTimeout = setTimeout(() => { - this.openValue = false; - this.hidePopover(); - }, 100); + this.closeTimeout = setTimeout(() => this.hidePopover(), 100); }; handleClick = (event) => { event.stopPropagation(); - this.openValue = !this.openValue; - this.openValue ? this.showPopover() : this.hidePopover(); + this.openValue ? this.hidePopover() : this.showPopover(); }; handleOutsideClick = (event) => { - if (!this.element.contains(event.target) && this.openValue) { - this.openValue = false; - this.hidePopover(); - } + if (this.element.contains(event.target)) return; + if (!this.openValue) return; + + this.hidePopover(); }; handleKeydown = (event) => { @@ -96,13 +88,15 @@ export default class extends Controller { if (!this.openValue) return; clearTimeout(this.closeTimeout); - this.openValue = false; this.hidePopover(); }; + // openValue is set here rather than by the callers, so a guarded early return can + // never leave the DOM claiming the popover is open while nothing is wired up. showPopover() { if (!this.hasTriggerTarget || !this.hasContentTarget) return; + this.openValue = true; this.contentTarget.classList.remove("hidden"); this.contentTarget.dataset.state = "open"; document.addEventListener("keydown", this.handleKeydown); @@ -112,11 +106,9 @@ export default class extends Controller { // Same rule as disconnect(): release what is held outside the element first, so a // missing content target cannot leave the keydown listener or autoUpdate running. hidePopover() { + this.openValue = false; document.removeEventListener("keydown", this.handleKeydown); - if (this.cleanup) { - this.cleanup(); - this.cleanup = null; - } + this.stopAutoUpdate(); if (!this.hasContentTarget) return; @@ -125,27 +117,44 @@ export default class extends Controller { } updatePosition() { - if (this.cleanup) { - this.cleanup(); - } - - this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => { - // autoUpdate keeps firing on scroll/resize; bail out rather than throw once - // a target is gone and nothing has torn the popover down yet. - if (!this.hasTriggerTarget || !this.hasContentTarget) return; - - computePosition(this.triggerTarget, this.contentTarget, { + this.stopAutoUpdate(); + + // Hold the exact pair this run positions. A target can be detached or swapped + // while the controller stays connected, and the stale element must not be + // written to by an observer callback or an in-flight computePosition. + const trigger = this.triggerTarget; + const content = this.contentTarget; + + this.cleanup = autoUpdate(trigger, content, () => { + if (!trigger.isConnected || !content.isConnected) { + // Release the observers instead of throwing on every scroll and resize. + // Deferred because autoUpdate runs this once synchronously, before the + // handle below has been assigned. + queueMicrotask(() => this.stopAutoUpdate()); + return; + } + + computePosition(trigger, content, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], }).then(({ x, y, placement }) => { - Object.assign(this.contentTarget.style, { + if (!content.isConnected) return; + + Object.assign(content.style, { left: `${x}px`, top: `${y}px`, }); // flip() can resolve to the opposite side of the requested placement, // so the directional slide-in classes must follow the resolved value. - this.contentTarget.dataset.side = placement.split("-")[0]; + content.dataset.side = placement.split("-")[0]; }); }); } + + stopAutoUpdate() { + if (!this.cleanup) return; + + this.cleanup(); + this.cleanup = null; + } } diff --git a/mcp/data/registry.json b/mcp/data/registry.json index 5cb5bf502..3ead22139 100644 --- a/mcp/data/registry.json +++ b/mcp/data/registry.json @@ -2057,7 +2057,7 @@ }, { "path": "popover_controller.js", - "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element)\n // arrives already open. Re-arm the parts that live on the controller instead of\n // the markup — the keydown listener and the autoUpdate positioning.\n if (this.openValue) this.showPopover();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n // Each target is guarded on its own: losing one of them must not strand the\n // listeners attached to the other.\n removeElementEventListeners() {\n if (this.hasTriggerTarget) {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n if (this.hasContentTarget) {\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n }\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.openValue = true;\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => {\n this.openValue = false;\n this.hidePopover();\n }, 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue = !this.openValue;\n this.openValue ? this.showPopover() : this.hidePopover();\n };\n\n handleOutsideClick = (event) => {\n if (!this.element.contains(event.target) && this.openValue) {\n this.openValue = false;\n this.hidePopover();\n }\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.openValue = false;\n this.hidePopover();\n };\n\n showPopover() {\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n // Same rule as disconnect(): release what is held outside the element first, so a\n // missing content target cannot leave the keydown listener or autoUpdate running.\n hidePopover() {\n document.removeEventListener(\"keydown\", this.handleKeydown);\n if (this.cleanup) {\n this.cleanup();\n this.cleanup = null;\n }\n\n if (!this.hasContentTarget) return;\n\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n }\n\n updatePosition() {\n if (this.cleanup) {\n this.cleanup();\n }\n\n this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => {\n // autoUpdate keeps firing on scroll/resize; bail out rather than throw once\n // a target is gone and nothing has torn the popover down yet.\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n computePosition(this.triggerTarget, this.contentTarget, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n Object.assign(this.contentTarget.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n this.contentTarget.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n}\n" + "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element)\n // arrives already open. Re-arm the parts that live on the controller instead of\n // the markup — the keydown listener and the autoUpdate positioning.\n if (this.openValue) this.showPopover();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n this.stopAutoUpdate();\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n // Each target is guarded on its own: losing one of them must not strand the\n // listeners attached to the other.\n removeElementEventListeners() {\n if (this.hasTriggerTarget) {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n if (this.hasContentTarget) {\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n }\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => this.hidePopover(), 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue ? this.hidePopover() : this.showPopover();\n };\n\n handleOutsideClick = (event) => {\n if (this.element.contains(event.target)) return;\n if (!this.openValue) return;\n\n this.hidePopover();\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.hidePopover();\n };\n\n // openValue is set here rather than by the callers, so a guarded early return can\n // never leave the DOM claiming the popover is open while nothing is wired up.\n showPopover() {\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n this.openValue = true;\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n // Same rule as disconnect(): release what is held outside the element first, so a\n // missing content target cannot leave the keydown listener or autoUpdate running.\n hidePopover() {\n this.openValue = false;\n document.removeEventListener(\"keydown\", this.handleKeydown);\n this.stopAutoUpdate();\n\n if (!this.hasContentTarget) return;\n\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n }\n\n updatePosition() {\n this.stopAutoUpdate();\n\n // Hold the exact pair this run positions. A target can be detached or swapped\n // while the controller stays connected, and the stale element must not be\n // written to by an observer callback or an in-flight computePosition.\n const trigger = this.triggerTarget;\n const content = this.contentTarget;\n\n this.cleanup = autoUpdate(trigger, content, () => {\n if (!trigger.isConnected || !content.isConnected) {\n // Release the observers instead of throwing on every scroll and resize.\n // Deferred because autoUpdate runs this once synchronously, before the\n // handle below has been assigned.\n queueMicrotask(() => this.stopAutoUpdate());\n return;\n }\n\n computePosition(trigger, content, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n if (!content.isConnected) return;\n\n Object.assign(content.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n content.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n\n stopAutoUpdate() {\n if (!this.cleanup) return;\n\n this.cleanup();\n this.cleanup = null;\n }\n}\n" }, { "path": "popover_trigger.rb", From 404c15d6984eaa6e287f80374db453c86f80e9dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Djalma=20Ara=C3=BAjo?= Date: Thu, 30 Jul 2026 14:45:00 -0300 Subject: [PATCH 5/5] [Bug Fix] Popover: bind deferred autoUpdate teardown to the run that scheduled it --- .../controllers/ruby_ui/popover_controller.js | 14 ++++++++++++-- gem/lib/ruby_ui/popover/popover_controller.js | 14 ++++++++++++-- mcp/data/registry.json | 2 +- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/docs/app/javascript/controllers/ruby_ui/popover_controller.js b/docs/app/javascript/controllers/ruby_ui/popover_controller.js index 660f76910..90e6f062f 100644 --- a/docs/app/javascript/controllers/ruby_ui/popover_controller.js +++ b/docs/app/javascript/controllers/ruby_ui/popover_controller.js @@ -125,12 +125,20 @@ export default class extends Controller { const trigger = this.triggerTarget; const content = this.contentTarget; - this.cleanup = autoUpdate(trigger, content, () => { + // Deferred teardown is bound to this run's own handle, so a newer positioning + // run installed before the microtask drains is never torn down by an older one. + let stop = null; + const releaseThisRun = () => { + stop?.(); + if (this.cleanup === stop) this.cleanup = null; + }; + + stop = autoUpdate(trigger, content, () => { if (!trigger.isConnected || !content.isConnected) { // Release the observers instead of throwing on every scroll and resize. // Deferred because autoUpdate runs this once synchronously, before the // handle below has been assigned. - queueMicrotask(() => this.stopAutoUpdate()); + queueMicrotask(releaseThisRun); return; } @@ -149,6 +157,8 @@ export default class extends Controller { content.dataset.side = placement.split("-")[0]; }); }); + + this.cleanup = stop; } stopAutoUpdate() { diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index 660f76910..90e6f062f 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -125,12 +125,20 @@ export default class extends Controller { const trigger = this.triggerTarget; const content = this.contentTarget; - this.cleanup = autoUpdate(trigger, content, () => { + // Deferred teardown is bound to this run's own handle, so a newer positioning + // run installed before the microtask drains is never torn down by an older one. + let stop = null; + const releaseThisRun = () => { + stop?.(); + if (this.cleanup === stop) this.cleanup = null; + }; + + stop = autoUpdate(trigger, content, () => { if (!trigger.isConnected || !content.isConnected) { // Release the observers instead of throwing on every scroll and resize. // Deferred because autoUpdate runs this once synchronously, before the // handle below has been assigned. - queueMicrotask(() => this.stopAutoUpdate()); + queueMicrotask(releaseThisRun); return; } @@ -149,6 +157,8 @@ export default class extends Controller { content.dataset.side = placement.split("-")[0]; }); }); + + this.cleanup = stop; } stopAutoUpdate() { diff --git a/mcp/data/registry.json b/mcp/data/registry.json index 3ead22139..4ad7014a6 100644 --- a/mcp/data/registry.json +++ b/mcp/data/registry.json @@ -2057,7 +2057,7 @@ }, { "path": "popover_controller.js", - "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element)\n // arrives already open. Re-arm the parts that live on the controller instead of\n // the markup — the keydown listener and the autoUpdate positioning.\n if (this.openValue) this.showPopover();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n this.stopAutoUpdate();\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n // Each target is guarded on its own: losing one of them must not strand the\n // listeners attached to the other.\n removeElementEventListeners() {\n if (this.hasTriggerTarget) {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n if (this.hasContentTarget) {\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n }\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => this.hidePopover(), 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue ? this.hidePopover() : this.showPopover();\n };\n\n handleOutsideClick = (event) => {\n if (this.element.contains(event.target)) return;\n if (!this.openValue) return;\n\n this.hidePopover();\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.hidePopover();\n };\n\n // openValue is set here rather than by the callers, so a guarded early return can\n // never leave the DOM claiming the popover is open while nothing is wired up.\n showPopover() {\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n this.openValue = true;\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n // Same rule as disconnect(): release what is held outside the element first, so a\n // missing content target cannot leave the keydown listener or autoUpdate running.\n hidePopover() {\n this.openValue = false;\n document.removeEventListener(\"keydown\", this.handleKeydown);\n this.stopAutoUpdate();\n\n if (!this.hasContentTarget) return;\n\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n }\n\n updatePosition() {\n this.stopAutoUpdate();\n\n // Hold the exact pair this run positions. A target can be detached or swapped\n // while the controller stays connected, and the stale element must not be\n // written to by an observer callback or an in-flight computePosition.\n const trigger = this.triggerTarget;\n const content = this.contentTarget;\n\n this.cleanup = autoUpdate(trigger, content, () => {\n if (!trigger.isConnected || !content.isConnected) {\n // Release the observers instead of throwing on every scroll and resize.\n // Deferred because autoUpdate runs this once synchronously, before the\n // handle below has been assigned.\n queueMicrotask(() => this.stopAutoUpdate());\n return;\n }\n\n computePosition(trigger, content, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n if (!content.isConnected) return;\n\n Object.assign(content.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n content.dataset.side = placement.split(\"-\")[0];\n });\n });\n }\n\n stopAutoUpdate() {\n if (!this.cleanup) return;\n\n this.cleanup();\n this.cleanup = null;\n }\n}\n" + "content": "import { Controller } from \"@hotwired/stimulus\";\nimport {\n computePosition,\n flip,\n shift,\n offset,\n autoUpdate,\n} from \"@floating-ui/dom\";\n\nexport default class extends Controller {\n static targets = [\"trigger\", \"content\"];\n static values = {\n open: { type: Boolean, default: false },\n options: { type: Object, default: {} },\n trigger: { type: String, default: \"hover\" },\n };\n\n connect() {\n this.closeTimeout = null;\n this.cleanup = null;\n this.addEventListeners();\n // openValue lives in the DOM, so a reconnect (frame swap, morph, moved element)\n // arrives already open. Re-arm the parts that live on the controller instead of\n // the markup — the keydown listener and the autoUpdate positioning.\n if (this.openValue) this.showPopover();\n }\n\n // Teardown that cannot fail comes first: resolving a target throws once the\n // element is gone, and Stimulus swallows that, skipping the rest of disconnect.\n disconnect() {\n clearTimeout(this.closeTimeout);\n document.removeEventListener(\"keydown\", this.handleKeydown);\n document.removeEventListener(\"click\", this.handleOutsideClick);\n this.stopAutoUpdate();\n this.removeElementEventListeners();\n }\n\n addEventListeners() {\n if (this.triggerValue === \"hover\") {\n this.triggerTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n this.contentTarget.addEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.addEventListener(\"mouseleave\", this.handleMouseLeave);\n } else if (this.triggerValue === \"click\") {\n this.triggerTarget.addEventListener(\"click\", this.handleClick);\n document.addEventListener(\"click\", this.handleOutsideClick);\n }\n }\n\n // Each target is guarded on its own: losing one of them must not strand the\n // listeners attached to the other.\n removeElementEventListeners() {\n if (this.hasTriggerTarget) {\n this.triggerTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.triggerTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n this.triggerTarget.removeEventListener(\"click\", this.handleClick);\n }\n\n if (this.hasContentTarget) {\n this.contentTarget.removeEventListener(\"mouseenter\", this.handleMouseEnter);\n this.contentTarget.removeEventListener(\"mouseleave\", this.handleMouseLeave);\n }\n }\n\n handleMouseEnter = () => {\n clearTimeout(this.closeTimeout);\n this.showPopover();\n };\n\n handleMouseLeave = () => {\n this.closeTimeout = setTimeout(() => this.hidePopover(), 100);\n };\n\n handleClick = (event) => {\n event.stopPropagation();\n this.openValue ? this.hidePopover() : this.showPopover();\n };\n\n handleOutsideClick = (event) => {\n if (this.element.contains(event.target)) return;\n if (!this.openValue) return;\n\n this.hidePopover();\n };\n\n handleKeydown = (event) => {\n if (event.key !== \"Escape\") return;\n if (!this.openValue) return;\n\n clearTimeout(this.closeTimeout);\n this.hidePopover();\n };\n\n // openValue is set here rather than by the callers, so a guarded early return can\n // never leave the DOM claiming the popover is open while nothing is wired up.\n showPopover() {\n if (!this.hasTriggerTarget || !this.hasContentTarget) return;\n\n this.openValue = true;\n this.contentTarget.classList.remove(\"hidden\");\n this.contentTarget.dataset.state = \"open\";\n document.addEventListener(\"keydown\", this.handleKeydown);\n this.updatePosition();\n }\n\n // Same rule as disconnect(): release what is held outside the element first, so a\n // missing content target cannot leave the keydown listener or autoUpdate running.\n hidePopover() {\n this.openValue = false;\n document.removeEventListener(\"keydown\", this.handleKeydown);\n this.stopAutoUpdate();\n\n if (!this.hasContentTarget) return;\n\n this.contentTarget.classList.add(\"hidden\");\n this.contentTarget.dataset.state = \"closed\";\n }\n\n updatePosition() {\n this.stopAutoUpdate();\n\n // Hold the exact pair this run positions. A target can be detached or swapped\n // while the controller stays connected, and the stale element must not be\n // written to by an observer callback or an in-flight computePosition.\n const trigger = this.triggerTarget;\n const content = this.contentTarget;\n\n // Deferred teardown is bound to this run's own handle, so a newer positioning\n // run installed before the microtask drains is never torn down by an older one.\n let stop = null;\n const releaseThisRun = () => {\n stop?.();\n if (this.cleanup === stop) this.cleanup = null;\n };\n\n stop = autoUpdate(trigger, content, () => {\n if (!trigger.isConnected || !content.isConnected) {\n // Release the observers instead of throwing on every scroll and resize.\n // Deferred because autoUpdate runs this once synchronously, before the\n // handle below has been assigned.\n queueMicrotask(releaseThisRun);\n return;\n }\n\n computePosition(trigger, content, {\n placement: this.optionsValue.placement || \"bottom\",\n middleware: [flip(), shift(), offset(8)],\n }).then(({ x, y, placement }) => {\n if (!content.isConnected) return;\n\n Object.assign(content.style, {\n left: `${x}px`,\n top: `${y}px`,\n });\n // flip() can resolve to the opposite side of the requested placement,\n // so the directional slide-in classes must follow the resolved value.\n content.dataset.side = placement.split(\"-\")[0];\n });\n });\n\n this.cleanup = stop;\n }\n\n stopAutoUpdate() {\n if (!this.cleanup) return;\n\n this.cleanup();\n this.cleanup = null;\n }\n}\n" }, { "path": "popover_trigger.rb",