Skip to content

Commit

Permalink
fix: ComboBox show all items on open (#2328)
Browse files Browse the repository at this point in the history
DH-18088 & DH-18087: ComboBox now clears search filter when opening the
ComboBox unless it is triggered by user input.

### Testing
- Typing in a combobox should open it filtered by the search text
- Clicking the dropdown should open the CombBox unfiltered
- Typing while open should filter results
> Note that there seems to be a Spectrum bug that can cause some weird
scrolling behavior when typing in an opened ComboBox
adobe/react-spectrum#7573

#### menu_trigger="focus"
- Clicking search input should open it unfiltered

Here's a script with a few different configurations

```python
import deephaven.ui as ui
from deephaven import time_table
import datetime

# Ticking table with initial row count of 200 that adds a row every second
initial_row_count = 200
_table = time_table(
    "PT1S",
    start_time=datetime.datetime.now() - datetime.timedelta(seconds=initial_row_count),
).update(
    [
        "Int=new Integer(i)",
        "Text=new String(`Display `+i)",
    ]
)

item_list = [ui.item(f"Display {i}") for i in range(1, 201)]


# Basic ComboBox
@ui.component
def ui_combo_box_basic():
    value, set_value = ui.use_state("Display 91")

    return ui.combo_box(
        item_list,
        label=f"Basic ({value})",
        selected_key=value,
        on_change=set_value,
        width="size-3000"
    )


# Uncontrolled ComboBox (Table source)
@ui.component
def ui_combo_box_uncontrolled(table):
    value, set_value = ui.use_state("")

    combo1 = ui.combo_box(
        ui.item_table_source(table, key_column="Text", label_column="Text"),
        default_selected_key="Display 92",
        label=f"Uncontrolled Table Source ({value or 'None'})",
        on_change=set_value,
        width="size-3000"
    )

    return combo1


# Controlled ComboBox (Table source)
@ui.component
def ui_combo_box_controlled(table, menu_trigger):
    value, set_value = ui.use_state("Display 93")

    combo1 = ui.combo_box(
        ui.item_table_source(table, key_column="Text", label_column="Text"),
        selected_key=value,
        label=f"Controlled Table Source ({value}) {menu_trigger or ''}",
        menu_trigger=menu_trigger,
        on_change=set_value,
        width="size-3000"
    )

    btn = ui.button("Set Value", on_press=lambda: set_value("Display 104"))

    return combo1, btn


# Controlled input ComboBox (Table source)
@ui.component
def ui_combo_box_input_controlled(table, menu_trigger):
    input_value, set_input_value = ui.use_state("Display 94")
    value, set_value = ui.use_state("Display 94")

    combo1 = ui.combo_box(
        ui.item_table_source(table, key_column="Text", label_column="Text"),
        input_value=input_value,
        on_input_change=set_input_value,
        default_selected_key=value,
        label=f"Controlled Input Table Source ({value}) {menu_trigger or ''}",
        menu_trigger=menu_trigger,
        on_change=set_value,
        width="size-3000"
    )

    btn = ui.button("Set Input", on_press=lambda: set_input_value("Display 104"))

    return combo1, btn

# Layout
@ui.component
def ui_layout():
    return (
        ui_combo_box_basic(),
        ui_combo_box_uncontrolled(_table), 
        ui_combo_box_controlled(_table, None), 
        ui_combo_box_controlled(_table, "focus"),
        ui_combo_box_input_controlled(_table, None),
    )

tests = ui_layout()
```
  • Loading branch information
bmingles authored Jan 9, 2025
1 parent 3608b15 commit c08bb7b
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 12 deletions.
2 changes: 2 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@react-spectrum/theme-default": "^3.5.1",
"@react-spectrum/toast": "^3.0.0-beta.16",
"@react-spectrum/utils": "^3.11.5",
"@react-types/combobox": "3.13.1",
"@react-types/radio": "^3.8.1",
"@react-types/shared": "^3.22.1",
"@react-types/textfield": "^3.9.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/spectrum/comboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { NormalizedItem } from '../utils';
import { type PickerPropsT, usePickerProps } from '../picker';

export type ComboBoxProps = PickerPropsT<SpectrumComboBoxProps<NormalizedItem>>;

export { type MenuTriggerAction } from '@react-types/combobox';
export { SpectrumComboBox };

export const ComboBox = React.forwardRef(function ComboBox(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,14 @@ describe('usePickerScrollOnOpen', () => {
expect(result.current.ref).toBe(mockUsePopoverOnScrollRefResult.ref);
});

it.each([true, false])(
'should return a callback that calls popoverOnOpenChange and onOpenChange: %s',
isOpen => {
it.each([
[true, undefined],
[false, undefined],
[true, 'input'],
[false, 'input'],
] as const)(
'should return a callback that calls popoverOnOpenChange and onOpenChange: %s, %s',
(isOpen, menuTrigger) => {
const { result } = renderHook(() =>
usePickerScrollOnOpen({
getInitialScrollPosition,
Expand All @@ -60,12 +65,12 @@ describe('usePickerScrollOnOpen', () => {
})
);

result.current.onOpenChange(isOpen);
result.current.onOpenChange(isOpen, menuTrigger);

expect(mockUsePopoverOnScrollRefResult.onOpenChange).toHaveBeenCalledWith(
isOpen
);
expect(onOpenChange).toHaveBeenCalledWith(isOpen);
expect(onOpenChange).toHaveBeenCalledWith(isOpen, menuTrigger);
}
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@ import {
findSpectrumPickerScrollArea,
usePopoverOnScrollRef,
} from '@deephaven/react-hooks';
import type { MenuTriggerAction } from '../comboBox';

export interface UsePickerScrollOnOpenOptions {
getInitialScrollPosition?: () => Promise<number | null | undefined>;
onScroll: (event: Event) => void;
onOpenChange?: (isOpen: boolean) => void;
onOpenChange?: (isOpen: boolean, menuTrigger?: MenuTriggerAction) => void;
}

export interface UsePickerScrollOnOpenResult<THtml extends HTMLElement> {
ref: DOMRef<THtml>;
onOpenChange: (isOpen: boolean) => void;
onOpenChange: (isOpen: boolean, menuTrigger?: MenuTriggerAction) => void;
}

/**
Expand All @@ -37,11 +38,11 @@ export function usePickerScrollOnOpen<THtml extends HTMLElement = HTMLElement>({
);

const onOpenChangeInternal = useCallback(
(isOpen: boolean): void => {
(isOpen: boolean, menuTrigger?: MenuTriggerAction): void => {
// Attach scroll event handling
popoverOnOpenChange(isOpen);

onOpenChange?.(isOpen);
onOpenChange?.(isOpen, menuTrigger);
},
[onOpenChange, popoverOnOpenChange]
);
Expand Down
43 changes: 41 additions & 2 deletions packages/jsapi-components/src/spectrum/ComboBox.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {
ComboBoxNormalized,
type MenuTriggerAction,
type NormalizedItem,
type SpectrumComboBoxProps,
} from '@deephaven/components';
import { useCallback } from 'react';
import { useCallback, useRef } from 'react';
import { type PickerWithTableProps } from './PickerProps';
import { usePickerProps } from './utils';

Expand All @@ -18,19 +19,57 @@ export function ComboBox(props: ComboBoxProps): JSX.Element {
...pickerProps
} = usePickerProps<ComboBoxProps>(props);

const isOpenRef = useRef(false);
const inputValueRef = useRef('');

const onInputChange = useCallback(
(value: string) => {
onInputChangeInternal?.(value);
onSearchTextChange(value);

// Only apply search text if ComboBox is open.
if (isOpenRef.current) {
onSearchTextChange(value);
}
// When the ComboBox is closed, `onInputChange` may have been called as a
// result of user search input, ComboBox selection, or by selected key
// prop changes. We can't determine the source here, so we clear the search
// text and store the search value so that the list is unfiltered the next
// time the ComboBox is opened. We also store the search value so we can
// re-apply it in `onOpenChange` if the ComboBox is opened by user search
// input.
else {
onSearchTextChange('');
inputValueRef.current = value;
}
},
[onInputChangeInternal, onSearchTextChange]
);

const onOpenChange = useCallback(
(isOpen: boolean, menuTrigger?: MenuTriggerAction) => {
pickerProps.onOpenChange?.(isOpen);

// Reset the search text when the ComboBox is closed.
if (!isOpen) {
onSearchTextChange('');
}
// Restore search text when ComboBox has been opened by user input.
else if (menuTrigger === 'input') {
onSearchTextChange(inputValueRef.current);
}

// Store the open state so that `onInputChange` has access to it.
isOpenRef.current = isOpen;
},
[onSearchTextChange, pickerProps]
);

return (
<ComboBoxNormalized
// eslint-disable-next-line react/jsx-props-no-spreading
{...pickerProps}
onInputChange={onInputChange}
onOpenChange={onOpenChange}
/>
);
}
Expand Down

0 comments on commit c08bb7b

Please sign in to comment.