fix: address presentation sync final review

This commit is contained in:
lda
2026-07-14 15:47:31 +07:00 Verified
parent f5eddae5a4
commit 7aafda05ab
8 changed files with 299 additions and 37 deletions
@@ -126,7 +126,9 @@ const connectAudience = async (
socket.serverMessage(locationSnapshot(initialHash, 0)); socket.serverMessage(locationSnapshot(initialHash, 0));
socket.serverMessage(presenceSnapshot); socket.serverMessage(presenceSnapshot);
}); });
await waitFor(() => expect(screen.getByRole("status", { name: "Connected" })).toBeInTheDocument()); await waitFor(() => expect(screen.getByRole("button", {
name: /Pair presentation Connected 1 presenter · 1 audience/,
})).toHaveAttribute("aria-expanded", "false"));
return socket; return socket;
}; };
@@ -136,6 +136,7 @@ describe("PresenterRoute", () => {
mockedUsePresentationSync.mockReturnValue(controller); mockedUsePresentationSync.mockReturnValue(controller);
const { rerender } = render(<PresenterRoute />); const { rerender } = render(<PresenterRoute />);
await userEvent.click(screen.getByRole("button", { name: /Pair presentation/ }));
await userEvent.click(screen.getByRole("button", { name: "End presentation" })); await userEvent.click(screen.getByRole("button", { name: "End presentation" }));
await userEvent.click(screen.getByRole("button", { name: "End presentation now" })); await userEvent.click(screen.getByRole("button", { name: "End presentation now" }));
expect(controller.endSession).toHaveBeenCalledOnce(); expect(controller.endSession).toHaveBeenCalledOnce();
@@ -73,7 +73,7 @@ describe("PresentationPairingPanel", () => {
}); });
it("gives expanded state one root surface owner without a double-card contract", () => { it("gives expanded state one root surface owner without a double-card contract", () => {
renderPanel("presenter", connectedState()); renderPanel("presenter", connectedState("waiting"));
const panel = screen.getByRole("complementary", { const panel = screen.getByRole("complementary", {
name: "Presentation pairing", name: "Presentation pairing",
@@ -143,7 +143,8 @@ describe("PresentationPairingPanel", () => {
it("uses concise connected and reconnecting status copy", () => { it("uses concise connected and reconnecting status copy", () => {
const { rerender } = renderPanel("audience", connectedState("connected")); const { rerender } = renderPanel("audience", connectedState("connected"));
expect(screen.getByRole("status", { name: "Connected" })).toBeInTheDocument(); expect(screen.getByRole("button", { name: /Pair presentation Connected/ }))
.toBeInTheDocument();
rerender( rerender(
<PresentationPairingPanel <PresentationPairingPanel
@@ -151,7 +152,67 @@ describe("PresentationPairingPanel", () => {
controller={controllerFor(connectedState("reconnecting"))} controller={controllerFor(connectedState("reconnecting"))}
/>, />,
); );
expect(screen.getByRole("status", { name: "Reconnecting" })).toBeInTheDocument(); expect(screen.getByRole("button", { name: /Pair presentation Reconnecting/ }))
.toBeInTheDocument();
});
it("collapses on connection and shows peer presence in the compact trigger", () => {
const { rerender } = renderPanel("presenter", connectedState("waiting"));
expect(screen.getByRole("button", { name: /Pair presentation/ }))
.toHaveAttribute("aria-expanded", "true");
rerender(
<PresentationPairingPanel
role="presenter"
controller={controllerFor(connectedState("connected"))}
/>,
);
const trigger = screen.getByRole("button", {
name: /Pair presentation Connected 1 presenter · 2 audiences/,
});
expect(trigger).toHaveAttribute("aria-expanded", "false");
expect(screen.queryByLabelText("Pairing QR code")).toBeNull();
});
it("allows the compact connected panel to be reopened manually", async () => {
const user = userEvent.setup();
renderPanel("presenter", connectedState());
const trigger = screen.getByRole("button", { name: /Pair presentation Connected/ });
expect(trigger).toHaveAttribute("aria-expanded", "false");
await user.click(trigger);
expect(trigger).toHaveAttribute("aria-expanded", "true");
expect(screen.getByLabelText("Pairing QR code")).toBeInTheDocument();
});
it.each([
[
"failed",
{ kind: "failed", message: "Socket unavailable", retryable: true } as const,
"Socket unavailable",
],
[
"ended",
{ kind: "ended", reason: "presenter_ended" } as const,
"The presenter ended this session.",
],
])("reopens %s details after connected collapse", (_kind, nextState, detail) => {
const { rerender } = renderPanel("presenter", connectedState());
expect(screen.getByRole("button", { name: /Pair presentation/ }))
.toHaveAttribute("aria-expanded", "false");
rerender(
<PresentationPairingPanel
role="presenter"
controller={controllerFor(nextState)}
/>,
);
expect(screen.getByText(detail)).toBeInTheDocument();
expect(screen.getByRole("button", { name: /Pair presentation/ }))
.toHaveAttribute("aria-expanded", "true");
}); });
it("offers retry for a retryable failure", async () => { it("offers retry for a retryable failure", async () => {
@@ -180,6 +241,8 @@ describe("PresentationPairingPanel", () => {
const user = userEvent.setup(); const user = userEvent.setup();
const { controller } = renderPanel("presenter", connectedState()); const { controller } = renderPanel("presenter", connectedState());
await user.click(screen.getByRole("button", { name: /Pair presentation/ }));
await user.click(screen.getByRole("button", { name: "End presentation" })); await user.click(screen.getByRole("button", { name: "End presentation" }));
expect(screen.getByText("End presentation for everyone?")).toBeInTheDocument(); expect(screen.getByText("End presentation for everyone?")).toBeInTheDocument();
expect(controller.endSession).not.toHaveBeenCalled(); expect(controller.endSession).not.toHaveBeenCalled();
@@ -193,4 +256,10 @@ describe("PresentationPairingPanel", () => {
expect(screen.queryByRole("button", { name: /End presentation/ })).toBeNull(); expect(screen.queryByRole("button", { name: /End presentation/ })).toBeNull();
}); });
it("does not expose presenter termination while reconnecting", () => {
renderPanel("presenter", connectedState("reconnecting"));
expect(screen.queryByRole("button", { name: /End presentation/ })).toBeNull();
});
}); });
@@ -77,7 +77,9 @@ export const PresentationPairingPanel = ({
controller, controller,
}: PresentationPairingPanelProps) => { }: PresentationPairingPanelProps) => {
const { state } = controller; const { state } = controller;
const [isOpen, setIsOpen] = useState(() => state.kind !== "standalone"); const [isOpen, setIsOpen] = useState(
() => state.kind !== "standalone" && state.kind !== "connected",
);
const [code, setCode] = useState(""); const [code, setCode] = useState("");
const [copyStatus, setCopyStatus] = useState(""); const [copyStatus, setCopyStatus] = useState("");
const [isConfirmingEnd, setIsConfirmingEnd] = useState(false); const [isConfirmingEnd, setIsConfirmingEnd] = useState(false);
@@ -87,10 +89,29 @@ export const PresentationPairingPanel = ({
const inputId = `${idPrefix}-code`; const inputId = `${idPrefix}-code`;
const inputCode = state.kind === "joining" ? state.code : code; const inputCode = state.kind === "joining" ? state.code : code;
const normalizedInputCode = normalizeJoinCode(inputCode); const normalizedInputCode = normalizeJoinCode(inputCode);
const compactPresence = state.kind === "connected"
? `${pluralize(state.presence.presenters, "presenter")} · ${pluralize(state.presence.audience, "audience")}`
: null;
const triggerLabel = ["Pair presentation", status, compactPresence]
.filter((part) => part !== null)
.join(" ");
// Lifecycle and error states reopen after transitions so recovery and end details remain reachable despite manual collapse. // Connection gets out of the deck's way once. Terminal details reopen, while
// same-state rerenders preserve a user's manual toggle choice.
useEffect(() => { useEffect(() => {
if (state.kind !== "standalone") setIsOpen(true); if (state.kind === "connected") {
setIsOpen(false);
return;
}
if (
state.kind === "creating" ||
state.kind === "joining" ||
state.kind === "waiting" ||
state.kind === "failed" ||
state.kind === "ended"
) {
setIsOpen(true);
}
}, [state.kind]); }, [state.kind]);
const openPanel = (): void => setIsOpen((open) => !open); const openPanel = (): void => setIsOpen((open) => !open);
@@ -138,10 +159,15 @@ export const PresentationPairingPanel = ({
type="button" type="button"
aria-controls={panelId} aria-controls={panelId}
aria-expanded={isOpen} aria-expanded={isOpen}
aria-label={triggerLabel}
onClick={openPanel} onClick={openPanel}
> >
<span>Pair presentation</span> <span>Pair presentation</span>
{status && <span className="presentation-pairing__trigger-status">{status}</span>} {status && (
<span className="presentation-pairing__trigger-status">
{compactPresence === null ? status : `${status} · ${compactPresence}`}
</span>
)}
</button> </button>
{isOpen && ( {isOpen && (
@@ -235,7 +261,7 @@ export const PresentationPairingPanel = ({
<span className="presentation-pairing__copy-status" role="status" aria-live="polite"> <span className="presentation-pairing__copy-status" role="status" aria-live="polite">
{copyStatus} {copyStatus}
</span> </span>
{role === "presenter" && ( {role === "presenter" && state.kind === "connected" && (
<div className="presentation-pairing__end"> <div className="presentation-pairing__end">
{isConfirmingEnd ? ( {isConfirmingEnd ? (
<div className="presentation-pairing__confirmation" role="alertdialog" aria-label="End presentation confirmation"> <div className="presentation-pairing__confirmation" role="alertdialog" aria-label="End presentation confirmation">
@@ -235,6 +235,22 @@ describe("presentation sync client", () => {
expect(sockets).toHaveLength(1); expect(sockets).toHaveLength(1);
}); });
it("refuses to end while the socket is reconnecting", () => {
vi.useFakeTimers();
const storage = new MemoryStorage();
const sockets: FakeSocket[] = [];
const client = createPresentationSyncClient(makeDependencies(storage, sockets));
client.connect(grant, () => {});
sockets[0]?.open();
sockets[0]?.close(1006, "network");
expect(client.end()).toBe(false);
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).not.toBeNull();
vi.advanceTimersByTime(500);
expect(sockets).toHaveLength(2);
});
it("closes socket errors without using reserved close code 1006", () => { it("closes socket errors without using reserved close code 1006", () => {
const storage = new MemoryStorage(); const storage = new MemoryStorage();
const sockets: FakeSocket[] = []; const sockets: FakeSocket[] = [];
@@ -302,14 +302,14 @@ export const createPresentationSyncClient = (
return messageId; return messageId;
}; };
const end = (): void => { const end = (): boolean => {
if (socket === null || !websocketIsOpen(socket) || !active) return false;
active = false; active = false;
clearReconnectTimer(); clearReconnectTimer();
if (socket !== null && websocketIsOpen(socket)) {
socket.send(JSON.stringify({ type: "session.end" })); socket.send(JSON.stringify({ type: "session.end" }));
}
clearSavedGrant(); clearSavedGrant();
closeSocket(); closeSocket();
return true;
}; };
const leave = (): void => { const leave = (): void => {
@@ -171,6 +171,86 @@ describe("usePresentationSync", () => {
}); });
}); });
it("publishes waiting navigation before a late join converges", async () => {
const creator = makeDependencies();
creator.fetch.mockImplementation(async () => resolvedGrant());
const applyCreatorHash = vi.fn();
const creatorView = renderHook(
({ hash }) =>
usePresentationSync({
role: "presenter",
currentHash: hash,
applyRemoteHash: applyCreatorHash,
dependencies: creator.dependencies,
}),
{ initialProps: { hash: "#scene/thesis/title" } },
);
await act(async () => {
await creatorView.result.current.startSession();
});
expect(creatorView.result.current.state.kind).toBe("waiting");
creatorView.rerender({ hash: "#scene/problem/direct-actions" });
expect(creator.sockets[0]?.sent).toHaveLength(0);
await act(async () => {
creator.sockets[0]?.open();
});
const publication = JSON.parse(creator.sockets[0]?.sent[0] ?? "{}");
expect(publication).toMatchObject({
type: "location.publish",
hash: "#scene/problem/direct-actions",
baseRevision: 0,
});
await act(async () => {
creator.sockets[0]?.serverMessage(snapshot("#scene/thesis/title", 0));
});
expect(applyCreatorHash).not.toHaveBeenCalled();
await act(async () => {
creator.sockets[0]?.serverMessage({
...snapshot("#scene/problem/direct-actions", 1),
originatingMessageId: publication.messageId,
});
});
const lateGrant = {
...grant,
connectionToken: "token-2",
snapshot: { hash: "#scene/problem/direct-actions", revision: 1 },
};
const lateJoin = makeDependencies();
lateJoin.fetch.mockImplementation(async () =>
new Response(JSON.stringify(lateGrant), { status: 200 }),
);
const applyLateHash = vi.fn();
const joinedView = renderHook(() =>
usePresentationSync({
role: "audience",
currentHash: "#scene/thesis/title",
applyRemoteHash: applyLateHash,
dependencies: lateJoin.dependencies,
}),
);
await act(async () => {
await joinedView.result.current.joinSession("ABC123");
lateJoin.sockets[0]?.open();
lateJoin.sockets[0]?.serverMessage(
snapshot("#scene/problem/direct-actions", 1),
);
});
expect(applyLateHash).toHaveBeenCalledWith(
"#scene/problem/direct-actions",
);
expect(joinedView.result.current.state).toMatchObject({
snapshot: { hash: "#scene/problem/direct-actions", revision: 1 },
});
});
it("uses committed props for stable actions and remote callbacks", async () => { it("uses committed props for stable actions and remote callbacks", async () => {
const { dependencies, sockets, fetch } = makeDependencies(); const { dependencies, sockets, fetch } = makeDependencies();
fetch.mockImplementation(async () => resolvedGrant()); fetch.mockImplementation(async () => resolvedGrant());
@@ -331,7 +411,7 @@ describe("usePresentationSync", () => {
); );
await connectSession(result, sockets); await connectSession(result, sockets);
sockets[0]?.close(1006, "network"); act(() => sockets[0]?.close(1006, "network"));
rerender({ hash: "#scene/problem/direct-actions" }); rerender({ hash: "#scene/problem/direct-actions" });
expect(sockets[0]?.sent).toHaveLength(0); expect(sockets[0]?.sent).toHaveLength(0);
@@ -351,6 +431,29 @@ describe("usePresentationSync", () => {
)).toHaveLength(0); )).toHaveLength(0);
}); });
it("does not report presenter termination while reconnecting", async () => {
vi.useFakeTimers();
const { dependencies, sockets, storage, fetch } = makeDependencies();
fetch.mockImplementation(async () => resolvedGrant());
const { result } = renderHook(() =>
usePresentationSync({
role: "presenter",
currentHash: "#scene/thesis/title",
applyRemoteHash: vi.fn(),
dependencies,
}),
);
await connectSession(result, sockets);
act(() => sockets[0]?.close(1006, "network"));
expect(result.current.state.kind).toBe("reconnecting");
act(() => result.current.endSession());
expect(result.current.state.kind).toBe("reconnecting");
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).not.toBeNull();
});
it("keeps local navigation standalone when session creation fails", async () => { it("keeps local navigation standalone when session creation fails", async () => {
const { dependencies, fetch } = makeDependencies(); const { dependencies, fetch } = makeDependencies();
fetch.mockRejectedValue(new Error("server unavailable")); fetch.mockRejectedValue(new Error("server unavailable"));
@@ -524,8 +627,8 @@ describe("usePresentationSync", () => {
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).not.toBeNull(); expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).not.toBeNull();
}); });
it("ignores a deferred result after leave, end, or unmount", async () => { it("ignores a deferred result after leave or unmount", async () => {
const makeDeferredHook = (role: "presenter" | "audience" = "presenter") => { const makeDeferredHook = () => {
const { dependencies, sockets, storage, fetch } = makeDependencies(); const { dependencies, sockets, storage, fetch } = makeDependencies();
let resolvePending: ((response: Response) => void) | null = null; let resolvePending: ((response: Response) => void) | null = null;
fetch.mockImplementation( fetch.mockImplementation(
@@ -533,7 +636,7 @@ describe("usePresentationSync", () => {
); );
const rendered = renderHook(() => const rendered = renderHook(() =>
usePresentationSync({ usePresentationSync({
role, role: "presenter",
currentHash: "#scene/thesis/title", currentHash: "#scene/thesis/title",
applyRemoteHash: () => {}, applyRemoteHash: () => {},
dependencies, dependencies,
@@ -556,18 +659,6 @@ describe("usePresentationSync", () => {
expect(left.result.current.state).toEqual({ kind: "ended", reason: "left" }); expect(left.result.current.state).toEqual({ kind: "ended", reason: "left" });
left.unmount(); left.unmount();
const ended = makeDeferredHook("audience");
await act(async () => { void ended.result.current.joinSession("AAA111"); });
await act(async () => { ended.result.current.endSession(); });
await act(async () => { ended.release(resolvedGrant()); await settleAsync(); });
expect(ended.sockets).toHaveLength(0);
expect(ended.storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toBeNull();
expect(ended.result.current.state).toEqual({
kind: "ended",
reason: "presenter_ended",
});
ended.unmount();
const unmounted = makeDeferredHook(); const unmounted = makeDeferredHook();
await act(async () => { void unmounted.result.current.startSession(); }); await act(async () => { void unmounted.result.current.startSession(); });
unmounted.unmount(); unmounted.unmount();
@@ -157,6 +157,8 @@ const createController = (options: {
let currentGrant: SessionGrant | null = null; let currentGrant: SessionGrant | null = null;
let lastOperation: LastOperation = null; let lastOperation: LastOperation = null;
let pendingJoin: PendingJoin | null = null; let pendingJoin: PendingJoin | null = null;
let pendingWaitingHash: string | null = null;
let waitingPublicationId: string | null = null;
const listeners = new Set<() => void>(); const listeners = new Set<() => void>();
const pendingMessageIds = new Set<string>(); const pendingMessageIds = new Set<string>();
@@ -187,6 +189,23 @@ const createController = (options: {
options.applyRemoteHash(snapshot.hash); options.applyRemoteHash(snapshot.hash);
}; };
const publishNow = (hash: string): string | null => {
if (
state.kind !== "waiting" &&
state.kind !== "connected"
) {
return null;
}
const messageId = options.client.publish(hash, state.snapshot.revision);
if (messageId !== null) pendingMessageIds.add(messageId);
return messageId;
};
const flushWaitingPublication = (): void => {
if (pendingWaitingHash === null || waitingPublicationId !== null) return;
waitingPublicationId = publishNow(pendingWaitingHash);
};
const handleClientEvent = (event: Parameters<PresentationSyncClient["connect"]>[1] extends ( const handleClientEvent = (event: Parameters<PresentationSyncClient["connect"]>[1] extends (
event: infer Event, event: infer Event,
) => void ) => void
@@ -196,8 +215,14 @@ const createController = (options: {
switch (event.type) { switch (event.type) {
case "open": case "open":
if (state.kind === "waiting") flushWaitingPublication();
return; return;
case "reconnecting": case "reconnecting":
// Navigation during reconnect is deliberately not queued: the next
// server snapshot remains authoritative for convergence.
pendingWaitingHash = null;
waitingPublicationId = null;
pendingMessageIds.clear();
dispatch({ type: "socket_reconnecting" }); dispatch({ type: "socket_reconnecting" });
return; return;
case "ended": case "ended":
@@ -220,14 +245,35 @@ const createController = (options: {
const isOwnPublish = const isOwnPublish =
message.originatingMessageId !== null && message.originatingMessageId !== null &&
pendingMessageIds.delete(message.originatingMessageId); pendingMessageIds.delete(message.originatingMessageId);
if (!isOwnPublish) applySnapshot(message.snapshot);
dispatch({ type: "location_snapshot", snapshot: message.snapshot }); dispatch({ type: "location_snapshot", snapshot: message.snapshot });
if (isOwnPublish) {
if (message.originatingMessageId === waitingPublicationId) {
waitingPublicationId = null;
if (pendingWaitingHash === message.snapshot.hash) {
pendingWaitingHash = null;
} else {
flushWaitingPublication();
}
}
return;
}
// The room's creation snapshot may arrive after the creator has
// published waiting navigation. Keep that local hash authoritative
// until the server accepts or rejects its publication.
if (pendingWaitingHash === null) applySnapshot(message.snapshot);
return; return;
} }
if (message.type === "location.rejected") { if (message.type === "location.rejected") {
pendingMessageIds.delete(message.messageId); pendingMessageIds.delete(message.messageId);
applySnapshot(message.current);
dispatch({ type: "location_rejected", snapshot: message.current }); dispatch({ type: "location_rejected", snapshot: message.current });
if (message.messageId === waitingPublicationId) {
waitingPublicationId = null;
if (pendingWaitingHash !== null) {
flushWaitingPublication();
return;
}
}
applySnapshot(message.current);
return; return;
} }
if (message.type === "presence.snapshot") { if (message.type === "presence.snapshot") {
@@ -253,10 +299,13 @@ const createController = (options: {
}; };
const publish = (hash: string): string | null => { const publish = (hash: string): string | null => {
if (state.kind !== "connected") return null; if (state.kind !== "waiting" && state.kind !== "connected") return null;
const messageId = options.client.publish(hash, state.snapshot.revision); if (state.kind === "connected" && pendingWaitingHash === null) {
if (messageId !== null) pendingMessageIds.add(messageId); return publishNow(hash);
return messageId; }
pendingWaitingHash = hash;
flushWaitingPublication();
return waitingPublicationId;
}; };
const restoreSavedGrant = (): SessionGrant | null => const restoreSavedGrant = (): SessionGrant | null =>
@@ -271,6 +320,8 @@ const createController = (options: {
const generation = beginOperation(); const generation = beginOperation();
lastOperation = { kind: "create" }; lastOperation = { kind: "create" };
currentGrant = null; currentGrant = null;
pendingWaitingHash = null;
waitingPublicationId = null;
pendingMessageIds.clear(); pendingMessageIds.clear();
options.client.leave(); options.client.leave();
dispatch({ type: "start_create" }); dispatch({ type: "start_create" });
@@ -302,6 +353,8 @@ const createController = (options: {
reusePending && pendingJoin?.code === normalizedCode ? pendingJoin : null; reusePending && pendingJoin?.code === normalizedCode ? pendingJoin : null;
lastOperation = { kind: "join", code: normalizedCode }; lastOperation = { kind: "join", code: normalizedCode };
currentGrant = null; currentGrant = null;
pendingWaitingHash = null;
waitingPublicationId = null;
pendingMessageIds.clear(); pendingMessageIds.clear();
options.client.leave(); options.client.leave();
dispatch({ type: "start_join", code: normalizedCode }); dispatch({ type: "start_join", code: normalizedCode });
@@ -344,16 +397,20 @@ const createController = (options: {
const leaveSession = (): void => { const leaveSession = (): void => {
beginOperation(); beginOperation();
currentGrant = null; currentGrant = null;
pendingWaitingHash = null;
waitingPublicationId = null;
pendingMessageIds.clear(); pendingMessageIds.clear();
options.client.leave(); options.client.leave();
dispatch({ type: "left" }); dispatch({ type: "left" });
}; };
const endSession = (): void => { const endSession = (): void => {
if (state.kind !== "connected" || !options.client.end()) return;
beginOperation(); beginOperation();
currentGrant = null; currentGrant = null;
pendingWaitingHash = null;
waitingPublicationId = null;
pendingMessageIds.clear(); pendingMessageIds.clear();
options.client.end();
dispatch({ type: "session_ended", reason: "presenter_ended" }); dispatch({ type: "session_ended", reason: "presenter_ended" });
}; };
@@ -490,7 +547,7 @@ export const usePresentationSync = ({
if (lastObservedHashRef.current === currentHash) return; if (lastObservedHashRef.current === currentHash) return;
lastObservedHashRef.current = currentHash; lastObservedHashRef.current = currentHash;
if (state.kind !== "connected") return; if (state.kind !== "waiting" && state.kind !== "connected") return;
controller.publish(currentHash); controller.publish(currentHash);
}, [controller, currentHash, state.kind]); }, [controller, currentHash, state.kind]);