Ctrl+Z/Y/S work from every dock panel, not only the focused 3D view

docs/KeyboardShortcuts.md

@@ -0,0 +1,58 @@
+# Keyboard shortcuts
+
+## Ctrl shortcuts work everywhere
+
+| Keys | Action |
+|---|---|
+| **Ctrl+Z** | undo the last drawing action |
+| **Ctrl+Y** | redo it |
+| **Ctrl+S** | save the scene (only once it has a path; otherwise use *Save* in the menu) |
+
+On macOS, Cmd works in place of Ctrl.
+
+These keys work in whichever part of the window has keyboard focus: the 3D view, the
+instrument view, any dock panel (annotation properties, annotation tree, surfaces, config, …)
+or the top menu. A panel gets focus when you click into it; the 3D view takes it as soon as
+the cursor enters it. The one exception is a focused **text or number field**. There, Ctrl+Z / Ctrl+Y undo and redo your
+typing as usual, and Ctrl+S does nothing (not even the browser's "save page"). Click outside the
+field (or press Tab to leave it) to undo drawing actions again.
+
+Shift is not looked at: Ctrl+Shift+Z undoes and Ctrl+Shift+S saves. Holding a key down does
+not repeat the shortcut. Ctrl+Alt+Z and the like are ignored, and so is
+Alt+Z on its own: earlier versions treated Alt like Ctrl in the 3D view, so Alt+Z used to undo.
+Alt+click still picks like Ctrl+click.
+
+## Keys handled only by the 3D view
+
+Everything else (F1–F4 interaction modes, Enter / Backspace / Escape while drawing, 0–3
+semantics, C, PageUp / PageDown, MapView WASD, …) is still handled by the render control.
+These keys only work while the 3D view has keyboard focus, which it takes as soon as the cursor
+enters it. Ctrl+click picking also depends on that focus, because the 3D view tracks whether
+Ctrl is held. When the view loses focus, for example because you click a dock panel, it
+forgets any Ctrl or Shift it saw go down. A modifier released elsewhere therefore cannot stay
+"stuck" and leave picking armed.
+
+## Implementation
+
+| What | Where |
+|---|---|
+| `pro3dCtrlShortcut(event)`: the client-side filter (Ctrl/Cmd, no Alt, not repeated, key Z/Y/S, not in a text-entry field) and `preventDefault` (Ctrl+S also inside text fields) | [utilities.js](../src/PRo3D.Viewer/resources/utilities.js) |
+| `Events.onCtrlShortcut`: the body `onkeydown` that asks it and sends the key code | [Utilities.fs](../src/PRo3D.Viewer/Utilities.fs) |
+| `ViewerAction.CtrlShortcut` | [Viewer-Model.fs](../src/PRo3D.Viewer/Viewer-Model.fs) |
+| Update case: undo / redo / save | [Viewer.fs](../src/PRo3D.Viewer/Viewer/Viewer.fs) |
+| The handler on every page body | `Gui.Pages.pageRouting` in [ViewerGUI.fs](../src/PRo3D.Viewer/Viewer/ViewerGUI.fs) |
+| `releaseModifiersOnBlur` on both render controls | [Viewer.fs](../src/PRo3D.Viewer/Viewer/Viewer.fs) |
+
+Every dock panel is a separate document (`?page=annotations`, `?page=properties`, …). Key
+events do not cross document boundaries, so the handler sits on each page's `<body>`, where
+key events from anywhere in that page bubble up. The render control's own `onkeydown` bubbles
+to the render page's body too. That is why the `KeyDown` update no longer handles Z/Y/S:
+handling them in both places would undo twice.
+
+The shortcut reads `event.ctrlKey` / `event.metaKey` from the key event instead of the model's
+`ctrlFlag`. `ctrlFlag` is only maintained by key events that reach the render control, and it
+stays what decides whether Ctrl+click picks.
+
+Before this change, the Ctrl shortcuts lived in the `KeyDown` update and checked `ctrlFlag`.
+They did nothing over a dock panel, did nothing if Ctrl was pressed before the cursor entered
+the 3D view, and fired on a plain Z/S if Ctrl had been released outside the view.

src/PRo3D.Viewer/Utilities.fs

@@ -79,4 +79,23 @@ module Events =
                             Seq.empty
                 )
         )
-
+
+    /// Ctrl (Cmd on macOS) + Z / Y / S, for a page's body: key events bubble up from
+    /// anywhere in the page, so the shortcut no longer needs the render control to have
+    /// focus. Which keys are sent, and what text fields keep, is decided client side by
+    /// pro3dCtrlShortcut in resources/utilities.js (docs/KeyboardShortcuts.md).
+    let onCtrlShortcut (cb : Keys -> 'msg) =
+        "onkeydown",
+        AttributeValue.Event {
+            clientSide = fun send id ->
+                "if (window.pro3dCtrlShortcut && pro3dCtrlShortcut(event)) { " + send id ["event.keyCode"] + " }"
+            serverSide = fun _ _ args ->
+                match args with
+                | keyCode :: _ ->
+                    match System.Int32.TryParse keyCode with
+                    | true, code -> Seq.delay (fun () -> Seq.singleton (cb (KeyConverter.keyFromVirtualKey code)))
+                    | _ -> Seq.empty
+                | _ ->
+                    Seq.empty
+        }
+

src/PRo3D.Viewer/Viewer-Model.fs

@@ -193,7 +193,8 @@ type ViewerAction =
 
 | NewScene
 | KeyDown                         of key : Aardvark.Application.Keys
-| KeyUp                           of key : Aardvark.Application.Keys      
+| KeyUp                           of key : Aardvark.Application.Keys
+| CtrlShortcut                    of key : Aardvark.Application.Keys // Ctrl+Z/Y/S from any page, see docs/KeyboardShortcuts.md
 | ResizeMainControl               of V2i * string
 | ResizeInstrumentControl         of V2i * string
 | SetKind                         of TrafoKind

src/PRo3D.Viewer/Viewer/Viewer.fs

@@ -2012,28 +2012,7 @@ module ViewerApp =
                 | Keyboard.Modifier -> { m with ctrlFlag = true }
                 | _ -> m
 
-            let m =
-                match (m.ctrlFlag, k, m.scene.scenePath) with
-                | true, Aardvark.Application.Keys.S, Some path ->
-                    { (saveScene path m) with ctrlFlag = false } |> shortFeedback "scene saved"
-                | true, Aardvark.Application.Keys.S, None ->
-                    { m with ctrlFlag = false } |> shortFeedback "please use \"save\" in the menu to save the scene"
-                    // (saveSceneAndAnnotations p m)
-                |_-> m
-
-            let m =
-                let view =
-                    match m.viewerMode with
-                    | ViewerMode.Standard    -> m.navigation.camera.view
-                    | ViewerMode.Instrument  -> m.scene.viewPlans.instrumentCam
-                match (m.ctrlFlag, k) with
-                | true, Aardvark.Application.Keys.Z ->
-                    let drawing = DrawingApp.update m.scene.referenceSystem drawingConfig None sendQueue view m.shiftFlag m.drawing DrawingAction.Undo
-                    { m with drawing = drawing }
-                | true, Aardvark.Application.Keys.Y ->
-                    let drawing = DrawingApp.update m.scene.referenceSystem drawingConfig None sendQueue view m.shiftFlag m.drawing DrawingAction.Redo
-                    { m with drawing = drawing }
-                | _ -> m
+            // Ctrl+Z/Y/S are not handled here but in CtrlShortcut, sent by every page's body
 
             // comparison areas: resize the area just placed, Enter to finish
             let m =
@@ -2111,6 +2090,24 @@ module ViewerApp =
             match k with
             | Keyboard.Modifier -> { m with ctrlFlag = false }
             | _ -> m
+        | CtrlShortcut k,_ ->
+            let view =
+                match m.viewerMode with
+                | ViewerMode.Standard    -> m.navigation.camera.view
+                | ViewerMode.Instrument  -> m.scene.viewPlans.instrumentCam
+            match k, m.scene.scenePath with
+            | Aardvark.Application.Keys.Z, _ ->
+                let drawing = DrawingApp.update m.scene.referenceSystem drawingConfig None sendQueue view m.shiftFlag m.drawing DrawingAction.Undo
+                { m with drawing = drawing }
+            | Aardvark.Application.Keys.Y, _ ->
+                let drawing = DrawingApp.update m.scene.referenceSystem drawingConfig None sendQueue view m.shiftFlag m.drawing DrawingAction.Redo
+                { m with drawing = drawing }
+            // ctrlFlag is left alone: Ctrl may still be held, and blur releases it otherwise
+            | Aardvark.Application.Keys.S, Some path ->
+                saveScene path m |> shortFeedback "scene saved"
+            | Aardvark.Application.Keys.S, None ->
+                m |> shortFeedback "please use \"save\" in the menu to save the scene"
+            | _ -> m
         | SetInteraction t,_ ->
 
             // let feedback = sprintf "pick refrence plane; confirm with ENTER" t |> UserFeedback.create 3000
@@ -2692,8 +2689,14 @@ module ViewerApp =
         }
         |> Sg.noEvents
         |> Sg.trafo(trafo)
-
-    let renderControlAttributes (id : string) (m: AdaptiveModel) = 
+
+    /// Ctrl/Shift held while focus leaves a render control are released elsewhere; that
+    /// KeyUp never arrives here, and a stale ctrlFlag/shiftFlag would keep picking armed.
+    let private releaseModifiersOnBlur : Attribute<ViewerAction> =
+        onEvent' "onblur" [] (fun _ ->
+            [KeyUp Aardvark.Application.Keys.LeftCtrl; KeyUp Aardvark.Application.Keys.LeftShift])
+
+    let renderControlAttributes (id : string) (m: AdaptiveModel) =
 
         let renderControlAtts (model: AdaptiveNavigationModel) =
             amap {
@@ -2745,7 +2748,8 @@ module ViewerApp =
                     let drawingAction = getDrawingActionForKey (m.interaction |> AVal.force) k
                     [KeyDown k; DrawingMessage drawingAction]
                 )
-                onKeyUp   (KeyUp)        
+                onKeyUp   (KeyUp)
+                releaseModifiersOnBlur
                 clazz "mainrendercontrol"
                 onEvent "resizeControl"  [] (
                     fun p -> 
@@ -2771,6 +2775,7 @@ module ViewerApp =
                 attribute "useMapping" "true"
                 onKeyDown (KeyDown)
                 onKeyUp (KeyUp)
+                releaseModifiersOnBlur
                 clazz "instrumentrendercontrol"
                 onEvent "resizeControl"  [] (
                     fun p -> 

src/PRo3D.Viewer/Viewer/ViewerGUI.fs

@@ -2036,7 +2036,12 @@ module Gui =
     module Pages =
         let mutable renderViewportSizeId = System.Guid.NewGuid().ToString()
         let pageRouting viewerDependencies bodyAttributes (m : AdaptiveModel) viewInstrumentView viewRenderView (runtime : IRuntime) (request : IHttpRequest) =
-
+            // Every dock panel is its own document: each page body carries the Ctrl+Z/Y/S
+            // handler so the shortcuts work wherever the cursor is (docs/KeyboardShortcuts.md).
+            let ctrlShortcut : Attribute<ViewerAnimationAction> =
+                Events.onCtrlShortcut CtrlShortcut |> ViewerUtils.mapAttribute ViewerMessage
+            let bodyAttributes = ctrlShortcut :: bodyAttributes
+
             match request.QueryParam "page" with
             | Some "instrumentview" ->
                 let id = System.Guid.NewGuid().ToString()
@@ -2060,7 +2065,7 @@ module Gui =
                       |> AttributeMap.mapAttributes (AttributeValue.map ViewerMessage)
 
                 require (viewerDependencies) (
-                    body [ style "background: #1B1C1E; width:100%; height:100%; overflow-y:auto; overflow-x:auto;"] [
+                    body [ ctrlShortcut; style "background: #1B1C1E; width:100%; height:100%; overflow-y:auto; overflow-x:auto;"] [
                       Incremental.div instrumentViewAttributes (
                         alist {
                             yield viewInstrumentView runtime id m 
@@ -2072,14 +2077,14 @@ module Gui =
             | Some "mapprojection" ->
                 // map projection panel (#772); gates itself to small bodies
                 require (viewerDependencies) (
-                    body [ style "background: #1B1C1E; width:100%; height:100%; overflow:hidden" ] [
+                    body [ ctrlShortcut; style "background: #1B1C1E; width:100%; height:100%; overflow:hidden" ] [
                         MapProjectionHost.view m |> UI.map ViewerMessage
                     ]
                 )
             | Some "imageinspector" ->
                 // selected projected image in 2D, hover-linked to 3D (docs/dev/shadowEstimation.md)
                 require (viewerDependencies) (
-                    body [ style "background: #1B1C1E; width:100%; height:100%; overflow:hidden" ] [
+                    body [ ctrlShortcut; style "background: #1B1C1E; width:100%; height:100%; overflow:hidden" ] [
                         ImageInspector.view m |> UI.map ViewerMessage
                     ]
                 )
@@ -2095,8 +2100,9 @@ module Gui =
                         onEvent "onfocus" ["{ X: $(document).width(), Y: $(document).height()  }"] (List.head >> Pickler.json.UnPickleOfString >> cb)
 
                     let renderViewAttributes : list<Attribute<ViewerAnimationAction>> = 
-                        [ 
+                        [
                         style "background: #1B1C1E; height:100%; width:100%"
+                        Events.onCtrlShortcut CtrlShortcut
                         Events.onClick (fun _ -> SwitchViewerMode ViewerMode.Standard)
                         onResize (fun s -> OnResize(s, renderViewportSizeId))
                         onFocus (fun s -> OnResize(s, renderViewportSizeId))
@@ -2302,7 +2308,7 @@ module Gui =
             | None ->
                 require (viewerDependencies) (
                     onBoot (sprintf "document.title = '%s'" Config.title) (
-                        body [] [
+                        body [ ctrlShortcut ] [
                             TopMenu.getTopMenu m
                             |> UI.map ViewerMessage
                             div [clazz "dockingMainDings"] [

src/PRo3D.Viewer/resources/utilities.js

@@ -89,3 +89,27 @@ function startBusyIndicator(id, thresholdMs) {
 			.then(function () { inFlight = false; });
 	}, 200);
 }
+
+// --- Ctrl shortcuts ----------------------------------------------------------------
+//
+// Every page body has an onkeydown that asks this whether to send a Ctrl+Z/Y/S to the
+// server (Events.onCtrlShortcut, docs/KeyboardShortcuts.md). A text field's own undo is
+// the browser's default action, not a listener that could stop the event, so the target
+// decides: typing keeps Z/Y, and S never reaches the browser's "save page".
+
+function pro3dIsTextEntry(t) {
+	return !!t && (t.isContentEditable || t.tagName === 'TEXTAREA' ||
+		(t.tagName === 'INPUT' && /^(text|number|search|email|url|tel|password)$/.test(t.type)));
+}
+
+function pro3dCtrlShortcut(event) {
+	var k = event.keyCode;                                       // 90 Z, 89 Y, 83 S
+	if (event.repeat || !(event.ctrlKey || event.metaKey) || event.altKey) { return false; }
+	if (k !== 90 && k !== 89 && k !== 83) { return false; }
+	if (pro3dIsTextEntry(event.target)) {
+		if (k === 83) { event.preventDefault(); }
+		return false;
+	}
+	event.preventDefault();
+	return true;
+}

src/Tests/Features/Section18_KeyboardShortcuts.fs

@@ -17,7 +17,8 @@ open Aardvark.Rendering                  // IRuntime, IFramebufferSignature
 open Expecto
 
 open PRo3D
-open PRo3D.Core                          // Interactions
+open PRo3D.Core                          // Interactions, GroupsModel
+open PRo3D.Core.Drawing                  // DrawingApp, AnnotationsDelta
 open PRo3D.Viewer
 open PRo3D.Tests
 
@@ -80,4 +81,58 @@ let tests =
             Expect.equal m.interaction Interactions.PickSurface
                 "selecting an action in the menu sets the same interaction the shortcuts do"
         }
+
+        // Ctrl+Z/Y arrive as CtrlShortcut from every page body (docs/KeyboardShortcuts.md)
+
+        test "TC-18.1 CtrlShortcut Z undoes and Y redoes, with no ctrlFlag" {
+            let model, update = Head.make ()
+            let delta = AnnotationsDelta.SnapshotDelta(GroupsModel.initial, GroupsModel.initial)
+            let m = { model with drawing = DrawingApp.pushUndo delta model.drawing }
+            Expect.isFalse m.ctrlFlag "the shortcut must not depend on a tracked Ctrl-down"
+            let undone = update m (ViewerAction.CtrlShortcut Keys.Z)
+            Expect.isEmpty undone.drawing.undoStack "Ctrl+Z pops the undo stack"
+            Expect.equal (List.length undone.drawing.redoStack) 1 "... onto the redo stack"
+            let redone = update undone (ViewerAction.CtrlShortcut Keys.Y)
+            Expect.equal (List.length redone.drawing.undoStack) 1 "Ctrl+Y pushes it back"
+            Expect.isEmpty redone.drawing.redoStack "... and empties the redo stack"
+        }
+
+        test "TC-18.1 the render control's KeyDown no longer undoes (would undo twice)" {
+            // the render control's keydown also bubbles to its page body, which sends CtrlShortcut
+            let model, update = Head.make ()
+            let delta = AnnotationsDelta.SnapshotDelta(GroupsModel.initial, GroupsModel.initial)
+            let m = { model with drawing = DrawingApp.pushUndo delta model.drawing }
+            let m = [ Keys.LeftCtrl; Keys.Z ] |> List.fold (fun m k -> update m (ViewerAction.KeyDown k)) m
+            Expect.isTrue m.ctrlFlag "Ctrl is still tracked for picking"
+            Expect.equal (List.length m.drawing.undoStack) 1 "KeyDown Z leaves the undo stack alone"
+        }
+
+        test "TC-18.1 CtrlShortcut S leaves a held Ctrl armed" {
+            let model, update = Head.make ()
+            let m = update model (ViewerAction.KeyDown Keys.LeftCtrl)
+            let m = update m (ViewerAction.CtrlShortcut Keys.S)   // no scene path: feedback only
+            Expect.isTrue m.ctrlFlag "Ctrl+S must not drop the Ctrl the user still holds"
+        }
+
+        test "TC-18.1 releasing Ctrl and Shift clears both flags (render control blur)" {
+            let model, update = Head.make ()
+            let m = [ Keys.LeftCtrl; Keys.LeftShift ] |> List.fold (fun m k -> update m (ViewerAction.KeyDown k)) model
+            Expect.isTrue (m.ctrlFlag && m.shiftFlag) "both held"
+            let m = [ Keys.LeftCtrl; Keys.LeftShift ] |> List.fold (fun m k -> update m (ViewerAction.KeyUp k)) m
+            Expect.isFalse (m.ctrlFlag || m.shiftFlag) "both released"
+        }
+
+        test "TC-18.1 onCtrlShortcut: calls the client filter, decodes the key" {
+            match Aardvark.UI.Events.onCtrlShortcut id with
+            | "onkeydown", Aardvark.UI.AttributeValue.Event e ->
+                let js = e.clientSide (fun _ args -> sprintf "SEND(%s)" (String.concat "," args)) "id"
+                Expect.stringContains js "pro3dCtrlShortcut(event)" "the filter lives in resources/utilities.js"
+                Expect.stringContains js "SEND(event.keyCode)" "sends the key code once the filter agrees"
+                let decode code = e.serverSide System.Guid.Empty "id" [ code ] |> Seq.toList
+                Expect.equal (decode "90") [ Keys.Z ] "90 is Z"
+                Expect.equal (decode "89") [ Keys.Y ] "89 is Y"
+                Expect.equal (decode "83") [ Keys.S ] "83 is S"
+                Expect.isEmpty (decode "garbage") "a non-numeric key code is dropped, not thrown"
+            | name, _ -> failtestf "expected an onkeydown event attribute, got %s" name
+        }
     ]