fix: replace isDeepStrictEqual with navigation-aware options comparison
The select cursor highlight was broken because isDeepStrictEqual in use-select-navigation.ts and use-multi-select-state.ts would fail when options contained identity-unstable properties (JSX label elements, function onChange callbacks, computed disabled booleans). This caused the reset logic to fire on every re-render, resetting focusedValue back to the first option. Replace isDeepStrictEqual with optionsNavigateEqual which only compares properties that affect navigation behavior: value, disabled, and type. ReactNode labels and function callbacks are intentionally excluded as they are identity-unstable but don't change navigation semantics. Fixes #472
This commit is contained in:
@@ -1,5 +1,4 @@
|
|||||||
import { useCallback, useState } from 'react'
|
import { useCallback, useState } from 'react'
|
||||||
import { isDeepStrictEqual } from 'util'
|
|
||||||
import { useRegisterOverlay } from '../../context/overlayContext.js'
|
import { useRegisterOverlay } from '../../context/overlayContext.js'
|
||||||
import type { InputEvent } from '../../ink/events/input-event.js'
|
import type { InputEvent } from '../../ink/events/input-event.js'
|
||||||
// eslint-disable-next-line custom-rules/prefer-use-keybindings -- raw space/arrow multiselect input
|
// eslint-disable-next-line custom-rules/prefer-use-keybindings -- raw space/arrow multiselect input
|
||||||
@@ -9,6 +8,7 @@ import {
|
|||||||
normalizeFullWidthSpace,
|
normalizeFullWidthSpace,
|
||||||
} from '../../utils/stringUtils.js'
|
} from '../../utils/stringUtils.js'
|
||||||
import type { OptionWithDescription } from './select.js'
|
import type { OptionWithDescription } from './select.js'
|
||||||
|
import { optionsNavigateEqual } from './use-select-navigation.js'
|
||||||
import { useSelectNavigation } from './use-select-navigation.js'
|
import { useSelectNavigation } from './use-select-navigation.js'
|
||||||
|
|
||||||
export type UseMultiSelectStateProps<T> = {
|
export type UseMultiSelectStateProps<T> = {
|
||||||
@@ -174,7 +174,7 @@ export function useMultiSelectState<T>({
|
|||||||
// and the deleted ui/useMultiSelectState.ts — without this, MCPServerDesktopImportDialog
|
// and the deleted ui/useMultiSelectState.ts — without this, MCPServerDesktopImportDialog
|
||||||
// keeps colliding servers checked after getAllMcpConfigs() resolves.
|
// keeps colliding servers checked after getAllMcpConfigs() resolves.
|
||||||
const [lastOptions, setLastOptions] = useState(options)
|
const [lastOptions, setLastOptions] = useState(options)
|
||||||
if (options !== lastOptions && !isDeepStrictEqual(options, lastOptions)) {
|
if (options !== lastOptions && !optionsNavigateEqual(options, lastOptions)) {
|
||||||
setSelectedValues(defaultValue)
|
setSelectedValues(defaultValue)
|
||||||
setLastOptions(options)
|
setLastOptions(options)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,10 +6,34 @@ import {
|
|||||||
useRef,
|
useRef,
|
||||||
useState,
|
useState,
|
||||||
} from 'react'
|
} from 'react'
|
||||||
import { isDeepStrictEqual } from 'util'
|
|
||||||
import OptionMap from './option-map.js'
|
import OptionMap from './option-map.js'
|
||||||
import type { OptionWithDescription } from './select.js'
|
import type { OptionWithDescription } from './select.js'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Compare two option arrays for structural equality on properties that
|
||||||
|
* affect navigation behavior. ReactNode `label` and function `onChange`
|
||||||
|
* are intentionally excluded — they are identity-unstable (new reference
|
||||||
|
* each render) but don't change navigation semantics.
|
||||||
|
*/
|
||||||
|
export function optionsNavigateEqual<T>(
|
||||||
|
a: OptionWithDescription<T>[],
|
||||||
|
b: OptionWithDescription<T>[],
|
||||||
|
): boolean {
|
||||||
|
if (a.length !== b.length) return false
|
||||||
|
for (let i = 0; i < a.length; i++) {
|
||||||
|
const ao = a[i]!
|
||||||
|
const bo = b[i]!
|
||||||
|
if (
|
||||||
|
ao.value !== bo.value ||
|
||||||
|
ao.disabled !== bo.disabled ||
|
||||||
|
ao.type !== bo.type
|
||||||
|
) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
type State<T> = {
|
type State<T> = {
|
||||||
/**
|
/**
|
||||||
* Map where key is option's value and value is option's index.
|
* Map where key is option's value and value is option's index.
|
||||||
@@ -524,7 +548,7 @@ export function useSelectNavigation<T>({
|
|||||||
|
|
||||||
const [lastOptions, setLastOptions] = useState(options)
|
const [lastOptions, setLastOptions] = useState(options)
|
||||||
|
|
||||||
if (options !== lastOptions && !isDeepStrictEqual(options, lastOptions)) {
|
if (options !== lastOptions && !optionsNavigateEqual(options, lastOptions)) {
|
||||||
dispatch({
|
dispatch({
|
||||||
type: 'reset',
|
type: 'reset',
|
||||||
state: createDefaultState({
|
state: createDefaultState({
|
||||||
|
|||||||
Reference in New Issue
Block a user