fix: resolve RadioButtonSelect array bounds crash and auth dialog navigation (#46)

- Add bounds checking in RadioButtonSelect to prevent accessing undefined array elements
- Add useEffect to ensure activeIndex stays within valid bounds when items array changes
- Add validation guards around navigation handlers (up/down arrow keys)
- Fix AuthDialog initialAuthIndex calculation to prevent negative values from findIndex
- Ensure Enter key works properly on authentication screen

Fixes TypeError: Cannot read properties of undefined (reading 'value') that occurred
when activeIndex was out of bounds due to dynamic array changes or invalid initialization.

Signed-off-by: loheagn <loheagn@icloud.com>
Co-authored-by: linan.loheagn3 <linan.loheagn3@bytedance.com>
This commit is contained in:
Nan Li
2025-07-23 14:15:35 +08:00
committed by GitHub
parent e4a3f2656e
commit 173246723e
2 changed files with 30 additions and 10 deletions

View File

@@ -47,7 +47,7 @@ export function AuthDialog({
const [showOpenAIKeyPrompt, setShowOpenAIKeyPrompt] = useState(false); const [showOpenAIKeyPrompt, setShowOpenAIKeyPrompt] = useState(false);
const items = [{ label: 'OpenAI', value: AuthType.USE_OPENAI }]; const items = [{ label: 'OpenAI', value: AuthType.USE_OPENAI }];
const initialAuthIndex = items.findIndex((item) => { const initialAuthIndex = Math.max(0, items.findIndex((item) => {
if (settings.merged.selectedAuthType) { if (settings.merged.selectedAuthType) {
return item.value === settings.merged.selectedAuthType; return item.value === settings.merged.selectedAuthType;
} }
@@ -64,7 +64,7 @@ export function AuthDialog({
} }
return item.value === AuthType.LOGIN_WITH_GOOGLE; return item.value === AuthType.LOGIN_WITH_GOOGLE;
}); }));
const handleAuthSelect = (authMethod: AuthType) => { const handleAuthSelect = (authMethod: AuthType) => {
const error = validateAuthMethod(authMethod); const error = validateAuthMethod(authMethod);

View File

@@ -59,6 +59,15 @@ export function RadioButtonSelect<T>({
const [activeIndex, setActiveIndex] = useState(initialIndex); const [activeIndex, setActiveIndex] = useState(initialIndex);
const [scrollOffset, setScrollOffset] = useState(0); const [scrollOffset, setScrollOffset] = useState(0);
// Ensure activeIndex is always within bounds when items change
useEffect(() => {
if (items.length === 0) {
setActiveIndex(0);
} else if (activeIndex >= items.length) {
setActiveIndex(Math.max(0, items.length - 1));
}
}, [items.length, activeIndex]);
useEffect(() => { useEffect(() => {
const newScrollOffset = Math.max( const newScrollOffset = Math.max(
0, 0,
@@ -74,17 +83,28 @@ export function RadioButtonSelect<T>({
useInput( useInput(
(input, key) => { (input, key) => {
if (input === 'k' || key.upArrow) { if (input === 'k' || key.upArrow) {
const newIndex = activeIndex > 0 ? activeIndex - 1 : items.length - 1; if (items.length > 0) {
setActiveIndex(newIndex); const newIndex = activeIndex > 0 ? activeIndex - 1 : items.length - 1;
onHighlight?.(items[newIndex]!.value); setActiveIndex(newIndex);
if (items[newIndex]) {
onHighlight?.(items[newIndex].value);
}
}
} }
if (input === 'j' || key.downArrow) { if (input === 'j' || key.downArrow) {
const newIndex = activeIndex < items.length - 1 ? activeIndex + 1 : 0; if (items.length > 0) {
setActiveIndex(newIndex); const newIndex = activeIndex < items.length - 1 ? activeIndex + 1 : 0;
onHighlight?.(items[newIndex]!.value); setActiveIndex(newIndex);
if (items[newIndex]) {
onHighlight?.(items[newIndex].value);
}
}
} }
if (key.return) { if (key.return) {
onSelect(items[activeIndex]!.value); // Add bounds check before accessing items[activeIndex]
if (activeIndex >= 0 && activeIndex < items.length && items[activeIndex]) {
onSelect(items[activeIndex].value);
}
} }
// Enable selection directly from number keys. // Enable selection directly from number keys.
@@ -98,7 +118,7 @@ export function RadioButtonSelect<T>({
} }
} }
}, },
{ isActive: isFocused && items.length > 0 }, { isActive: isFocused && items.length > 0 && activeIndex >= 0 && activeIndex < items.length },
); );
const visibleItems = items.slice(scrollOffset, scrollOffset + maxItemsToShow); const visibleItems = items.slice(scrollOffset, scrollOffset + maxItemsToShow);