From 8e5ce88c43f0bb3412ff01a4c309abcb9a2b082f Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Wed, 7 Oct 2026 00:32:40 +0300 Subject: [PATCH] refactor: use LiveView modal focus and mount lifecycle --- assets/js/hooks/modal.js | 67 +++++++++---------- demo/lib/demo_web/live/fixtures_live.ex | 7 ++ .../modal_rerender_title_fixture.html.heex | 7 +- .../wallaby/demo_web/modal_focus_test.exs | 51 ++++++-------- .../demo_web/modal_rerender_title_test.exs | 67 ++++++++++++------- lib/prima/modal.ex | 4 +- 6 files changed, 110 insertions(+), 93 deletions(-) diff --git a/assets/js/hooks/modal.js b/assets/js/hooks/modal.js index 79708ed..0308813 100644 --- a/assets/js/hooks/modal.js +++ b/assets/js/hooks/modal.js @@ -13,6 +13,7 @@ export default { }, destroyed() { + this.finishClose() this.cleanup() }, @@ -20,7 +21,6 @@ export default { this.cleanup() this.setupElements() this.setupDOMEventListeners() - this.checkInitialShow() this.js().setAttribute(this.el, 'data-prima-ready', 'true') }, @@ -61,7 +61,7 @@ export default { // Focus management - when panel is shown, focus first element if (this.ref("modal-panel")) { this.listeners.push( - [this.ref("modal-panel"), "phx:show-end", this.handlePanelShowEnd.bind(this)] + [this.ref("modal-panel"), "phx:show-end", this.focusFirstElement.bind(this)] ) } @@ -75,29 +75,22 @@ export default { cleanup() { if (this.listeners) { this.listeners.forEach(([element, event, handler]) => { - element.removeEventListener(event, handler) + element?.removeEventListener(event, handler) }) this.listeners = [] } }, - checkInitialShow() { - if (Object.hasOwn(this.el.dataset, 'primaShow')) { - this.el.dispatchEvent(new Event('prima:modal:open')) - } - }, - handleModalOpen() { - this.storeFocusedElement() + if (this.isOpen()) return + + // Execute against the active element so LiveView saves the opener, not the modal. + this.liveSocket.execJS(document.activeElement, this.el.getAttribute('js-push-focus')) this.preventBodyScroll() this.js().removeAttribute(this.el, 'aria-hidden') this.maybeExecJS(this.el, "js-show"); this.maybeExecJS(this.ref("modal-overlay"), "js-show"); - if (this.async) { - this.maybeExecJS(this.ref("modal-loader"), "js-show"); - } else { - this.maybeExecJS(this.ref("modal-panel"), "js-show"); - } + this.maybeExecJS(this.ref(this.async ? "modal-loader" : "modal-panel"), "js-show"); }, handlePanelMounted() { @@ -106,7 +99,7 @@ export default { this.setupAriaRelationships() this.js().removeAttribute(this.el, 'aria-hidden') - const panelShowEndHandler = this.handlePanelShowEnd.bind(this) + const panelShowEndHandler = this.focusFirstElement.bind(this) this.ref("modal-panel").addEventListener("phx:show-end", panelShowEndHandler); this.listeners.push([this.ref("modal-panel"), "phx:show-end", panelShowEndHandler]) }, @@ -118,33 +111,45 @@ export default { }, handleModalClose() { - this.restoreBodyScroll() + if (!this.isOpen()) return + this.maybeExecJS(this.ref("modal-overlay"), "js-hide"); this.maybeExecJS(this.ref("modal-panel"), "js-hide"); this.maybeExecJS(this.ref("modal-loader"), "js-hide"); - if (this.async) { - this.ref("modal-panel").dataset.primaDirty = true + const panel = this.ref("modal-panel") + if (this.async && panel) { + panel.dataset.primaDirty = true } }, handleOverlayHideEnd() { + if (!this.isOpen()) return + this.maybeExecJS(this.el, "js-hide"); - this.js().setAttribute(this.el, 'aria-hidden', 'true') - this.restoreFocusedElement() + this.finishClose() + }, + + isOpen() { + return this.el.getAttribute('aria-hidden') !== 'true' }, - handlePanelShowEnd() { - this.focusFirstElement() + finishClose() { + if (!this.isOpen()) return + + this.js().setAttribute(this.el, 'aria-hidden', 'true') + this.restoreBodyScroll() + this.maybeExecJS(this.el, 'js-pop-focus') }, maybeExecJS(el, attribute) { - if (el && el.getAttribute(attribute)) { - this.liveSocket.execJS(el, el.getAttribute(attribute)); + const command = el?.getAttribute(attribute) + if (command) { + this.liveSocket.execJS(el, command); } }, panelIsDirty() { - return this.ref('modal-panel') && this.ref("modal-panel").dataset.primaDirty + return this.ref('modal-panel')?.dataset.primaDirty }, ref(ref) { @@ -180,16 +185,6 @@ export default { } }, - storeFocusedElement() { - this.previouslyFocusedElement = document.activeElement - }, - - restoreFocusedElement() { - if (this.previouslyFocusedElement && this.previouslyFocusedElement.focus) { - this.previouslyFocusedElement.focus() - } - }, - focusFirstElement() { const panel = this.ref("modal-panel") diff --git a/demo/lib/demo_web/live/fixtures_live.ex b/demo/lib/demo_web/live/fixtures_live.ex index dd9856f..66d6b44 100644 --- a/demo/lib/demo_web/live/fixtures_live.ex +++ b/demo/lib/demo_web/live/fixtures_live.ex @@ -28,6 +28,8 @@ defmodule DemoWeb.FixturesLive do |> assign(listbox_disabled?: false, submitted_fruit: "not submitted") |> assign(trigger_label: "Open Dropdown") |> assign(modal_title: "Good news") + |> assign(modal_initial_show?: params["show"] == "true") + |> assign(modal_present?: true) |> stream_configure(:suggestions, dom_id: &"suggestions-#{&1}") |> stream(:suggestions, []) @@ -116,6 +118,11 @@ defmodule DemoWeb.FixturesLive do {:noreply, assign(socket, modal_title: "Updated Title")} end + @impl true + def handle_event("remove-modal", _params, socket) do + {:noreply, assign(socket, modal_present?: false)} + end + def handle_event("close-frontend-modal", _params, socket) do {:noreply, Prima.Modal.push_close(socket)} end diff --git a/demo/lib/demo_web/live/fixtures_live/modal_rerender_title_fixture.html.heex b/demo/lib/demo_web/live/fixtures_live/modal_rerender_title_fixture.html.heex index ab0ab4a..86b7307 100644 --- a/demo/lib/demo_web/live/fixtures_live/modal_rerender_title_fixture.html.heex +++ b/demo/lib/demo_web/live/fixtures_live/modal_rerender_title_fixture.html.heex @@ -3,11 +3,11 @@ Open Modal - - <.modal id="demo-modal"> + <.modal :if={@modal_present?} id="demo-modal" show={@modal_initial_show?}> <.modal_overlay />
@@ -26,6 +26,9 @@ + <.button phx-click={Prima.Modal.JS.close()} type="button"> Got it diff --git a/demo/test/wallaby/demo_web/modal_focus_test.exs b/demo/test/wallaby/demo_web/modal_focus_test.exs index de4e09e..8ebb66c 100644 --- a/demo/test/wallaby/demo_web/modal_focus_test.exs +++ b/demo/test/wallaby/demo_web/modal_focus_test.exs @@ -4,51 +4,40 @@ defmodule DemoWeb.ModalFocusTest do @modal_container Query.css("#demo-modal") @autofocus_modal_container Query.css("#autofocus-modal") - feature "focuses first focusable element when no autofocus element present", %{session: session} do + feature "default focus is restored across repeated opens and closes", %{session: session} do session |> visit_fixture("/fixtures/simple-modal", "#demo-modal") |> click(Query.css("#simple-modal button")) - |> assert_has(@modal_container |> Query.visible(true)) - # The first focusable element (close button) should be focused |> assert_has(Query.css("#demo-modal [testing-ref=close-button]:focus")) + |> execute_script(""" + document.querySelector('#demo-modal').dispatchEvent(new Event('prima:modal:open')) + """) + |> send_keys([:escape]) + |> assert_has(@modal_container |> Query.visible(false)) + |> assert_has(Query.css("#simple-modal button:focus")) + |> click(Query.css("#simple-modal button")) + |> assert_has(Query.css("#demo-modal [testing-ref=close-button]:focus")) + |> send_keys([:escape]) + |> assert_has(Query.css("#simple-modal button:focus")) end - feature "focuses element with data-autofocus when present", %{session: session} do + feature "backend open and close restore focus to the trigger", %{session: session} do session - |> visit_fixture("/fixtures/modal-focus-autofocus", "#autofocus-modal") - |> click(Query.css("#modal-focus-autofocus button")) - |> assert_has(@autofocus_modal_container |> Query.visible(true)) - # The input with data-autofocus should be focused - |> assert_has(Query.css("#autofocus-modal [testing-ref=autofocus-input]:focus")) + |> visit_fixture("/fixtures/modal-push-event", "#frontend-modal") + |> click(Query.button("Open Modal One via Backend")) + |> assert_has(Query.css("#modal-one [testing-ref=modal-one-close]:focus")) + |> click(Query.css("#backend-close-modal-one")) + |> assert_has(Query.css("#modal-one", visible: false)) + |> assert_has(Query.css("#modal-push-event button:focus", text: "Open Modal One via Backend")) end - feature "restores focus to trigger when modal closes with autofocus", %{session: session} do + feature "autofocus is applied on open and restored to the trigger on close", %{session: session} do session |> visit_fixture("/fixtures/modal-focus-autofocus", "#autofocus-modal") - # Focus the trigger button first - |> execute_script("document.querySelector('#modal-focus-autofocus button').focus()") - |> assert_has(Query.css("#modal-focus-autofocus button:focus")) |> click(Query.css("#modal-focus-autofocus button")) - |> assert_has(@autofocus_modal_container |> Query.visible(true)) - # Close with escape key + |> assert_has(Query.css("#autofocus-modal [testing-ref=autofocus-input]:focus")) |> send_keys([:escape]) |> assert_has(@autofocus_modal_container |> Query.visible(false)) - # Focus should return to the trigger button |> assert_has(Query.css("#modal-focus-autofocus button:focus")) end - - feature "restores focus to trigger when modal closes with default focus", %{session: session} do - session - |> visit_fixture("/fixtures/simple-modal", "#demo-modal") - # Focus the trigger button first - |> execute_script("document.querySelector('#simple-modal button').focus()") - |> assert_has(Query.css("#simple-modal button:focus")) - |> click(Query.css("#simple-modal button")) - |> assert_has(@modal_container |> Query.visible(true)) - # Close with escape key - |> send_keys([:escape]) - |> assert_has(@modal_container |> Query.visible(false)) - # Focus should return to the trigger button - |> assert_has(Query.css("#simple-modal button:focus")) - end end diff --git a/demo/test/wallaby/demo_web/modal_rerender_title_test.exs b/demo/test/wallaby/demo_web/modal_rerender_title_test.exs index 0565931..fe3c392 100644 --- a/demo/test/wallaby/demo_web/modal_rerender_title_test.exs +++ b/demo/test/wallaby/demo_web/modal_rerender_title_test.exs @@ -7,65 +7,86 @@ defmodule DemoWeb.ModalRerenderTitleTest do @update_button Query.css("#update-title") @update_button_inside Query.css("#update-title-inside") - feature "modal remains functional after title is re-rendered", %{session: session} do + feature "removing an open modal restores focus and body scrolling", %{session: session} do session |> visit_fixture("/fixtures/modal-rerender-title", "#demo-modal") - # Open modal |> click(@open_button) + |> assert_has(Query.css("#demo-modal [testing-ref=close-button]:focus")) + |> click(Query.css("#remove-modal")) + |> assert_missing(Query.css("#demo-modal", visible: :any)) + |> assert_has(Query.css("#modal-rerender button:focus", text: "Open Modal")) + |> execute_script("return document.body.style.overflow", fn overflow -> + assert overflow == "" + end) + end + + feature "dismissed initially shown modal stays closed through rerender and reconnect", %{ + session: session + } do + session + |> visit_fixture("/fixtures/modal-rerender-title?show=true", "#demo-modal") |> assert_has(@modal_container |> Query.visible(true)) - |> assert_has(@modal_panel |> Query.visible(true)) - # Close modal + |> assert_has(Query.css("#demo-modal [testing-ref=close-button]:focus")) |> send_keys([:escape]) |> assert_has(@modal_container |> Query.visible(false)) - # Trigger LiveView update that re-renders the title |> click(@update_button) - # Open modal again - tests that DOM listeners are still intact + |> assert_has(Query.css("#update-title[data-title='Updated Title']")) + |> assert_has(@modal_container |> Query.visible(false)) + |> execute_script("window.liveSocket.disconnect()") + |> execute_script("window.liveSocket.connect()") + |> assert_has(Query.css(".phx-connected[data-phx-main]")) + |> assert_has(@modal_container |> Query.visible(false)) |> click(@open_button) - |> assert_has(@modal_container |> Query.visible(true)) - |> assert_has(@modal_panel |> Query.visible(true)) - # Verify the title was updated - |> assert_has(Query.css("#demo-modal [data-prima-ref=modal-title]", text: "Updated Title")) - # Close with escape to verify keyboard listener works + |> assert_has(Query.css("#demo-modal [testing-ref=close-button]:focus")) + |> send_keys([:escape]) + |> assert_has(Query.css("#modal-rerender button:focus", text: "Open Modal")) + end + + feature "initially shown modal preserves focus and scroll restoration across a rerender", %{ + session: session + } do + session + |> visit_fixture("/fixtures/modal-rerender-title?show=true", "#demo-modal") + |> assert_has(Query.css("#demo-modal [testing-ref=close-button]:focus")) + |> click(@update_button_inside) + |> assert_has(Query.css("#demo-modal-title", text: "Updated Title")) + |> assert_has(Query.css("#update-title-inside:focus")) |> send_keys([:escape]) |> assert_has(@modal_container |> Query.visible(false)) + |> execute_script("return document.body.style.overflow", fn overflow -> + assert overflow == "" + end) end - feature "ARIA attributes are correctly set after title is re-rendered", %{session: session} do + feature "title rerenders preserve accessibility and open/close behavior", %{session: session} do session |> visit_fixture("/fixtures/modal-rerender-title", "#demo-modal") - # Open modal and verify initial ARIA relationships |> click(@open_button) - |> assert_has(@modal_container |> Query.visible(true)) |> assert_has(Query.css("#demo-modal[aria-labelledby='demo-modal-title']")) |> assert_has(Query.css("#demo-modal-title", text: "Good news")) - # Close modal |> send_keys([:escape]) |> assert_has(@modal_container |> Query.visible(false)) - # Trigger LiveView update that re-renders the title |> click(@update_button) - # Open modal again + |> assert_has(Query.css("#update-title[data-title='Updated Title']")) |> click(@open_button) - |> assert_has(@modal_container |> Query.visible(true)) - # Verify ARIA relationships are still correct after re-render + |> assert_has(@modal_panel |> Query.visible(true)) |> assert_has(Query.css("#demo-modal[aria-labelledby='demo-modal-title']")) |> assert_has(Query.css("#demo-modal-title", text: "Updated Title")) + |> send_keys([:escape]) + |> assert_has(@modal_container |> Query.visible(false)) end feature "modal remains open when re-rendered while open", %{session: session} do session |> visit_fixture("/fixtures/modal-rerender-title", "#demo-modal") - # Open modal |> click(@open_button) |> assert_has(@modal_container |> Query.visible(true)) |> assert_has(@modal_panel |> Query.visible(true)) |> assert_has(Query.css("#demo-modal-title", text: "Good news")) - # Trigger LiveView update while modal is open (using button inside modal) |> click(@update_button_inside) - # Modal should remain open and show updated content |> assert_has(@modal_container |> Query.visible(true)) |> assert_has(@modal_panel |> Query.visible(true)) |> assert_has(Query.css("#demo-modal-title", text: "Updated Title")) - # Modal should still be functional - close with escape |> send_keys([:escape]) |> assert_has(@modal_container |> Query.visible(false)) end diff --git a/lib/prima/modal.ex b/lib/prima/modal.ex index cf80881..9b59ba7 100644 --- a/lib/prima/modal.ex +++ b/lib/prima/modal.ex @@ -176,7 +176,9 @@ defmodule Prima.Modal do id={@id} js-show={JS.show()} js-hide={@on_close |> JS.hide()} - data-prima-show={@show} + js-push-focus={JS.push_focus()} + js-pop-focus={JS.pop_focus()} + phx-mounted={@show && Prima.Modal.JS.open(@id)} style="display: none;" phx-hook="Modal" class={@class}