fix(dictation): extract tested computeDictationAvailability + gate mic from the reactive atom (#314 F1/F2)
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) <noreply@anthropic.com>
This commit is contained in:
+32
-47
@@ -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(<DictationGroup editor={editor} />);
|
||||
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(
|
||||
<Provider store={store}>
|
||||
<DictationGroup editor={editor} />
|
||||
</Provider>,
|
||||
);
|
||||
|
||||
// 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);
|
||||
|
||||
@@ -83,7 +83,7 @@ export const DictationGroup: FC<Props> = ({ editor, color, iconSize }) => {
|
||||
streaming={streamingDictation}
|
||||
onStart={handleStart}
|
||||
onText={handleText}
|
||||
disabled={!editor.isEditable}
|
||||
disabled={!dictationAvailability.isEditable}
|
||||
unavailableReason={dictationAvailability.reason ?? undefined}
|
||||
color={color}
|
||||
iconSize={iconSize}
|
||||
|
||||
@@ -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" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user