Skip to content

Improve React code quality #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
johannesschiessl wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Improve React code quality #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fe64c68
Improve React code quality
johannesschiessl Jul 15, 2026
a241c66
Merge main and preserve React quality refactors
johannesschiessl Jul 24, 2026
b5e8278
Resolve remaining React Doctor findings
johannesschiessl Jul 24, 2026
17444a9
Merge main and preserve React quality fixes
johannesschiessl Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions apps/web/src/chats/ChatNavigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
import { router } from "@/router";
import { makeChatNavigation } from "./ChatNavigationState";
import type { ShowId } from "@showtime/contracts";
import { makeChatNavigation, type ChatOpenRequest } from "./ChatNavigationState";

export type { ChatOpenRequest } from "./ChatNavigationState";

const eventName = "showtime-chat-open";

const activeShowId = () => {
for (const match of router.state.matches) {
const showId = (match.params as { readonly showId?: unknown }).showId;
if (typeof showId === "string") return showId;
}
return undefined;
};
let chatNavigation: ReturnType<typeof makeChatNavigation> | undefined;

const chatNavigation = makeChatNavigation({
getActiveShowId: activeShowId,
navigateToShow: (showId) => router.navigate({ to: "/shows/$showId", params: { showId } }),
publishOpenRequest: () => window.dispatchEvent(new Event(eventName)),
});
export const configureChatNavigation = (options: {
readonly getActiveShowId: () => string | undefined;
readonly navigateToShow: (showId: ShowId) => Promise<unknown>;
}) => {
chatNavigation = makeChatNavigation({
...options,
publishOpenRequest: () => window.dispatchEvent(new Event(eventName)),
});
};

export const openChat = chatNavigation.open;
export const openChat = (request: ChatOpenRequest) => {
if (!chatNavigation) throw new Error("Chat navigation has not been configured");
return chatNavigation.open(request);
};

export const consumeChatOpenRequest = chatNavigation.consume;
export const consumeChatOpenRequest = (showId: ShowId) => chatNavigation?.consume(showId);

export const subscribeChatOpenRequests = (listener: () => void) => {
window.addEventListener(eventName, listener);
Expand Down
142 changes: 84 additions & 58 deletions apps/web/src/components/chats/ChatDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ function ChatDrawerView({
const open = controlledOpen ?? internalOpen;
const setOpen = onOpenChange ?? setInternalOpen;
const [selectedChannelId, setSelectedChannelId] = React.useState<ChatChannelId>();
const [pendingAnswers, setPendingAnswers] = React.useState<ReadonlyArray<AnswerRequest>>([]);
const newestSequences = React.useRef<ChatAnswerRequestSequences | undefined>(undefined);
const selectChannel = React.useCallback(
(channelId: ChatChannelId) => {
Expand All @@ -113,54 +112,17 @@ function ChatDrawerView({
return () => query.removeEventListener("change", update);
}, []);

React.useLayoutEffect(() => {
if (!profile || open) return;
return registerChatAnswerDialog(showId, profile.id);
}, [open, profile, showId]);
const openRequestedChat = React.useEffectEvent(() => {
const request = consumeChatOpenRequest(showId);
if (!request) return;
selectChannel(request.channelId);
setOpen(true);
});

React.useEffect(() => {
const openRequestedChat = () => {
const request = consumeChatOpenRequest(showId);
if (!request) return;
selectChannel(request.channelId);
setOpen(true);
};
openRequestedChat();
return subscribeChatOpenRequests(openRequestedChat);
}, [selectChannel, setOpen, showId]);

React.useEffect(() => {
if (!snapshot || !profile) return;
const { requests, sequences } = planChatAnswerRequests({
channels: snapshot.channels,
profileId: profile.id,
previousSequences: newestSequences.current,
shouldPrompt: !open,
});
newestSequences.current = sequences;
if (requests.length > 0)
setPendingAnswers((current) => [
...current,
...requests.filter((request) => !current.some((item) => item.id === request.id)),
]);
}, [open, profile, snapshot]);

React.useEffect(() => {
if (open) setPendingAnswers([]);
}, [open]);

const pendingAnswer = pendingAnswers[0];
const pendingChannel = pendingAnswer
? snapshot?.channels.find((channel) => channel.id === pendingAnswer.channelId)
: undefined;
const pendingAnswered = Boolean(
pendingAnswer &&
pendingChannel?.messages.some(
(message) =>
message.replyToMessageId === pendingAnswer.id && message.senderProfileId === profile?.id,
),
);
const dismissPendingAnswer = () => setPendingAnswers((current) => current.slice(1));
}, []);

return (
<>
Expand Down Expand Up @@ -189,26 +151,90 @@ function ChatDrawerView({
</DrawerContent>
</Drawer>
{profile && (
<ChatPresetAnswerDialog
open={Boolean(pendingAnswer) && !open}
onOpenChange={(nextOpen) => {
if (!nextOpen) dismissPendingAnswer();
}}
<ChatAnswerPrompt
key={open ? "open" : "closed"}
showId={showId}
profileId={profile.id}
request={pendingAnswer}
senderName={
profiles.find((candidate) => candidate.id === pendingAnswer?.senderProfileId)?.name ??
"the sender"
}
answered={pendingAnswered}
onAnswered={dismissPendingAnswer}
open={open}
profile={profile}
profiles={profiles}
snapshot={snapshot}
newestSequences={newestSequences}
/>
)}
</>
);
}

function ChatAnswerPrompt({
showId,
open,
profile,
profiles,
snapshot,
newestSequences,
}: {
readonly showId: ShowId;
readonly open: boolean;
readonly profile: Profile;
readonly profiles: ReadonlyArray<Profile>;
readonly snapshot: ChatSnapshot | undefined;
readonly newestSequences: React.RefObject<ChatAnswerRequestSequences | undefined>;
}) {
const [pendingAnswers, setPendingAnswers] = React.useState<ReadonlyArray<AnswerRequest>>([]);

React.useLayoutEffect(() => {
if (open) return;
return registerChatAnswerDialog(showId, profile.id);
}, [open, profile.id, showId]);

React.useEffect(() => {
if (!snapshot) return;
const { requests, sequences } = planChatAnswerRequests({
channels: snapshot.channels,
profileId: profile.id,
previousSequences: newestSequences.current,
shouldPrompt: !open,
});
newestSequences.current = sequences;
if (requests.length > 0)
setPendingAnswers((current) => [
...current,
...requests.filter((request) => !current.some((item) => item.id === request.id)),
]);
}, [newestSequences, open, profile.id, snapshot]);

const pendingAnswer = pendingAnswers[0];
const pendingChannel = pendingAnswer
? snapshot?.channels.find((channel) => channel.id === pendingAnswer.channelId)
: undefined;
const pendingAnswered = Boolean(
pendingAnswer &&
pendingChannel?.messages.some(
(message) =>
message.replyToMessageId === pendingAnswer.id && message.senderProfileId === profile.id,
),
);
const dismissPendingAnswer = () => setPendingAnswers((current) => current.slice(1));

return (
<ChatPresetAnswerDialog
open={Boolean(pendingAnswer) && !open}
onOpenChange={(nextOpen) => {
if (!nextOpen) dismissPendingAnswer();
}}
showId={showId}
profileId={profile.id}
request={pendingAnswer}
senderName={
profiles.find((candidate) => candidate.id === pendingAnswer?.senderProfileId)?.name ??
"the sender"
}
answered={pendingAnswered}
onAnswered={dismissPendingAnswer}
/>
);
}

export function ChatUnreadBadge({ showId }: { readonly showId: ShowId }) {
const profilesResult = useAtomValue(profileAtoms.state);
const profileState = AsyncResult.isSuccess(profilesResult) ? profilesResult.value : undefined;
Expand Down
9 changes: 6 additions & 3 deletions apps/web/src/components/chats/ChatMessageBody.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ export function ChatMessageBody({
readonly parts?: ReadonlyArray<ChatMessagePart>;
}) {
if (!parts?.length) return body;
return parts.map((part, index) => {
if (part.type === "text") return <span key={index}>{part.text}</span>;
let characterOffset = 0;
return parts.map((part) => {
const key = `${part.type}:${characterOffset}:${part.text}`;
Comment on lines +15 to +16

@cubic-dev-ai cubic-dev-ai Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Empty adjacent parts can still produce duplicate React keys, so React may warn and reconcile these sibling spans unpredictably. Including the part index as a uniqueness fallback (or otherwise assigning an ID) would preserve unique keys for all contract-valid ChatMessagePart arrays.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/chats/ChatMessageBody.tsx, line 15:

<comment>Empty adjacent parts can still produce duplicate React keys, so React may warn and reconcile these sibling spans unpredictably. Including the part index as a uniqueness fallback (or otherwise assigning an ID) would preserve unique keys for all contract-valid `ChatMessagePart` arrays.</comment>

<file context>
@@ -11,14 +11,17 @@ export function ChatMessageBody({
-  return parts.map((part, index) => {
-    if (part.type === "text") return <span key={index}>{part.text}</span>;
+  let characterOffset = 0;
+  return parts.map((part) => {
+    const key = `${part.type}:${characterOffset}:${part.text}`;
+    characterOffset += part.text.length;
</file context>
Suggested change
return parts.map((part) => {
const key = `${part.type}:${characterOffset}:${part.text}`;
return parts.map((part, index) => {
const key = `${part.type}:${characterOffset}:${part.text}:${index}`;

characterOffset += part.text.length;
if (part.type === "text") return <span key={key}>{part.text}</span>;
const colors = microphoneColorClassNames[part.color];
const label = part.type === "microphone" ? "Microphone" : "Mix";
const Icon = part.type === "microphone" ? Mic2Icon : SpeakerIcon;
return (
<span
key={index}
key={key}
className="mx-0.5 inline-flex max-w-full translate-y-px items-center gap-1 rounded-md border bg-background/80 py-0.5 pr-1.5 pl-0.5 align-baseline text-xs font-medium text-foreground"
aria-label={`${label} ${part.number}${part.name ? `, ${part.name}` : ""}`}
title={part.name || `${label} ${part.number}`}
Expand Down
6 changes: 4 additions & 2 deletions apps/web/src/components/chats/ChatPresetAnswer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import {
import { ArrowUpIcon, CheckIcon } from "lucide-react";
import {
ChatPresetFieldInputs,
initialChatPresetValues,
resolveChatPresetDefinition,
useChatPresetResources,
} from "@/components/chats/ChatPresetFields";
import {
initialChatPresetValues,
resolveChatPresetDefinition,
} from "@/components/chats/ChatPresetFieldState";
import { ChatMessageBody } from "@/components/chats/ChatMessageBody";
import { useSendChatMessage } from "@/components/chats/useSendChatMessage";
import { Button } from "@/components/ui/button";
Expand Down
Loading