Skip to content

Commit 50460ed

Browse files
committed
refactor(chat): trim the welcome card to what it actually needs
Ponytail pass over the diff. No behaviour change; the composer gating and the toast routing are untouched. - three copy-pasted <li> blocks collapse into a map over the feature keys - the bullets were `list-style: none` plus a hand-rolled dot span per row; native markers do it, so the spans and their 9-line rule are gone (the ul/li must stay non-flex or they stop being list-items, and the global reset means `list-style: disc` has to be set back explicitly) - the Sparkles glyph loses its 40x40 tinted tile, keeps its colour - role="region" + useId + aria-labelledby wrapped a transient empty state in a named landmark the <h2> already announces - the es and ja-JP test cases repeated the fr one against 2 arbitrary locales of 13; localeParity.test.ts already enforces key presence, and fr keeps the English-fallback assertion parity cannot see - comment blocks that restated the code, or restated each other, cut back to the parts that are not obvious from the lines below them - dead `background` in the CTA transition (its background never changes) net: -93 lines.
1 parent 6b74a49 commit 50460ed

5 files changed

Lines changed: 37 additions & 130 deletions

File tree

src/components/ai-edition/ChatWelcome.test.tsx

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
// ChatWelcome guards the "no provider connected" empty state.
2-
//
3-
// Two things matter here:
4-
// 1. the welcome card is fully localized — every locale must render the
5-
// CTA and the disclaimer, not fall back to a key like "chat.welcome.cta"
6-
// 2. the CTA opens the provider settings dialog (whatever the parent
7-
// decided to do, it just has to fire the callback)
1+
// ChatWelcome guards the "no provider connected" empty state: the copy reaches
2+
// the DOM, the CTA fires, and a non-English locale is really translated rather
3+
// than falling back to English. localeParity.test.ts covers key presence for
4+
// the other locales; only the fallback check needs a rendered card.
85

96
import "@testing-library/jest-dom";
107
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
@@ -61,29 +58,4 @@ describe("ChatWelcome", () => {
6158
expect(screen.queryByText(/transcript will be sent/i)).not.toBeInTheDocument();
6259
expect(screen.getByText(/transcription de votre vidéo/i)).toBeInTheDocument();
6360
});
64-
65-
it("renders the Spanish welcome card with translated copy", () => {
66-
renderIn("es", <ChatWelcome onOpenProviderSettings={vi.fn()} />);
67-
68-
expect(screen.getByText(/configurar un proveedor/i)).toBeInTheDocument();
69-
expect(screen.getByText(/transcripción de tu vídeo/i)).toBeInTheDocument();
70-
});
71-
72-
it("renders the Japanese welcome card with translated copy", () => {
73-
renderIn("ja-JP", <ChatWelcome onOpenProviderSettings={vi.fn()} />);
74-
75-
expect(screen.getByText(//)).toBeInTheDocument();
76-
// The Japanese disclaimer uses 「動画」 — guard against the English
77-
// fallback by asserting the locale-specific substring is present.
78-
expect(screen.getByText(//)).toBeInTheDocument();
79-
});
80-
81-
it("exposes the welcome region to assistive tech", () => {
82-
renderIn("en", <ChatWelcome onOpenProviderSettings={vi.fn()} />);
83-
84-
// The region is labelled by the <h2> itself (aria-labelledby), so the
85-
// title is announced once, not duplicated by a redundant aria-label.
86-
const region = screen.getByRole("region", { name: /bring your own ai/i });
87-
expect(region).toBeInTheDocument();
88-
});
8961
});

src/components/ai-edition/ChatWelcome.tsx

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,36 @@
11
// Welcome view for the LM chat panel.
22
//
33
// Shown in the chat body when the chat has nothing it can talk to — see
4-
// canSendChat() in chatAvailability.ts for the exact condition. It explains
5-
// what the chat can do, then routes the user to the provider settings dialog
6-
// with a single CTA. A small disclaimer under the button makes it clear that,
7-
// once a provider IS connected, the video's transcript will be sent to it.
8-
//
9-
// It replaces the `chat.emptyState` hint (which would be a dead end with no
10-
// provider), but only while the conversation is empty: a user who disconnects
11-
// mid-project keeps their history on screen, with the composer disabled.
4+
// canSendChat() in chatAvailability.ts. It replaces the `chat.emptyState` hint
5+
// (a dead end with no provider) only while the conversation is empty, so a user
6+
// who disconnects mid-project keeps their history with the composer disabled.
127

138
import { ArrowRight, Info, Sparkles } from "lucide-react";
14-
import { useId } from "react";
159
import { useScopedT } from "@/contexts/I18nContext";
1610
import styles from "./NewEditorShell.module.css";
1711

12+
const FEATURE_KEYS = ["feature1", "feature2", "feature3"] as const;
13+
1814
interface ChatWelcomeProps {
1915
/** Open the provider settings modal so the user can pick + connect one. */
2016
onOpenProviderSettings: () => void;
2117
}
2218

2319
export function ChatWelcome({ onOpenProviderSettings }: ChatWelcomeProps) {
2420
const t = useScopedT("editor");
25-
const titleId = useId();
2621

2722
return (
28-
<div className={styles.chatWelcome} role="region" aria-labelledby={titleId}>
23+
<div className={styles.chatWelcome}>
2924
<header className={styles.chatWelcomeHero}>
30-
<span className={styles.chatWelcomeIcon} aria-hidden="true">
31-
<Sparkles size={20} />
32-
</span>
33-
<h2 className={styles.chatWelcomeTitle} id={titleId}>
34-
{t("chat.welcome.title")}
35-
</h2>
25+
<Sparkles size={20} className={styles.chatWelcomeIcon} aria-hidden="true" />
26+
<h2 className={styles.chatWelcomeTitle}>{t("chat.welcome.title")}</h2>
3627
<p className={styles.chatWelcomeSubtitle}>{t("chat.welcome.subtitle")}</p>
3728
</header>
3829

3930
<ul className={styles.chatWelcomeFeatures}>
40-
<li className={styles.chatWelcomeFeature}>
41-
<span className={styles.chatWelcomeFeatureDot} aria-hidden="true" />
42-
<span>{t("chat.welcome.feature1")}</span>
43-
</li>
44-
<li className={styles.chatWelcomeFeature}>
45-
<span className={styles.chatWelcomeFeatureDot} aria-hidden="true" />
46-
<span>{t("chat.welcome.feature2")}</span>
47-
</li>
48-
<li className={styles.chatWelcomeFeature}>
49-
<span className={styles.chatWelcomeFeatureDot} aria-hidden="true" />
50-
<span>{t("chat.welcome.feature3")}</span>
51-
</li>
31+
{FEATURE_KEYS.map((key) => (
32+
<li key={key}>{t(`chat.welcome.${key}`)}</li>
33+
))}
5234
</ul>
5335

5436
<button type="button" className={styles.chatWelcomeCta} onClick={onOpenProviderSettings}>

src/components/ai-edition/LeftPanel.tsx

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -679,12 +679,7 @@ function ChatStripPanel() {
679679
const [reasoningBusy, setReasoningBusy] = useState(false);
680680
// null until the first llmGetSnapshot() lands: "unknown", not "none".
681681
const [connectedProviders, setConnectedProviders] = useState<string[] | null>(null);
682-
// Derive "can the user actually chat right now?" once, then use it
683-
// everywhere. A stale active config that points at a provider with no
684-
// credentials still counts as not-ready — the welcome view + disabled
685-
// composer are the right surface. See chatAvailability.ts for the truth
686-
// table; the empty-string provider case is what you get immediately
687-
// after `llmDisconnect` writes its reset.
682+
// unknown ≠ none; see chatAvailability.ts.
688683
const canChat = canSendChat(llmConfig, connectedProviders);
689684
const [modelPopoverOpen, setModelPopoverOpen] = useState(false);
690685
const modelButtonRef = useRef<HTMLButtonElement | null>(null);
@@ -783,13 +778,9 @@ function ChatStripPanel() {
783778
const send = async (overrideText?: string) => {
784779
const text = (overrideText ?? input).trim();
785780
if (!projectId || !text || busy) return;
786-
// ponytail: no connected provider = nothing to talk to. Bounce the user
787-
// to the settings modal instead of sending a doomed request and
788-
// surfacing a generic LLM error. The composer is also disabled in this
789-
// state, but Enter-to-send could still slip through (e.g. focus
790-
// restored via shortcut), so the check lives here too. The toast is what
791-
// explains the modal to prompts that never touched the composer — the
792-
// timeline's Auto-enhance hands its text straight to send().
781+
// ponytail: nothing to talk to. Bounce to the settings modal instead of
782+
// firing a doomed request. The composer is disabled in this state too,
783+
// but Auto-enhance calls send() directly and Enter can slip through.
793784
if (!canChat) {
794785
toast.error(t("chat.composerDisabledNoProvider"));
795786
setSettingsOpen(true);
@@ -868,14 +859,10 @@ function ChatStripPanel() {
868859
// timeline's Auto-enhance → "Smart zooms + cuts with AI"). Routes through
869860
// the normal send() so sessions/checkpoints/rewind all keep working; the
870861
// message shows in the composer's history exactly as if typed.
871-
//
872-
// The confirmation toast lives here rather than at the submit site because
873-
// only this side knows whether the prompt was taken: with no usable
874-
// provider send() bounces it to the settings modal, and the producer would
875-
// otherwise claim success on a request that never left.
876-
// ponytail: one producer today (Auto-enhance), so the queued prompt and the
877-
// toast copy are assumed to be that one. A second producer needs the bus to
878-
// carry its own confirmation string.
862+
// The confirmation toast lives here because only this side knows the prompt
863+
// was taken — send() bounces it to the settings modal with no provider.
864+
// ponytail: one producer today, so the toast copy is assumed to be its own.
865+
// A second producer needs the bus to carry its confirmation string.
879866
const pendingPrompt = useChatPromptBus((s) => s.pending);
880867
const consumePrompt = useChatPromptBus((s) => s.consume);
881868
// biome-ignore lint/correctness/useExhaustiveDependencies: send() is intentionally not a dep (recreated each render); consume() clears `pending` so this fires once per queued prompt.

src/components/ai-edition/NewEditorShell.module.css

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2139,12 +2139,6 @@
21392139
}
21402140

21412141
.chatWelcomeIcon {
2142-
display: inline-grid;
2143-
place-items: center;
2144-
width: 40px;
2145-
height: 40px;
2146-
border-radius: 12px;
2147-
background: var(--accent-soft);
21482142
color: var(--accent);
21492143
margin-bottom: 4px;
21502144
}
@@ -2163,35 +2157,23 @@
21632157
max-width: 32ch;
21642158
}
21652159

2160+
/* Native list markers — no flex on the ul/li, or the items stop being
2161+
list-items and the markers vanish. padding-inline-start keeps them inside
2162+
the card in both writing directions. */
21662163
.chatWelcomeFeatures {
2167-
list-style: none;
2164+
/* explicit: the global reset sets list-style: none on every ul */
2165+
list-style: disc;
21682166
margin: 0;
21692167
padding: 10px 12px;
2170-
display: flex;
2171-
flex-direction: column;
2172-
gap: 6px;
2168+
padding-inline-start: 26px;
21732169
background: var(--surface);
21742170
border: 1px solid var(--border-soft);
21752171
border-radius: var(--r-md);
2176-
}
2177-
2178-
.chatWelcomeFeature {
2179-
display: flex;
2180-
align-items: flex-start;
2181-
gap: 8px;
21822172
font: 400 12px/1.45 var(--font-body);
21832173
color: var(--fg-2);
21842174
}
2185-
2186-
.chatWelcomeFeatureDot {
2187-
flex: 0 0 auto;
2188-
margin-top: 6px;
2189-
width: 5px;
2190-
height: 5px;
2191-
border-radius: 50%;
2192-
background: var(--accent);
2193-
opacity: 0.85;
2194-
}
2175+
.chatWelcomeFeatures li + li { margin-top: 6px; }
2176+
.chatWelcomeFeatures li::marker { color: var(--accent); }
21952177

21962178
.chatWelcomeCta {
21972179
display: inline-flex;
@@ -2206,7 +2188,7 @@
22062188
border: 1px solid var(--accent);
22072189
font: 600 12.5px var(--font-body);
22082190
cursor: pointer;
2209-
transition: filter 120ms ease, background 120ms ease;
2191+
transition: filter 120ms ease;
22102192
}
22112193
.chatWelcomeCta:hover { filter: brightness(0.95); }
22122194
.chatWelcomeCta:focus-visible {
@@ -2226,7 +2208,5 @@
22262208
.chatWelcomeDisclaimerIcon {
22272209
flex: 0 0 auto;
22282210
margin-top: 2px;
2229-
color: var(--muted);
22302211
opacity: 0.8;
22312212
}
2232-
Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,19 @@
11
// "Can the user actually send a chat message right now?"
22
//
3-
// Both inputs come from the `aiEdition.llmGetSnapshot` IPC: the selected
4-
// LLM config (may be null, or carry an empty-string provider after a
5-
// disconnect) and the list of providers that currently have valid
6-
// credentials. The chat composer is usable only when those two agree — a
7-
// stale config that points at a provider with no credentials should look
8-
// the same as "nothing set up yet" to the UI.
9-
//
10-
// This mirrors `runChat`'s own preflight in electron/ai-edition/chat-service.ts
11-
// (config present → provider known → credential resolves), so the composer is
12-
// disabled exactly when a send would have failed.
13-
//
14-
// Kept as a tiny pure function so the welcome-vs-composer gating in
15-
// <LeftPanel /> can be tested in isolation.
3+
// Mirrors runChat's preflight (electron/ai-edition/chat-service.ts), so the
4+
// composer is disabled exactly when a send would have failed.
165

176
import type { AiEditionLlmConfig } from "@/native/contracts";
187

198
export function canSendChat(
209
llmConfig: AiEditionLlmConfig | null,
2110
connectedProviders: string[] | null,
2211
): boolean {
23-
// null = the snapshot has not landed yet, which is "unknown", not "none".
24-
// Treating it as none flashed the welcome view at every user on every mount
25-
// and, since refreshLlm() swallows its errors, locked the panel behind an
26-
// undismissable welcome whenever the IPC call failed. Staying optimistic
27-
// degrades to the pre-welcome behaviour instead: type, send, get the real
28-
// error from the main process.
12+
// Snapshot not landed yet: unknown, not none. refreshLlm() swallows its
13+
// errors, so pessimism here would strand the panel behind the welcome view.
2914
if (connectedProviders === null) return true;
3015
if (llmConfig === null) return false;
16+
// llmDisconnect resets the active config to provider: "".
3117
if (llmConfig.provider === "") return false;
3218
return connectedProviders.includes(llmConfig.provider);
3319
}

0 commit comments

Comments
 (0)