From 0177632a6ee99c28cce4e3b7f42b38a90074196d Mon Sep 17 00:00:00 2001 From: Stephen Nelson Date: Sat, 28 Oct 2023 13:16:42 +1030 Subject: [PATCH] Keep modal in history when opened via a direct link --- Gemfile.lock | 2 +- .../kpop/controllers/frame_controller.js | 14 +++++++++-- app/javascript/kpop/modals/content_modal.js | 2 +- lib/katalyst/kpop/version.rb | 2 +- spec/system/content_modal_spec.rb | 20 +++++++-------- spec/system/redirect_modal_spec.rb | 25 +++++++++++-------- 6 files changed, 40 insertions(+), 25 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 144d6b5..b35b7d8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,7 +9,7 @@ GIT PATH remote: . specs: - katalyst-kpop (3.0.0.beta.2) + katalyst-kpop (3.0.0.beta.3) html-attributes-utils turbo-rails view_component diff --git a/app/javascript/kpop/controllers/frame_controller.js b/app/javascript/kpop/controllers/frame_controller.js index 92482b4..c2efae0 100644 --- a/app/javascript/kpop/controllers/frame_controller.js +++ b/app/javascript/kpop/controllers/frame_controller.js @@ -45,7 +45,11 @@ export default class Kpop__FrameController extends Controller { this.scrimConnected = true; - if (this.openValue) scrim.show({ animate: false }); + if (this.openValue) { + scrim.show({ animate: false }); + } else { + scrim.hide({ animate: false }); + } } openValueChanged(open) { @@ -57,6 +61,7 @@ export default class Kpop__FrameController extends Controller { async open(modal, { animate = true } = {}) { if (this.isOpen) { this.debug("skip open as already open"); + this.modal ||= modal; return false; } @@ -152,6 +157,12 @@ export default class Kpop__FrameController extends Controller { async #dismiss({ animate = true, reason = "" } = {}) { this.debug("dismiss-start", { animate, reason }); + // if this element is detached then we've experienced a turbo navigation + if (!this.element.isConnected) { + this.debug("skip dismiss, element detached"); + return; + } + if (!this.modal) { console.warn("modal missing on dismiss"); if (DEBUG) debugger; @@ -168,7 +179,6 @@ export default class Kpop__FrameController extends Controller { } async #nextFrame(callback) { - // return Promise.resolve().then(callback); return new Promise(window.requestAnimationFrame).then(callback); } diff --git a/app/javascript/kpop/modals/content_modal.js b/app/javascript/kpop/modals/content_modal.js index 647a2cf..cffd23d 100644 --- a/app/javascript/kpop/modals/content_modal.js +++ b/app/javascript/kpop/modals/content_modal.js @@ -23,7 +23,7 @@ export class ContentModal extends Modal { return this.pop("turbo:load", () => { this.debug("turbo-visit", this.fallbackLocationValue); - Turbo.visit(this.fallbackLocationValue, { action: "replace" }); + Turbo.visit(this.fallbackLocationValue); }); // no specific close action required, this is turbo's responsibility diff --git a/lib/katalyst/kpop/version.rb b/lib/katalyst/kpop/version.rb index a2571a6..69792b1 100644 --- a/lib/katalyst/kpop/version.rb +++ b/lib/katalyst/kpop/version.rb @@ -2,6 +2,6 @@ module Katalyst module Kpop - VERSION = "3.0.0.beta.2" + VERSION = "3.0.0.beta.3" end end diff --git a/spec/system/content_modal_spec.rb b/spec/system/content_modal_spec.rb index 72a1401..c02fefd 100644 --- a/spec/system/content_modal_spec.rb +++ b/spec/system/content_modal_spec.rb @@ -24,10 +24,10 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button reaches the end of the history stack + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(nil) + expect(page).to have_current_path(modal_path) end it "supports scrim closing" do @@ -37,10 +37,10 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button reaches the end of the history stack + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(nil) + expect(page).to have_current_path(modal_path) end it "supports forward navigation" do @@ -73,10 +73,10 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button reaches the end of the history stack + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(nil) + expect(page).to have_current_path(modal_path) end it "supports redirect via form submission" do @@ -114,10 +114,10 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button reaches the end of the history stack + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(nil) + expect(page).to have_current_path(modal_path) end it "supports navigation re-opening" do @@ -153,9 +153,9 @@ expect(page).to have_current_path(root_path) - # Clicking the back button reaches the end of the history stack + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(nil) + expect(page).to have_current_path(modal_path) end end diff --git a/spec/system/redirect_modal_spec.rb b/spec/system/redirect_modal_spec.rb index 78123e8..66d4b08 100644 --- a/spec/system/redirect_modal_spec.rb +++ b/spec/system/redirect_modal_spec.rb @@ -25,10 +25,11 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button returns to the first page (root) + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(root_path) + expect(page).to have_current_path(modal_path) + expect(page).to have_css(".kpop-modal") end it "supports scrim closing" do @@ -38,10 +39,11 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button returns to the first page (root) + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(root_path) + expect(page).to have_current_path(modal_path) + expect(page).to have_css(".kpop-modal") end it "supports forward navigation" do @@ -74,10 +76,11 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button returns to the first page (root) + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(root_path) + expect(page).to have_current_path(modal_path) + expect(page).to have_css(".kpop-modal") end it "supports redirect via form submission" do @@ -115,10 +118,11 @@ expect(page).to have_current_path(root_path) expect(page).not_to have_css(".kpop-modal") - # Clicking the back button returns to the first page (root) + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(root_path) + expect(page).to have_current_path(modal_path) + expect(page).to have_css(".kpop-modal") end it "supports navigation re-opening" do @@ -154,9 +158,10 @@ expect(page).to have_current_path(root_path) - # Clicking the back button returns to the first page (root) + # Clicking the back button returns to the modal page.go_back - expect(page).to have_current_path(root_path) + expect(page).to have_current_path(modal_path) + expect(page).to have_css(".kpop-modal") end end