fix(ux): delete system key handler could cause confirmation dialog to…

… be created each time the key is pressed
updraft0 · Jun 15, 2024 · a037524 · a037524
1 parent 9b6c5f5
commit a037524
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 21 deletions.
diff --git a/ui/src/main/scala/controltower/component/Modal.scala b/ui/src/main/scala/controltower/component/Modal.scala
@@ -44,6 +44,7 @@ object Modal:
 
   def show(
       content: (Observer[Unit], Owner) => Element,
+      onCloseObs: Observer[Unit],
       clickCloses: Boolean,
       mods: Mod[ReactiveHtmlElement[dom.HTMLDialogElement]]*
   ): Unit =
@@ -59,6 +60,7 @@ object Modal:
     val detached = renderDetached(dialog, activateNow = true)
     dialog.amend(
       onClose --> { _ =>
+        onCloseObs.onNext(())
         owner.killSubscriptions()
         detached.deactivate()
         dom.document.body.removeChild(dialog.ref)
@@ -69,7 +71,13 @@ object Modal:
     )
     dialog.ref.showModal()
 
-  def showConfirmation(title: HtmlMod, description: HtmlMod, onOk: Observer[Unit], isDestructive: Boolean = false) =
+  def showConfirmation(
+      title: HtmlMod,
+      description: HtmlMod,
+      onOk: Observer[Unit],
+      isDestructive: Boolean = false,
+      onClose: Observer[Unit] = Observer.empty
+  ) =
     show(
       (closeMe, _) =>
         div(
@@ -101,6 +109,7 @@ object Modal:
             )
           )
         ),
+      onClose,
       clickCloses = false,
       cls := "confirm-dialog"
     )

diff --git a/ui/src/main/scala/controltower/page/landing.scala b/ui/src/main/scala/controltower/page/landing.scala
@@ -107,6 +107,7 @@ object LandingPage:
       onClick.preventDefault --> { _ =>
         Modal.show(
           (closeMe, _) => EditMapView(char, closeMe, None)(using ct),
+          onCloseObs = Observer.empty[Unit],
           clickCloses = false,
           idAttr := "edit-map-dialog"
         )

diff --git a/ui/src/main/scala/controltower/page/map/view/MapView.scala b/ui/src/main/scala/controltower/page/map/view/MapView.scala
@@ -20,7 +20,7 @@ import scala.language.implicitConversions
 // TODO: duplication
 given equalEventTarget[El <: org.scalajs.dom.Element]: CanEqual[org.scalajs.dom.EventTarget, El] = CanEqual.derived
 
-private class MapView(
+private final class MapView(
     viewId: Int,
     page: Page.Map,
     ct: ControlTowerBackend,
@@ -30,6 +30,8 @@ private class MapView(
 ) extends ViewController:
 
   private val mapTop                    = Var[Option[Element]](None)
+  private val inKeyHandler              = Var(false)
+  private val notInKeyHandler           = inKeyHandler.signal.map(!_)
   private var controller: MapController = _
 
   override def view: Element = div(
@@ -158,15 +160,12 @@ private class MapView(
 
     div(
       idAttr := "map-view-inner",
-
       // delete -> remove system
-
-      // TODO: make helpers for these
-      documentEvents(
-        _.onKeyDown
-          .filter(ev => !ev.repeat && ev.code == "Delete")
-      ).mapToUnit.compose(_.withCurrentValueOf(controller.selectedSystem)).filterNot(_.isEmpty).map(_.get) --> (
-        system => removeSystemConfirm(system, controller.actionsBus)(using rds)
+      modalKeyBinding(
+        "Delete",
+        ws.isConnected,
+        _.withCurrentValueOf(controller.selectedSystem).filterNot(_.isEmpty).map(_.get),
+        (system, onClose) => removeSystemConfirm(system, controller.actionsBus, onClose)(using rds)
       ),
       div(
         idAttr := "map-parent",
@@ -202,6 +201,18 @@ private class MapView(
       )
     )
 
+  private def modalKeyBinding[B](
+      code: String,
+      isConnected: Signal[Boolean],
+      compose: EventStream[Unit] => EventStream[B],
+      action: (B, Observer[Unit]) => Unit
+  ) =
+    documentEvents(_.onKeyDown.filter(ev => !ev.repeat && ev.code == code)).mapToUnit
+      .compose(es => compose(es).filterWith(isConnected).filterWith(notInKeyHandler.signal)) --> { b =>
+      inKeyHandler.set(true)
+      action(b, Observer(_ => inKeyHandler.set(false)))
+    }
+
 object MapView:
   import org.updraft0.controltower.protocol.jsoncodec.given
   import sttp.client3.UriContext

diff --git a/ui/src/main/scala/controltower/page/map/view/NavTopView.scala b/ui/src/main/scala/controltower/page/map/view/NavTopView.scala
@@ -51,7 +51,8 @@ class NavTopView(
             case scala.util.Success(Right(mapInfoWithPermissions)) =>
               Modal.show(
                 (closeMe, _) => EditMapView(meta.character, closeMe, Some(mapInfoWithPermissions)),
-                clickCloses = false,
+                Observer.empty[Unit],
+                false,
                 idAttr := "edit-map-dialog"
               )
             case other => org.scalajs.dom.console.error(s"Failed to get map permissions: $other")

diff --git a/ui/src/main/scala/controltower/page/map/view/SystemSignatureView.scala b/ui/src/main/scala/controltower/page/map/view/SystemSignatureView.scala
@@ -129,7 +129,8 @@ private inline def sigView(
                     time,
                     actions
                   ),
-                  clickCloses = false,
+                  Observer.empty[Unit],
+                  false,
                   cls := "system-paste-signatures"
                 )
               )
@@ -150,7 +151,8 @@ private inline def sigView(
                     static.wormholeTypes,
                     actions
                   ),
-                  clickCloses = true,
+                  Observer.empty[Unit],
+                  true,
                   cls := "system-add-signature"
                 )
               )
@@ -178,7 +180,8 @@ private inline def sigView(
                     static.wormholeTypes,
                     actions
                   ),
-                  clickCloses = true,
+                  Observer.empty[Unit],
+                  true,
                   cls := "system-add-signature"
                 )
               )

diff --git a/ui/src/main/scala/controltower/page/map/view/SystemView.scala b/ui/src/main/scala/controltower/page/map/view/SystemView.scala
@@ -128,7 +128,8 @@ class SystemView(
               Observer({ case (_, mss: MapSystemSnapshot) =>
                 Modal.show(
                   (closeMe, owner) => systemRenameView(systemId, mss.system.name.getOrElse(""), ctx.actions, closeMe),
-                  clickCloses = true,
+                  Observer.empty[Unit],
+                  true,
                   cls := "system-rename-dialog"
                 )
               }),

diff --git a/ui/src/main/scala/controltower/page/map/view/ToolbarView.scala b/ui/src/main/scala/controltower/page/map/view/ToolbarView.scala
@@ -33,7 +33,8 @@ class ToolbarView(
         onClick.stopPropagation --> (_ =>
           Modal.show(
             (closeMe, owner) => systemAddView(actions, closeMe, rds, positionController)(using owner),
-            clickCloses = true,
+            Observer.empty[Unit],
+            true,
             cls := "system-add-dialog"
           )
         )
@@ -43,7 +44,7 @@ class ToolbarView(
         "ti-trash",
         disableWhenNotSelectedAndRole(selected, mapRole, RoleController.canRemoveSystem, isConnected),
         onClick.stopPropagation.compose(_.sampleCollectSome(selected)) --> (system =>
-          removeSystemConfirm(system, actions)(using rds)
+          removeSystemConfirm(system, actions, Observer.empty)(using rds)
         )
       ),
       toolbarButtonS(
@@ -224,8 +225,8 @@ private def systemAddView(
     )
   )
 
-private[map] def removeSystemConfirm(system: MapSystemSnapshot, actions: WriteBus[MapAction])(using
-    rds: ReferenceDataStore
+private[map] def removeSystemConfirm(system: MapSystemSnapshot, actions: WriteBus[MapAction], onClose: Observer[Unit])(
+    using rds: ReferenceDataStore
 ) =
   Modal.showConfirmation(
     "Confirm removal",
@@ -239,5 +240,6 @@ private[map] def removeSystemConfirm(system: MapSystemSnapshot, actions: WriteBu
           )
     ),
     actions.contramap(_ => MapAction.Remove(system.system.systemId)),
-    isDestructive = true
+    isDestructive = true,
+    onClose = onClose
   )
diff --git a/ui/src/main/scala/controltower/page/mapeditor.scala b/ui/src/main/scala/controltower/page/mapeditor.scala
@@ -45,7 +45,8 @@ object MapEditorPage:
             case scala.util.Success(Right(mapInfoWithPermissions)) =>
               Modal.show(
                 (closeMe, _) => EditMapView(char, closeMe, Some(mapInfoWithPermissions)),
-                clickCloses = false,
+                Observer.empty[Unit],
+                false,
                 idAttr := "edit-map-dialog"
               )
             case other => org.scalajs.dom.console.error(s"Failed to get map permissions: $other")