Skip to content

Commit 13796c2

Browse files
committed
Fix empty custom field bug
1 parent 4e313a0 commit 13796c2

File tree

1 file changed

+42
-70
lines changed

1 file changed

+42
-70
lines changed

ai/ai-react-app/src/components/Layout/LeftSidebar.tsx

Lines changed: 42 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { useState, useEffect } from "react";
22
import { AppMode } from "../../App";
33
import styles from "./LeftSidebar.module.css";
4-
import { BackendType, ModelParams } from "firebase/ai";
4+
import { BackendType, Content, ModelParams } from "firebase/ai";
55
import { PREDEFINED_PERSONAS } from "../../config/personas";
66

77
interface LeftSidebarProps {
@@ -26,70 +26,17 @@ const LeftSidebar: React.FC<LeftSidebarProps> = ({
2626
generativeParams,
2727
setGenerativeParams,
2828
}) => {
29+
// This component now manages its own UI state and pushes updates upwards.
30+
// It does not rely on a useEffect to sync systemInstruction from the parent,
31+
// following the pattern in RightSidebar.tsx to prevent state-reversion bugs.
2932
const [selectedPersonaId, setSelectedPersonaId] = useState<string>("default");
3033
const [customPersona, setCustomPersona] = useState<string>("");
3134

32-
// Memoize the calculation of the new instruction text based on UI state.
33-
const newInstructionText = React.useMemo(() => {
34-
const selected = PREDEFINED_PERSONAS.find(
35-
(p) => p.id === selectedPersonaId,
36-
);
37-
if (!selected) return ""; // Should not happen, but a safe fallback.
38-
39-
return selected.id === "custom"
40-
? customPersona
41-
: selected.systemInstruction;
42-
}, [selectedPersonaId, customPersona]);
43-
44-
// Effect to sync UI changes (from dropdown or textarea) UP to the parent state.
45-
useEffect(() => {
46-
setGenerativeParams((prevParams) => {
47-
const currentInstructionText =
48-
prevParams.systemInstruction?.parts?.[0]?.text ?? "";
49-
50-
// Only update if the text content has actually changed to prevent re-renders.
51-
if (newInstructionText === currentInstructionText) {
52-
return prevParams;
53-
}
54-
55-
const newSystemInstruction = newInstructionText
56-
? { parts: [{ text: newInstructionText }] }
57-
: undefined;
58-
59-
return {
60-
...prevParams,
61-
systemInstruction: newSystemInstruction,
62-
};
63-
});
64-
}, [newInstructionText, setGenerativeParams]);
65-
66-
// Effect to sync parent state changes DOWN to the local UI state.
67-
// This ensures the UI reflects the state if it's changed elsewhere.
68-
useEffect(() => {
69-
const instructionText =
70-
generativeParams.systemInstruction?.parts?.[0]?.text ?? "";
71-
72-
const matchingPersona = PREDEFINED_PERSONAS.find(
73-
(p) => p.id !== "custom" && p.systemInstruction === instructionText,
74-
);
75-
76-
if (matchingPersona) {
77-
// A predefined persona matches the current instruction.
78-
setSelectedPersonaId(matchingPersona.id);
79-
setCustomPersona("");
80-
} else {
81-
// No predefined persona matches. It's either custom or the default empty state.
82-
if (instructionText) {
83-
// It's a custom persona.
84-
setSelectedPersonaId("custom");
85-
setCustomPersona(instructionText);
86-
} else {
87-
// It's the default empty state.
88-
setSelectedPersonaId("default");
89-
setCustomPersona("");
90-
}
91-
}
92-
}, [generativeParams.systemInstruction]);
35+
const handleModelParamsUpdate = (
36+
updateFn: (prevState: ModelParams) => ModelParams,
37+
) => {
38+
setGenerativeParams((prevState) => updateFn(prevState));
39+
};
9340

9441
// Define the available modes and their display names
9542
const modes: { id: AppMode; label: string }[] = [
@@ -102,22 +49,47 @@ const LeftSidebar: React.FC<LeftSidebarProps> = ({
10249
};
10350

10451
const handlePersonaChange = (e: React.ChangeEvent<HTMLSelectElement>) => {
105-
setSelectedPersonaId(e.target.value);
52+
const newPersonaId = e.target.value;
53+
setSelectedPersonaId(newPersonaId); // 1. Update UI state
54+
55+
let newSystemInstructionText: string;
56+
57+
if (newPersonaId === "custom") {
58+
// When switching to custom, the instruction is whatever is in the textarea.
59+
newSystemInstructionText = customPersona;
60+
} else {
61+
// When switching to a predefined persona, find its instruction text.
62+
const selected = PREDEFINED_PERSONAS.find((p) => p.id === newPersonaId);
63+
newSystemInstructionText = selected?.systemInstruction ?? "";
64+
// We are no longer in 'custom', but we don't clear the customPersona state
65+
// in case the user wants to switch back and forth.
66+
}
67+
68+
const newSystemInstruction: Content | undefined = newSystemInstructionText
69+
? { parts: [{ text: newSystemInstructionText }], role: "system" }
70+
: undefined;
71+
72+
// 2. Update model state upwards
73+
handleModelParamsUpdate((prev: ModelParams) => ({
74+
...prev,
75+
systemInstruction: newSystemInstruction,
76+
}));
10677
};
10778

10879
const handleCustomPersonaChange = (
10980
e: React.ChangeEvent<HTMLTextAreaElement>,
11081
) => {
11182
const newSystemInstructionText = e.target.value;
112-
setCustomPersona(newSystemInstructionText); // 1. update UI state
113-
const newSystemInstruction: Content = {
114-
parts: [{ text: newSystemInstructionText}],
115-
role: 'system'
116-
}
117-
// 2. Update model state upwards, triggering re-rendering
83+
setCustomPersona(newSystemInstructionText); // 1. Update UI state
84+
85+
const newSystemInstruction: Content | undefined = newSystemInstructionText
86+
? { parts: [{ text: newSystemInstructionText }], role: "system" }
87+
: undefined;
88+
89+
// 2. Update model state upwards
11890
handleModelParamsUpdate((prev: ModelParams) => ({
11991
...prev,
120-
systemInstruction: newSystemInstruction
92+
systemInstruction: newSystemInstruction,
12193
}));
12294
};
12395

0 commit comments

Comments
 (0)