From 130e039cf12a6e5d89090ac67243d07116a05972 Mon Sep 17 00:00:00 2001 From: Christian Hausknecht Date: Wed, 9 Oct 2024 10:41:02 +0200 Subject: [PATCH] Fixes some bug with modals and reactive renderings (#907) The combination of a trigger inside some reactive scope and a headless modal would lead to corrupt application state before this fix. Due to the natur of portalling, portalled portions of components are not automatically coupled to the reactive rendering of fritz2's mountpoint-concept. The portal-root container resides out of its scope. That is why we forward the scope to each portal-container and could use the latter to register some lifecycle-hooks in order to remove obsolete portals based upon the re-rendering happening inside fritz2's main `renderContext`. As modals do not have any relation to its trigger(s) and do not reside inside the same mount-point, they were not coupled to the trigger-flow and its cancellation. This is now fixed! If the trigger is removed due to some reactive rendering, also the triggered modal will be removed. This holds also for the usage from within some `Router`-based reactive render-context! So if one manipulates the hash-routing segment, the modal will also be closed now. Notice: Even though toasts are rendered in a similar fashion like modals, the former are handled totally agnostic from any reactive coupled trigger! As the triggering is decoupled from the destruction of a toast, the described bug did not affect toasts! - fixes #906 Co-authored-by: christian.hausknecht --- .../kotlin/dev/fritz2/headlessdemo/app.kt | 12 ++++---- .../dev/fritz2/headless/components/modal.kt | 28 +++++++++++++++++-- .../fritz2/headless/foundation/portalling.kt | 21 ++++++++++---- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/headless-demo/src/jsMain/kotlin/dev/fritz2/headlessdemo/app.kt b/headless-demo/src/jsMain/kotlin/dev/fritz2/headlessdemo/app.kt index 7f5207576..f8e85b2c2 100644 --- a/headless-demo/src/jsMain/kotlin/dev/fritz2/headlessdemo/app.kt +++ b/headless-demo/src/jsMain/kotlin/dev/fritz2/headlessdemo/app.kt @@ -137,12 +137,14 @@ fun main() { val router = routerOf("") render { - router.data.render { route -> - div("p-4", scope = { set(SHOW_COMPONENT_STRUCTURE, true) }) { - (pages[route]?.content ?: RenderContext::overview)() + main(scope = { set(SHOW_COMPONENT_STRUCTURE, true) }) { + router.data.render { route -> + div("p-4") { + (pages[route]?.content ?: RenderContext::overview)() + } } - } - portalRoot() + portalRoot() + } } } diff --git a/headless/src/jsMain/kotlin/dev/fritz2/headless/components/modal.kt b/headless/src/jsMain/kotlin/dev/fritz2/headless/components/modal.kt index 5c38c5b95..77c6cb6b8 100644 --- a/headless/src/jsMain/kotlin/dev/fritz2/headless/components/modal.kt +++ b/headless/src/jsMain/kotlin/dev/fritz2/headless/components/modal.kt @@ -5,6 +5,7 @@ import dev.fritz2.headless.foundation.* import kotlinx.coroutines.Job import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.take import org.w3c.dom.* @@ -30,12 +31,35 @@ class Modal(id: String?) : OpenClose() { fun init() { opened.filter { it }.handledBy { PortalRenderContext.run { - portal(id = componentId, tag = RenderContext::dialog, scope = scopeContext) { close -> + portal(id = componentId, tag = RenderContext::dialog, scope = scopeContext) { remove -> inlineStyle("display: contents") panel?.invoke(this)!!.apply { trapFocusInMountpoint(restoreFocus, setInitialFocus) } - opened.filter { !it }.map { }.take(1) handledBy close + opened.onCompletion { + /* + * This needs to be explained: + * As a `modal` is not dependent on any `Tag<*>`, we cannot provide some + * `PortalContainer.reference`-object. The latter is needed to couple the portal-portion of + * a component to its counterpart inside the normal `RenderContext` (or fritz2 controlled + * subtree if that is more understandable). From that reference we can get its nearest + * `MountPoint` and rely on the latter to register some `DomLifecycleHandler` by the + * `beforeUnmount`-lifecycle hook. In the case of a portal, we can call its `remove`-handler, + * which itself will change the global `PortalStack`, which will then reactively execute + * `renderEach` on all portals. So the portal-portion will get removed if its reference is + * reactively removed. This is always the case if the reference lives inside some reactive + * scope, which are created by any `render*`-call. (This is true for `Router`-based content + * too of course!) + * + * As we do not have such a reference here, we can only refer to the data-binding-flow, which + * will normally reside inside some reactive scope. So if the `Job` of this `Flow` is canceled + * due to some normal fritz2 reactive action, we know that the modal must also be removed from + * the DOM. + */ + remove(Unit) + }.filter { !it } + .map { } + .take(1) handledBy remove } } } diff --git a/headless/src/jsMain/kotlin/dev/fritz2/headless/foundation/portalling.kt b/headless/src/jsMain/kotlin/dev/fritz2/headless/foundation/portalling.kt index b2c919868..19dd37b3b 100644 --- a/headless/src/jsMain/kotlin/dev/fritz2/headless/foundation/portalling.kt +++ b/headless/src/jsMain/kotlin/dev/fritz2/headless/foundation/portalling.kt @@ -13,7 +13,6 @@ private val portalRootId by lazy { "portal-root".also { addGlobalStyle("#$it { d private object PortalStack : RootStore>>(emptyList(), job = Job()) { val add = handle> { stack, it -> stack + it } - val remove = handle { stack, id -> stack.filterNot { it.portalId == id } } } private data class PortalContainer( @@ -22,21 +21,29 @@ private data class PortalContainer( val scope: (ScopeContext.() -> Unit), val tag: TagFactory>, val reference: MountPoint?, - val content: Tag.(close: suspend (Unit) -> Unit) -> Unit + val content: Tag.(remove: suspend (Unit) -> Unit) -> Unit ) { - val portalId = Id.next() // used for renderEach only + /** + * used as ID-provider for the rendering of `PortalStack.data` + */ + val portalId = Id.next() val remove = PortalStack.handle { list -> list.filterNot { it.portalId == portalId } } fun render(ctx: RenderContext) = tag(ctx, classes, id, scope + { ctx.scope[MOUNT_POINT_KEY]?.let { set(MOUNT_POINT_KEY, it) } }) { + scope[SHOW_COMPONENT_STRUCTURE]?.let { + if (it) attr("data-portal-id", portalId) + } content.invoke(this) { remove.invoke() } reference?.beforeUnmount(this, null) { _, _ -> remove.invoke() } } } /** - * A [portalRoot] is needed to use floating components like [modal], [toast] and [popupPanel]. + * A [portalRoot] is needed to use floating components like [dev.fritz2.headless.components.modal], + * [dev.fritz2.headless.components.toast] and [dev.fritz2.headless.components.popOver]. + * Basically all components based upon [PopUpPanel]. * * Should be the last element in `document.body` to ensure it will not be clipped by other elements. * @@ -91,7 +98,11 @@ fun RenderContext.portal( ) { val portalId = id ?: Id.next() - // toasts and modals are rendered directly into the PortalRenderContext, they do not need a reference + /** + * toasts and modals are rendered directly into the PortalRenderContext, they do not need a reference. + * To be more precise: They do not have any valid reference, as for example the modal is rendered completely + * agnostic from the fritz2 controlled `RenderContext`. + */ val reference = if (this != PortalRenderContext) this else null PortalStack.add(