fix(tui): use keyboard-navigable buttons in update and confirm dialogs (#62906)

packages/tui/src/component/dialog-update.tsx

@@ -1,9 +1,8 @@
-import { createEffect, createMemo, createResource, createSignal, For, Match, onCleanup, Show, Switch } from "solid-js"
+import { createEffect, createMemo, createResource, createSignal, Match, onCleanup, Show, Switch } from "solid-js"
 import { Keymap } from "../context/keymap"
-import { useNewDesigns } from "../config"
 import { useTheme } from "../context/theme"
 import type { UpdateState } from "../context/update-notification"
-import { DialogHeader, DialogLayout, useDialog } from "../ui/dialog"
+import { DialogButtons, DialogHeader, DialogLayout, useDialog } from "../ui/dialog"
 import { errorMessage } from "../util/error"
 import { Spinner } from "./spinner"
 
@@ -16,7 +15,6 @@ export function DialogUpdate(props: {
 }) {
   const dialog = useDialog()
   const theme = useTheme().surface("dialog")
-  const newDesigns = useNewDesigns()
   const [error, setError] = createSignal<string>()
   const [active, setActive] = createSignal(0)
   const controller = new AbortController()
@@ -65,24 +63,19 @@ export function DialogUpdate(props: {
 
   createEffect(() => setActive(Math.max(0, buttons().length - 1)))
 
+  const move = (step: number) => {
+    const count = buttons().length
+    if (count) setActive((value) => (value + step + count) % count)
+  }
+
   Keymap.createLayer(() => ({
     mode: "modal",
     commands: [
-      {
-        bind: "return",
-        title: "Confirm update action",
-        group: "Dialog",
-        run: () => void buttons()[newDesigns() ? buttons().length - 1 : active()]?.run(),
-      },
-      ...(newDesigns() ? [] : ["left", "right", "tab", "shift+tab"]).map((bind) => ({
-        bind,
-        title: bind === "left" || bind === "shift+tab" ? "Previous update action" : "Next update action",
-        group: "Dialog",
-        run: () => {
-          const count = buttons().length
-          if (count) setActive((value) => (value + 1) % count)
-        },
-      })),
+      { bind: "return", title: "Confirm update action", group: "Dialog", run: () => void buttons()[active()]?.run() },
+      { bind: "left", title: "Previous update action", group: "Dialog", run: () => move(-1) },
+      { bind: "shift+tab", title: "Previous update action", group: "Dialog", run: () => move(-1) },
+      { bind: "right", title: "Next update action", group: "Dialog", run: () => move(1) },
+      { bind: "tab", title: "Next update action", group: "Dialog", run: () => move(1) },
     ],
   }))
 
@@ -99,33 +92,13 @@ export function DialogUpdate(props: {
           }
         />
       }
-      footer={{
-        actions: {
-          primary: buttons().map((button, index) => ({
-            title: button.label.toLowerCase(),
-            key: index === buttons().length - 1 ? "enter" : undefined,
-            onTrigger: () => void button.run(),
-          })),
-        },
-      }}
-      legacyFooter={
+      buttons={
         <Show when={buttons().length > 0}>
-          <box flexDirection="row" justifyContent="flex-end">
-            <For each={buttons()}>
-              {(button, index) => (
-                <box
-                  paddingLeft={1}
-                  paddingRight={1}
-                  backgroundColor={active() === index() ? theme.background.action.primary.focused : undefined}
-                  onMouseUp={() => void button.run()}
-                >
-                  <text fg={active() === index() ? theme.text.action.primary.focused : theme.text.muted}>
-                    {button.label}
-                  </text>
-                </box>
-              )}
-            </For>
-          </box>
+          <DialogButtons
+            buttons={buttons().map((button) => ({ label: button.label, run: () => void button.run() }))}
+            active={active()}
+            onActive={setActive}
+          />
         </Show>
       }
     >

packages/tui/src/ui/dialog-confirm.tsx

@@ -1,9 +1,8 @@
 import { Keymap } from "../context/keymap"
 import { useTheme } from "../context/theme"
 import { useNewDesigns } from "../config"
-import { DialogHeader, DialogLayout, useDialog } from "./dialog"
+import { DialogButtons, DialogHeader, DialogLayout, useDialog } from "./dialog"
 import { createStore } from "solid-js/store"
-import { For } from "solid-js"
 import { Locale } from "../util/locale"
 
 export type DialogConfirmProps = {
@@ -17,6 +16,8 @@ export type DialogConfirmProps = {
   }
 }
 
+const keys = ["cancel", "confirm"] as const
+
 export function DialogConfirm(props: DialogConfirmProps) {
   const dialog = useDialog()
   const theme = useTheme().surface("dialog")
@@ -31,71 +32,28 @@ export function DialogConfirm(props: DialogConfirmProps) {
     dialog.clear()
   }
 
+  const toggle = () => setStore("active", store.active === "confirm" ? "cancel" : "confirm")
+
   Keymap.createLayer(() => ({
     mode: "modal",
-    commands: newDesigns()
-      ? [
-          { bind: "return", title: "Confirm", group: "Dialog", run: () => choose("confirm") },
-          { bind: "escape", title: "Cancel", group: "Dialog", run: () => choose("cancel") },
-        ]
-      : [
-          {
-            bind: "return",
-            title: "Confirm dialog selection",
-            group: "Dialog",
-            run: () => choose(store.active),
-          },
-          {
-            bind: "left",
-            title: "Previous dialog option",
-            group: "Dialog",
-            run: () => {
-              setStore("active", store.active === "confirm" ? "cancel" : "confirm")
-            },
-          },
-          {
-            bind: "right",
-            title: "Next dialog option",
-            group: "Dialog",
-            run: () => {
-              setStore("active", store.active === "confirm" ? "cancel" : "confirm")
-            },
-          },
-        ],
+    commands: [
+      { bind: "return", title: "Confirm dialog selection", group: "Dialog", run: () => choose(store.active) },
+      { bind: "left", title: "Previous dialog option", group: "Dialog", run: toggle },
+      { bind: "right", title: "Next dialog option", group: "Dialog", run: toggle },
+      { bind: "tab", title: "Next dialog option", group: "Dialog", run: toggle },
+      { bind: "shift+tab", title: "Previous dialog option", group: "Dialog", run: toggle },
+      ...(newDesigns() ? [{ bind: "escape", title: "Cancel", group: "Dialog", run: () => choose("cancel") }] : []),
+    ],
   }))
   return (
     <DialogLayout
       header={<DialogHeader title={props.title} />}
-      footer={{
-        actions: {
-          primary: [
-            // Footer labels are lowercase; the legacy buttons title-case the same labels.
-            { title: (props.label?.cancel ?? "cancel").toLowerCase(), key: "esc", onTrigger: () => choose("cancel") },
-            {
-              title: (props.label?.confirm ?? "confirm").toLowerCase(),
-              key: "enter",
-              onTrigger: () => choose("confirm"),
-            },
-          ],
-        },
-      }}
-      legacyFooter={
-        <box flexDirection="row" justifyContent="flex-end">
-          <For each={["cancel", "confirm"] as const}>
-            {(key) => (
-              <box
-                paddingLeft={1}
-                paddingRight={1}
-                backgroundColor={key === store.active ? theme.background.action.primary.focused : undefined}
-                onMouseUp={() => choose(key)}
-              >
-                <text fg={key === store.active ? theme.text.action.primary.focused : theme.text.muted}>
-                  {Locale.titlecase(props.label?.[key] ?? key)}
-                </text>
-              </box>
-            )}
-          </For>
-        </box>
+      buttons={
+        <DialogButtons
+          buttons={keys.map((key) => ({ label: Locale.titlecase(props.label?.[key] ?? key), run: () => choose(key) }))}
+          active={keys.indexOf(store.active)}
+          onActive={(index) => setStore("active", keys[index]!)}
+        />
       }
     >
       <text fg={theme.text.muted}>{props.message}</text>

packages/tui/src/ui/dialog.tsx

@@ -111,11 +111,7 @@ export function DialogHeader(
         </text>
       )}
       {props.children}
-      <text
-        fg={theme.text.muted}
-        flexShrink={0}
-        onMouseUp={() => (props.onClose ?? dialog.clear)()}
-      >
+      <text fg={theme.text.muted} flexShrink={0} onMouseUp={() => (props.onClose ?? dialog.clear)()}>
         {props.closeLabel ?? "esc"}
       </text>
     </box>
@@ -145,6 +141,8 @@ export function DialogLayout(
     footer?: DialogFooterProps
     /** Footer shown while the "new-designs" experiment is off. Remove when the experiment graduates. */
     legacyFooter?: JSX.Element
+    /** A `DialogButtons` row, shown in place of the footer in both designs. */
+    buttons?: JSX.Element
     /** Side padding in cells. Defaults to DIALOG_INSET. */
     inset?: number
     /** Blank rows between the body and the footer under the "new-designs" experiment. Defaults to 1. */
@@ -157,19 +155,47 @@ export function DialogLayout(
     <box id={props.id} paddingLeft={inset()} paddingRight={inset()} paddingBottom={1} gap={1}>
       {props.header}
       <box paddingBottom={newDesigns() && props.footer ? (props.footerGap ?? 1) - 1 : 0}>{props.children}</box>
-      <Show when={newDesigns() || props.legacyFooter === undefined} fallback={props.legacyFooter}>
-        <Show when={props.footer}>
-          {(footer) => (
-            <DialogFooter actions={footer().actions} inset={footer().inset}>
-              {footer().children}
-            </DialogFooter>
-          )}
+      <Show when={props.buttons === undefined} fallback={props.buttons}>
+        <Show when={newDesigns() || props.legacyFooter === undefined} fallback={props.legacyFooter}>
+          <Show when={props.footer}>
+            {(footer) => (
+              <DialogFooter actions={footer().actions} inset={footer().inset}>
+                {footer().children}
+              </DialogFooter>
+            )}
+          </Show>
         </Show>
       </Show>
     </box>
   )
 }
 
+export type DialogButton = { label: string; run: () => void }
+
+/** A right-aligned row of buttons; the active one is highlighted. Arrow keys and Enter are bound by the dialog. */
+export function DialogButtons(props: { buttons: DialogButton[]; active: number; onActive?: (index: number) => void }) {
+  const theme = useTheme().surface("dialog")
+  return (
+    <box flexDirection="row" justifyContent="flex-end">
+      <For each={props.buttons}>
+        {(button, index) => (
+          <box
+            paddingLeft={1}
+            paddingRight={1}
+            backgroundColor={props.active === index() ? theme.background.action.primary.focused : undefined}
+            onMouseOver={() => props.onActive?.(index())}
+            onMouseUp={() => button.run()}
+          >
+            <text fg={props.active === index() ? theme.text.action.primary.focused : theme.text.muted}>
+              {button.label}
+            </text>
+          </box>
+        )}
+      </For>
+    </box>
+  )
+}
+
 /** Stretches a raised band inside a `DialogLayout` across the side padding; its content re-applies `DIALOG_INSET`. */
 export function DialogBleed(props: ParentProps) {
   return (

packages/tui/test/component/dialog-update.test.tsx

@@ -1,7 +1,7 @@
 /** @jsxImportSource @opentui/solid */
 import { testRender } from "@opentui/solid"
 import { expect, test } from "bun:test"
-import { createSignal } from "solid-js"
+import { createSignal, onCleanup } from "solid-js"
 import { DialogUpdate } from "../../src/component/dialog-update"
 import { ConfigProvider } from "../../src/config"
 import type { UpdateState } from "../../src/context/update-notification"
@@ -60,3 +60,52 @@ test("installation progress replaces checking while the update job is still pend
     app.renderer.destroy()
   }
 })
+
+test.each([false, true])("update choices are keyboard-navigable buttons with new designs %p", async (newDesigns) => {
+  await using temporary = await tmpdir()
+  const [state] = createSignal<UpdateState>({ type: "available", version: "2.0.0" } as UpdateState)
+  const ran: string[] = []
+  const app = await testRender(
+    () => (
+      <TestTuiContexts directory={temporary.path} paths={{ state: temporary.path }}>
+        <ConfigProvider config={createTuiResolvedConfig({ experimental: { "new-designs": newDesigns } })}>
+          <ThemeProvider mode="dark" source={emptyThemeSource}>
+            <Keymap.Provider>
+              <ToastProvider>
+                <DialogProvider>
+                  <ModalUpdate
+                    state={state}
+                    skip={() => ran.push("skip")}
+                    install={async () => void ran.push("install")}
+                    restart={() => {}}
+                  />
+                </DialogProvider>
+              </ToastProvider>
+            </Keymap.Provider>
+          </ThemeProvider>
+        </ConfigProvider>
+      </TestTuiContexts>
+    ),
+    { width: 80, height: 24, kittyKeyboard: true },
+  )
+
+  try {
+    app.renderer.start()
+    await app.waitForFrame((frame) => frame.includes("Update available") && frame.includes("Skip"))
+    const frame = app.captureCharFrame()
+    expect(frame).toMatch(/Skip\s+Update/)
+    expect(frame).not.toContain("enter")
+
+    app.mockInput.pressArrow("left")
+    app.mockInput.pressEnter()
+    await app.waitFor(() => ran.length > 0)
+    expect(ran).toEqual(["skip"])
+  } finally {
+    app.renderer.destroy()
+  }
+})
+
+function ModalUpdate(props: Parameters<typeof DialogUpdate>[0]) {
+  onCleanup(Keymap.use().mode.push("modal"))
+  return <DialogUpdate {...props} />
+}