Skip to content

Commit

Permalink
[selection input] Don't reset selected index when dropdown results do…
Browse files Browse the repository at this point in the history
…n't change (#28109)

## Summary & Motivation

as titled.

## How I Tested These Changes



https://github.com/user-attachments/assets/14cd6219-0d3f-4cdc-9adf-752218b320ec
  • Loading branch information
salazarm authored Feb 28, 2025
1 parent 5d4264b commit 87716f9
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import {useEffect, useRef} from 'react';

export const usePrevious = <T,>(value: T): T | undefined => {
const ref = useRef<T>();
useEffect(() => {
ref.current = value;
}, [value]);
return ref.current;
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {SelectionAutoCompleteInputCSS} from './SelectionInputHighlighter';
import {useSelectionInputLintingAndHighlighting} from './useSelectionInputLintingAndHighlighting';
import {useTrackEvent} from '../app/analytics';
import {useDangerousRenderEffect} from '../hooks/useDangerousRenderEffect';
import {usePrevious} from '../hooks/usePrevious';
import {useUpdatingRef} from '../hooks/useUpdatingRef';

import 'codemirror/addon/edit/closebrackets';
Expand Down Expand Up @@ -83,9 +84,23 @@ export const SelectionAutoCompleteInput = ({

const [selectedIndexRef, setSelectedIndex] = useState({current: 0});

// Memoize the stringified results to avoid resetting the selected index down below
const resultsJson = useMemo(() => {
return JSON.stringify(autoCompleteResults?.list.map((l) => l.text));
}, [autoCompleteResults]);

const prevJson = usePrevious(resultsJson);
const prevAutoCompleteResults = usePrevious(autoCompleteResults);

// Handle selection reset
useDangerousRenderEffect(() => {
if (prevAutoCompleteResults?.from !== autoCompleteResults?.from || prevJson !== resultsJson) {
selectedIndexRef.current = 0;
}
}, [resultsJson, autoCompleteResults, prevAutoCompleteResults, prevJson, selectedIndexRef]);

// Handle hiding results
useDangerousRenderEffect(() => {
// Rather then using a useEffect + setState (extra render), we just set the current value directly
selectedIndexRef.current = 0;
if (!autoCompleteResults.list.length && !loading) {
showResults.current = false;
}
Expand Down

1 comment on commit 87716f9

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-t39o16sq8-elementl.vercel.app

Built with commit 87716f9.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.