Usability improvements for custom keyboard controls (#2145)

Co-authored-by: José Valim <jose.valim@gmail.com>

assets/js/hooks/keyboard_control.js

@@ -1,14 +1,20 @@
 import { getAttributeOrThrow, parseBoolean } from "../lib/attribute";
-import { cancelEvent, isEditableElement } from "../lib/utils";
+import { cancelEvent, isEditableElement, isMacOS } from "../lib/utils";
 
 /**
  * A hook for ControlComponent to handle user keyboard interactions.
  *
  * ## Configuration
  *
- *   * `data-keydown-enabled` - whether keydown events should be intercepted
+ *   * `data-cell-id` - id of the cell in which the control is rendered
  *
- *   * `data-keyup-enabled` - whether keyup events should be intercepted
+ *   * `data-default-handlers` - whether keyboard events should be
+ *     intercepted and canceled, disabling session shortcuts. Must be
+ *     one of "off", "on", or "disable_only"
+ *
+ *   * `data-keydown-enabled` - whether keydown events should be listened to
+ *
+ *   * `data-keyup-enabled` - whether keyup events should be listened to
  *
  *   * `data-target` - the target to send live events to
  */
@@ -42,6 +48,8 @@ const KeyboardControl = {
 
   getProps() {
     return {
+      cellId: getAttributeOrThrow(this.el, "data-cell-id"),
+      defaultHandlers: getAttributeOrThrow(this.el, "data-default-handlers"),
       isKeydownEnabled: getAttributeOrThrow(
         this.el,
         "data-keydown-enabled",
@@ -57,33 +65,58 @@ const KeyboardControl = {
   },
 
   handleDocumentKeyDown(event) {
-    if (this.keyboardEnabled()) {
+    if (
+      this.isKeyboardToggle(event) &&
+      !isEditableElement(document.activeElement)
+    ) {
       cancelEvent(event);
+      this.keyboardEnabled() ? this.disableKeyboard() : this.enableKeyboard();
+      return;
     }
 
-    if (this.props.isKeydownEnabled) {
+    if (this.keyboardEnabled()) {
+      if (this.props.defaultHandlers !== "on") {
+        cancelEvent(event);
+      }
+
       if (event.repeat) {
         return;
       }
 
-      const { key } = event;
-      this.pushEventTo(this.props.target, "keydown", { key });
+      if (this.props.isKeydownEnabled) {
+        const { key } = event;
+        this.pushEventTo(this.props.target, "keydown", { key });
+      }
     }
   },
 
   handleDocumentKeyUp(event) {
     if (this.keyboardEnabled()) {
-      cancelEvent(event);
-    }
+      if (this.props.defaultHandlers !== "on") {
+        cancelEvent(event);
+      }
 
-    if (this.props.isKeyupEnabled) {
-      const { key } = event;
-      this.pushEventTo(this.props.target, "keyup", { key });
+      if (this.props.isKeyupEnabled) {
+        const { key } = event;
+        this.pushEventTo(this.props.target, "keyup", { key });
+      }
     }
   },
 
   handleDocumentFocus(event) {
     if (this.props.isKeydownEnabled && isEditableElement(event.target)) {
+      this.disableKeyboard();
+    }
+  },
+
+  enableKeyboard() {
+    if (!this.keyboardEnabled()) {
+      this.pushEventTo(this.props.target, "enable_keyboard", {});
+    }
+  },
+
+  disableKeyboard() {
+    if (this.keyboardEnabled()) {
       this.pushEventTo(this.props.target, "disable_keyboard", {});
     }
   },
@@ -91,6 +124,32 @@ const KeyboardControl = {
   keyboardEnabled() {
     return this.props.isKeydownEnabled || this.props.isKeyupEnabled;
   },
+
+  isKeyboardToggle(event) {
+    if (event.repeat) {
+      return false;
+    }
+
+    const { metaKey, ctrlKey, key } = event;
+    const cmd = isMacOS() ? metaKey : ctrlKey;
+
+    if (cmd && key === "k" && this.isCellFocused()) {
+      return (
+        !this.keyboardEnabled() ||
+        ["on", "disable_only"].includes(this.props.defaultHandlers)
+      );
+    } else {
+      return false;
+    }
+  },
+
+  isCellFocused() {
+    const sessionEl = this.el.closest("[data-el-session]");
+    return (
+      sessionEl &&
+      sessionEl.getAttribute("data-js-focused-id") === this.props.cellId
+    );
+  },
 };
 
 export default KeyboardControl;

assets/js/hooks/session.js

@@ -1033,6 +1033,12 @@ const Session = {
   setFocusedEl(focusableId, { scroll = true, focusElement = true } = {}) {
     this.focusedId = focusableId;
 
+    if (focusableId) {
+      this.el.setAttribute("data-js-focused-id", focusableId);
+    } else {
+      this.el.removeAttribute("data-js-focused-id");
+    }
+
     if (focusableId) {
       // If the element is inside collapsed section, expand that section
       if (!this.isSection(focusableId)) {

lib/livebook_web/live/output.ex

@@ -253,14 +253,16 @@ defmodule LivebookWeb.Output do
          id: id,
          input_views: input_views,
          session_pid: session_pid,
-         client_id: client_id
+         client_id: client_id,
+         cell_id: cell_id
        }) do
     live_component(Output.ControlComponent,
       id: id,
       attrs: attrs,
       input_views: input_views,
       session_pid: session_pid,
-      client_id: client_id
+      client_id: client_id,
+      cell_id: cell_id
     )
   end
 

lib/livebook_web/live/output/control_component.ex

@@ -13,6 +13,8 @@ defmodule LivebookWeb.Output.ControlComponent do
       class="flex"
       id={"#{@id}-root"}
       phx-hook="KeyboardControl"
+      data-cell-id={@cell_id}
+      data-default-handlers={Map.get(@attrs, :default_handlers, :off)}
       data-keydown-enabled={to_string(@keyboard_enabled and :keydown in @attrs.events)}
       data-keyup-enabled={to_string(@keyboard_enabled and :keyup in @attrs.events)}
       data-target={@myself}
@@ -72,22 +74,19 @@ defmodule LivebookWeb.Output.ControlComponent do
 
   @impl true
   def handle_event("toggle_keyboard", %{}, socket) do
-    socket = update(socket, :keyboard_enabled, &not/1)
-    maybe_report_status(socket)
-    {:noreply, socket}
+    enabled = !socket.assigns.keyboard_enabled
+    maybe_report_status(socket, enabled)
+    {:noreply, assign(socket, keyboard_enabled: enabled)}
+  end
+
+  def handle_event("enable_keyboard", %{}, socket) do
+    maybe_report_status(socket, true)
+    {:noreply, assign(socket, keyboard_enabled: true)}
   end
 
   def handle_event("disable_keyboard", %{}, socket) do
-    socket =
-      if socket.assigns.keyboard_enabled do
-        socket = assign(socket, keyboard_enabled: false)
-        maybe_report_status(socket)
-        socket
-      else
-        socket
-      end
-
-    {:noreply, socket}
+    maybe_report_status(socket, false)
+    {:noreply, assign(socket, keyboard_enabled: false)}
   end
 
   def handle_event("button_click", %{}, socket) do
@@ -105,9 +104,11 @@ defmodule LivebookWeb.Output.ControlComponent do
     {:noreply, socket}
   end
 
-  defp maybe_report_status(socket) do
-    if :status in socket.assigns.attrs.events do
-      report_event(socket, %{type: :status, enabled: socket.assigns.keyboard_enabled})
+  defp maybe_report_status(socket, enabled) do
+    %{assigns: %{attrs: attrs, keyboard_enabled: current}} = socket
+
+    if :status in attrs.events and enabled != current do
+      report_event(socket, %{type: :status, enabled: enabled})
     end
   end
 

lib/livebook_web/live/session_live/shortcuts_component.ex

@@ -107,7 +107,13 @@ defmodule LivebookWeb.SessionLive.ShortcutsComponent do
       %{seq: ["s", "r"], desc: "Show runtime panel"},
       %{seq: ["s", "b"], desc: "Show bin"},
       %{seq: ["s", "p"], desc: "Show package search"},
-      %{seq: ["0", "0"], desc: "Reconnect current runtime"}
+      %{seq: ["0", "0"], desc: "Reconnect current runtime"},
+      %{
+        seq: ["ctrl", "k"],
+        seq_mac: ["⌘", "k"],
+        press_all: true,
+        desc: "Toggle keyboard control in cell output"
+      }
     ],
     universal: [
       %{