From d57392b5af5f849848db8ca4993e9018b4ec019d Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 19:25:02 +0300 Subject: [PATCH] fix(dictation): extract tested computeDictationAvailability + gate mic from the reactive atom (#314 F1/F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: the availability publish-effect duplicated the #218 editability gate (editable && inEditMode && !showStatic) inline — a copy that could silently diverge from the tested isBodyEditable — and the reason computation (the core of #309) had no tests. Extract computeDictationAvailability into editor-sync-state.ts REUSING isBodyEditable; the effect is now a one-line call. Unit tests cover the branches (synced→null; pre-sync disconnected→offline / else connecting; !editable/!edit→read-only). F2: DictationGroup gated the mic on the non-reactive editor.isEditable while the PR already publishes the reactive dictationAvailability.isEditable (same signals) — so gate and reason came from different sources and the mic could stick. Gate on dictationAvailability.isEditable: one reactive source of truth for both. vitest (editor-sync-state + dictation): 37 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../groups/dictation-group.test.tsx | 79 ++++++++----------- .../fixed-toolbar/groups/dictation-group.tsx | 2 +- .../features/editor/editor-sync-state.test.ts | 54 ++++++++++++- .../src/features/editor/editor-sync-state.ts | 30 +++++++ .../src/features/editor/page-editor.tsx | 26 +++--- 5 files changed, 125 insertions(+), 66 deletions(-) diff --git a/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.test.tsx b/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.test.tsx index 6aaf6983..28bf9be4 100644 --- a/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.test.tsx +++ b/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.test.tsx @@ -1,36 +1,15 @@ import { describe, it, expect, vi } from "vitest"; -import { useEffect, useState } from "react"; import { render, act } from "@testing-library/react"; +import { Provider, createStore } from "jotai"; +import { dictationAvailabilityAtom } from "@/features/editor/atoms/editor-atoms.ts"; -// Regression test for #311: on a page the user can edit, the byline mic stayed -// stuck disabled until an unrelated re-render happened, because DictationGroup -// read the non-reactive field `editor.isEditable` directly. The fix reads it via -// `useEditorState`, which subscribes to the editor's own events. -// -// The mock below mirrors the real `useEditorState` contract: it runs the -// selector, and re-runs it (re-rendering the consumer) whenever the editor emits -// an event. This is what makes the test faithful — with the pre-fix code -// (`disabled={!editor.isEditable}`) DictationGroup never subscribes, so emitting -// an event would NOT re-render and the mic would stay disabled. -vi.mock("@tiptap/react", () => ({ - useEditorState: ({ editor, selector }: any) => { - const [value, setValue] = useState(() => selector({ editor })); - useEffect(() => { - const handler = () => setValue(selector({ editor })); - editor.on("update", handler); - return () => editor.off("update", handler); - }, [editor, selector]); - return value; - }, -})); - -// The mic only cares about the workspace's streaming flag; return a stable stub. -vi.mock("jotai", () => ({ - useAtomValue: () => ({ settings: { ai: { dictationStreaming: false } } }), -})); -vi.mock("@/features/user/atoms/current-user-atom.ts", () => ({ - workspaceAtom: {}, -})); +// Regression test for the byline mic staying stuck disabled (#311 / #309): on a +// page the user can edit, the mic must un-grey once the body becomes editable. +// #311 first fixed this by reading `editor.isEditable` via `useEditorState`; #309 +// superseded that with a reactive `dictationAvailabilityAtom` that page-editor +// publishes (carrying both the editable gate AND the unavailable reason). The mic +// now gates on `dictationAvailability.isEditable`, so a change to that atom must +// re-render the group and flip the disabled state (jotai drives the subscription). // Detectable stand-in that surfaces the `disabled` prop the component computes. vi.mock("@/features/dictation/components/mic-button", () => ({ @@ -41,33 +20,39 @@ vi.mock("@/features/dictation/components/mic-button", () => ({ import { DictationGroup } from "./dictation-group"; -// Minimal editor stand-in: a mutable `isEditable` field plus a tiny event -// emitter, matching the surface DictationGroup + the mocked useEditorState use. -function makeFakeEditor(isEditable: boolean) { - const listeners = new Set<() => void>(); +// Minimal editor stand-in matching the surface DictationGroup uses (handleStart / +// handleText). The disabled gate no longer reads this — it reads the atom. +function makeFakeEditor() { return { - isEditable, + isEditable: false, isDestroyed: false, state: { selection: { from: 0, to: 0 }, doc: { content: { size: 0 } } }, - on: (_event: string, cb: () => void) => listeners.add(cb), - off: (_event: string, cb: () => void) => listeners.delete(cb), - emit: () => listeners.forEach((cb) => cb()), } as any; } -describe("DictationGroup editable reactivity (#311)", () => { - it("re-enables the mic when the editor flips isEditable false -> true", () => { - const editor = makeFakeEditor(false); - const { getByTestId } = render(); +describe("DictationGroup editable reactivity (#309 atom / #311)", () => { + it("re-enables the mic when dictationAvailability flips isEditable false -> true", () => { + const editor = makeFakeEditor(); + const store = createStore(); + // Pre-sync: page editor publishes not-editable (with a reason). + store.set(dictationAvailabilityAtom, { + isEditable: false, + reason: "connecting", + }); - // Pre-sync: not editable yet, so the mic is disabled (preserves #218 intent). + const { getByTestId } = render( + + + , + ); + + // Not editable yet -> disabled (preserves the #218 pre-sync intent). expect(getByTestId("mic").hasAttribute("disabled")).toBe(true); - // Collab sync flips the editor editable via editor.setEditable(true), which - // mutates the field and emits — the mic must react and enable itself. + // Collab sync -> page editor republishes editable; the atom change must + // re-render the group and enable the mic. act(() => { - editor.isEditable = true; - editor.emit(); + store.set(dictationAvailabilityAtom, { isEditable: true, reason: null }); }); expect(getByTestId("mic").hasAttribute("disabled")).toBe(false); diff --git a/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.tsx b/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.tsx index 2c55d5cd..82222a18 100644 --- a/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.tsx +++ b/apps/client/src/features/editor/components/fixed-toolbar/groups/dictation-group.tsx @@ -83,7 +83,7 @@ export const DictationGroup: FC = ({ editor, color, iconSize }) => { streaming={streamingDictation} onStart={handleStart} onText={handleText} - disabled={!editor.isEditable} + disabled={!dictationAvailability.isEditable} unavailableReason={dictationAvailability.reason ?? undefined} color={color} iconSize={iconSize} diff --git a/apps/client/src/features/editor/editor-sync-state.test.ts b/apps/client/src/features/editor/editor-sync-state.test.ts index 7d31c292..e21a8585 100644 --- a/apps/client/src/features/editor/editor-sync-state.test.ts +++ b/apps/client/src/features/editor/editor-sync-state.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect } from "vitest"; import { WebSocketStatus } from "@hocuspocus/provider"; -import { isCollabSynced, isBodyEditable } from "./editor-sync-state"; +import { + isCollabSynced, + isBodyEditable, + computeDictationAvailability, +} from "./editor-sync-state"; describe("isCollabSynced", () => { it("is true only when Connected and synced", () => { @@ -30,3 +34,51 @@ describe("isBodyEditable (pre-sync data-loss gate, #218)", () => { expect(isBodyEditable({ ...base, inEditMode: false })).toBe(false); }); }); + +describe("computeDictationAvailability (mic reason precedence, #309)", () => { + const base = { + editable: true, + inEditMode: true, + showStatic: false, + isDisconnected: false, + }; + + it("is available with no reason once synced (showStatic false)", () => { + expect(computeDictationAvailability(base)).toEqual({ + isEditable: true, + reason: null, + }); + }); + + it("reports 'offline' during pre-sync while disconnected", () => { + expect( + computeDictationAvailability({ + ...base, + showStatic: true, + isDisconnected: true, + }), + ).toEqual({ isEditable: false, reason: "offline" }); + }); + + it("reports 'connecting' during pre-sync while still connecting", () => { + expect( + computeDictationAvailability({ + ...base, + showStatic: true, + isDisconnected: false, + }), + ).toEqual({ isEditable: false, reason: "connecting" }); + }); + + it("reports 'read-only' without edit permission", () => { + expect( + computeDictationAvailability({ ...base, editable: false }), + ).toEqual({ isEditable: false, reason: "read-only" }); + }); + + it("reports 'read-only' when not in edit mode", () => { + expect( + computeDictationAvailability({ ...base, inEditMode: false }), + ).toEqual({ isEditable: false, reason: "read-only" }); + }); +}); diff --git a/apps/client/src/features/editor/editor-sync-state.ts b/apps/client/src/features/editor/editor-sync-state.ts index 6bb657cc..48066269 100644 --- a/apps/client/src/features/editor/editor-sync-state.ts +++ b/apps/client/src/features/editor/editor-sync-state.ts @@ -1,4 +1,5 @@ import { WebSocketStatus } from "@hocuspocus/provider"; +import type { DictationUnavailableReason } from "@/features/dictation/dictation-status"; /** * The collab document is usable only once the provider is Connected AND has @@ -30,3 +31,32 @@ export function isBodyEditable(opts: { }): boolean { return opts.editable && opts.inEditMode && !opts.showStatic; } + +/** + * Whether dictation can start and, when it can't, the cause-specific reason the + * mic button surfaces. Derives editability from `isBodyEditable` (the single, + * tested gate) so the published `isEditable` can never diverge from the actual + * body-editable state and make the tooltip lie (#309). + * + * `isDisconnected` is the caller's own boolean (collab connection is in the + * Disconnected state), passed in so this module stays free of the collab enum. + */ +export function computeDictationAvailability(opts: { + editable: boolean; + inEditMode: boolean; + showStatic: boolean; + isDisconnected: boolean; +}): { isEditable: boolean; reason: DictationUnavailableReason | null } { + const isEditable = isBodyEditable({ + editable: opts.editable, + inEditMode: opts.inEditMode, + showStatic: opts.showStatic, + }); + if (isEditable) return { isEditable, reason: null }; + // Permitted to edit and in edit mode but not yet synced (showStatic) → pre-sync. + if (opts.editable && opts.inEditMode && opts.showStatic) { + return { isEditable, reason: opts.isDisconnected ? "offline" : "connecting" }; + } + // No edit permission or not in edit mode. + return { isEditable, reason: "read-only" }; +} diff --git a/apps/client/src/features/editor/page-editor.tsx b/apps/client/src/features/editor/page-editor.tsx index 634bd79f..9953d6c5 100644 --- a/apps/client/src/features/editor/page-editor.tsx +++ b/apps/client/src/features/editor/page-editor.tsx @@ -36,7 +36,6 @@ import { pageEditorAtom, yjsConnectionStatusAtom, } from "@/features/editor/atoms/editor-atoms"; -import type { DictationUnavailableReason } from "@/features/dictation/dictation-status"; import { asideStateAtom } from "@/components/layouts/global/hooks/atoms/sidebar-atom"; import { activeCommentIdAtom, @@ -90,6 +89,7 @@ import { PageEmbedAncestryProvider } from "@/features/editor/components/page-emb import PageEmbedPicker from "@/features/editor/components/page-embed/page-embed-picker"; import { useTranslation } from "react-i18next"; import { + computeDictationAvailability, isBodyEditable, isCollabSynced, } from "@/features/editor/editor-sync-state"; @@ -495,22 +495,14 @@ export default function PageEditor({ // the mic button surfaces. Recomputed on the same signals that drive body // editability so the tooltip never lies about the current state. useEffect(() => { - const inEditMode = currentPageEditMode === PageEditMode.Edit; - const isEditable = editable && inEditMode && !showStatic; // mirrors editor.isEditable - let reason: DictationUnavailableReason | null = null; - if (!isEditable) { - if (editable && inEditMode && showStatic) { - // Permitted to edit and in edit mode, but the collab doc hasn't synced yet. - reason = - yjsConnectionStatus === WebSocketStatus.Disconnected - ? "offline" - : "connecting"; - } else { - // No edit permission or not in edit mode. - reason = "read-only"; - } - } - setDictationAvailability({ isEditable, reason }); + setDictationAvailability( + computeDictationAvailability({ + editable, + inEditMode: currentPageEditMode === PageEditMode.Edit, + showStatic, + isDisconnected: yjsConnectionStatus === WebSocketStatus.Disconnected, + }), + ); }, [ editable, currentPageEditMode,