diff --git a/docs/app/javascript/controllers/ruby_ui/popover_controller.js b/docs/app/javascript/controllers/ruby_ui/popover_controller.js index 9b847764d..90e6f062f 100644 --- a/docs/app/javascript/controllers/ruby_ui/popover_controller.js +++ b/docs/app/javascript/controllers/ruby_ui/popover_controller.js @@ -19,13 +19,20 @@ 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 + // element is gone, and Stimulus swallows that, skipping the rest of disconnect. disconnect() { - this.removeEventListeners(); - if (this.cleanup) { - this.cleanup(); - } + clearTimeout(this.closeTimeout); + document.removeEventListener("keydown", this.handleKeydown); + document.removeEventListener("click", this.handleOutsideClick); + this.stopAutoUpdate(); + this.removeElementEventListeners(); } addEventListeners() { @@ -40,68 +47,124 @@ export default class extends Controller { } } - removeEventListeners() { - 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); + // Each target is guarded on its own: losing one of them must not strand the + // listeners attached to the other. + removeElementEventListeners() { + 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 = () => { 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) => { + if (event.key !== "Escape") return; + if (!this.openValue) return; + + clearTimeout(this.closeTimeout); + 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); 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.openValue = false; + document.removeEventListener("keydown", this.handleKeydown); + this.stopAutoUpdate(); + + if (!this.hasContentTarget) return; + this.contentTarget.classList.add("hidden"); - if (this.cleanup) { - this.cleanup(); - } + this.contentTarget.dataset.state = "closed"; } updatePosition() { - if (this.cleanup) { - this.cleanup(); - } + 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; + + // 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; + }; - this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => { - computePosition(this.triggerTarget, this.contentTarget, { + 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(releaseThisRun); + return; + } + + computePosition(trigger, content, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], - }).then(({ x, y }) => { - Object.assign(this.contentTarget.style, { + }).then(({ x, y, placement }) => { + 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. + content.dataset.side = placement.split("-")[0]; }); }); + + this.cleanup = stop; + } + + stopAutoUpdate() { + if (!this.cleanup) return; + + this.cleanup(); + this.cleanup = null; } } 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..90e6f062f 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -19,13 +19,20 @@ 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 + // element is gone, and Stimulus swallows that, skipping the rest of disconnect. disconnect() { - this.removeEventListeners(); - if (this.cleanup) { - this.cleanup(); - } + clearTimeout(this.closeTimeout); + document.removeEventListener("keydown", this.handleKeydown); + document.removeEventListener("click", this.handleOutsideClick); + this.stopAutoUpdate(); + this.removeElementEventListeners(); } addEventListeners() { @@ -40,68 +47,124 @@ export default class extends Controller { } } - removeEventListeners() { - 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); + // Each target is guarded on its own: losing one of them must not strand the + // listeners attached to the other. + removeElementEventListeners() { + 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 = () => { 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) => { + if (event.key !== "Escape") return; + if (!this.openValue) return; + + clearTimeout(this.closeTimeout); + 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); 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.openValue = false; + document.removeEventListener("keydown", this.handleKeydown); + this.stopAutoUpdate(); + + if (!this.hasContentTarget) return; + this.contentTarget.classList.add("hidden"); - if (this.cleanup) { - this.cleanup(); - } + this.contentTarget.dataset.state = "closed"; } updatePosition() { - if (this.cleanup) { - this.cleanup(); - } + 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; + + // 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; + }; - this.cleanup = autoUpdate(this.triggerTarget, this.contentTarget, () => { - computePosition(this.triggerTarget, this.contentTarget, { + 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(releaseThisRun); + return; + } + + computePosition(trigger, content, { placement: this.optionsValue.placement || "bottom", middleware: [flip(), shift(), offset(8)], - }).then(({ x, y }) => { - Object.assign(this.contentTarget.style, { + }).then(({ x, y, placement }) => { + 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. + content.dataset.side = placement.split("-")[0]; }); }); + + this.cleanup = stop; + } + + stopAutoUpdate() { + if (!this.cleanup) return; + + this.cleanup(); + this.cleanup = null; } } 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..4ad7014a6 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 // 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",