Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Picker - Table support for key + label columns #1876

Merged
merged 40 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a8c773a
Use KeyedItem interface in Picker (#1858)
bmingles Mar 19, 2024
03191ce
Added `reuseItemsOnTableResize` param (#1858)
bmingles Mar 19, 2024
c65ebce
Support passing `NormalizedPickerItem[]` to Picker (#1858)
bmingles Mar 19, 2024
eafc98c
Tests (#1858)
bmingles Mar 20, 2024
3b4ed94
Updated comment (#1858)
bmingles Mar 20, 2024
bb7d4ba
Added item key prefix (#1858)
bmingles Mar 20, 2024
5a3d852
Scroll area utils (#1858)
bmingles Mar 20, 2024
f37d2f2
Windowed scrolling (#1858)
bmingles Mar 20, 2024
5ac92e2
Scroll when popover opens (#1858)
bmingles Mar 21, 2024
e063389
Simplified scroll to position (#1858)
bmingles Mar 21, 2024
69afe60
JS api Picker component (#1858)
bmingles Mar 21, 2024
d668beb
Fixed a bug with selectedKey types always being strings (#1858)
bmingles Mar 21, 2024
1a4f98c
Updated e2e snapshots (#1858)
bmingles Mar 21, 2024
920ae45
Fixed tests (#1858)
bmingles Mar 21, 2024
e7b112d
selectedKey + defaultSelectedKey support (#1858)
bmingles Mar 21, 2024
31c7d2e
Updated e2e snapshots (#1858)
bmingles Mar 21, 2024
aa840a4
Fixed tests (#1858)
bmingles Mar 22, 2024
539a0fe
Remove top-level key (#1858)
bmingles Mar 22, 2024
af14c98
Tests (#1858)
bmingles Mar 22, 2024
9b961dd
Cleanup (#1858)
bmingles Mar 22, 2024
89952ad
Tests (#1858)
bmingles Mar 22, 2024
2f9a5fa
Label (#1858)
bmingles Mar 22, 2024
8323201
getPositionOfSelectedItem (#1858)
bmingles Mar 22, 2024
2b94503
usePopoverOnScrollRef tests (#1858)
bmingles Mar 22, 2024
613fc6c
label (#1858)
bmingles Mar 22, 2024
a94aeb7
Comment (#1858)
bmingles Mar 22, 2024
491c275
Comment (#1858)
bmingles Mar 22, 2024
40b08dd
PickerUtils tests (#1858)
bmingles Mar 22, 2024
a58678b
Fixed scroll to item on subsequent popover opens (#1858)
bmingles Mar 22, 2024
275378a
getValueType tests (#1858)
bmingles Mar 22, 2024
8f58c19
useGetItemIndexByValue test (#1858)
bmingles Mar 25, 2024
11d0750
test label (#1858)
bmingles Mar 25, 2024
d3327f5
Fixed boolean mapping (#1858)
bmingles Mar 25, 2024
4d117a8
Re-arranged test (#1858)
bmingles Mar 25, 2024
982c839
Removed children prop (#1858)
bmingles Mar 25, 2024
39d57d2
Default Formatter (#1858)
bmingles Mar 26, 2024
fad37c9
Tweaked test (#1858)
bmingles Mar 26, 2024
290d2ff
Addressed review comments (#1858)
bmingles Mar 27, 2024
435b095
Added catch for promise (#1858)
bmingles Mar 27, 2024
4b70ed0
Logger namespace (#1858)
bmingles Mar 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions __mocks__/dh-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1989,6 +1989,15 @@ class SourceType {
static Z = 'Z';
}

class ValueType {
static BOOLEAN = 'BOOLEAN';
static DATETIME = 'DATETIME';
static DOUBLE = 'DOUBLE';
static LONG = 'LONG';
static NUMBER = 'NUMBER';
static STRING = 'STRING';
}

const dh = {
FilterCondition: FilterCondition,
FilterValue: FilterValue,
Expand Down Expand Up @@ -2034,6 +2043,7 @@ const dh = {
DayOfWeek,
},
DateWrapper: DateWrapper,
ValueType,
ViewportData,
VariableType,
storage: {
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

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

30 changes: 26 additions & 4 deletions packages/code-studio/src/styleguide/Pickers.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import React from 'react';
import { Item, Picker, Section } from '@deephaven/components';
import React, { useCallback, useState } from 'react';
import { Picker, PickerItemKey, Section } from '@deephaven/components';
import { vsPerson } from '@deephaven/icons';
import { Flex, Icon, Text } from '@adobe/react-spectrum';
import { Flex, Icon, Item, Text } from '@adobe/react-spectrum';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { sampleSectionIdAndClasses } from './utils';

// Generate enough items to require scrolling
const items = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'
.split('')
.map((key, i) => ({
key,
item: { key: (i + 1) * 100, content: `${key}${key}${key}` },
}));

function PersonIcon(): JSX.Element {
return (
<Icon>
Expand All @@ -14,6 +22,12 @@ function PersonIcon(): JSX.Element {
}

export function Pickers(): JSX.Element {
const [selectedKey, setSelectedKey] = useState<PickerItemKey>();

const onChange = useCallback((key: PickerItemKey): void => {
setSelectedKey(key);
}, []);

return (
// eslint-disable-next-line react/jsx-props-no-spreading
<div {...sampleSectionIdAndClasses('pickers')}>
Expand All @@ -24,7 +38,7 @@ export function Pickers(): JSX.Element {
<Item>Aaa</Item>
</Picker>

<Picker label="Mixed Children Types" tooltip>
<Picker label="Mixed Children Types" defaultSelectedKey={999} tooltip>
{/* eslint-disable react/jsx-curly-brace-presence */}
{'String 1'}
{'String 2'}
Expand Down Expand Up @@ -76,6 +90,14 @@ export function Pickers(): JSX.Element {
</Item>
</Section>
</Picker>

<Picker
label="Controlled"
selectedKey={selectedKey}
onChange={onChange}
>
{items}
</Picker>
</Flex>
</div>
);
Expand Down
156 changes: 122 additions & 34 deletions packages/components/src/spectrum/picker/Picker.tsx
Original file line number Diff line number Diff line change
@@ -1,36 +1,57 @@
import { Key, ReactNode, useCallback, useMemo } from 'react';
import { DOMRef } from '@react-types/shared';
import { Flex, Picker as SpectrumPicker, Text } from '@adobe/react-spectrum';
import { isElementOfType } from '@deephaven/react-hooks';
import {
getPositionOfSelectedItem,
findSpectrumPickerScrollArea,
isElementOfType,
usePopoverOnScrollRef,
} from '@deephaven/react-hooks';
import {
EMPTY_FUNCTION,
PICKER_ITEM_HEIGHT,
PICKER_TOP_OFFSET,
} from '@deephaven/utils';
import cl from 'classnames';
import { Tooltip } from '../../popper';
import {
isNormalizedPickerSection,
NormalizedSpectrumPickerProps,
normalizePickerItemList,
normalizeTooltipOptions,
NormalizedPickerItem,
PickerItemOrSection,
PickerItemKey,
TooltipOptions,
NormalizedPickerItem,
isNormalizedPickerSection,
PickerItemKey,
getPickerItemKey,
} from './PickerUtils';
import { PickerItemContent } from './PickerItemContent';
import { Item, Section } from '../shared';

export type PickerProps = {
children: PickerItemOrSection | PickerItemOrSection[];
children:
| PickerItemOrSection
| PickerItemOrSection[]
| NormalizedPickerItem[];
/** Can be set to true or a TooltipOptions to enable item tooltips */
tooltip?: boolean | TooltipOptions;
/** The currently selected key in the collection (controlled). */
selectedKey?: PickerItemKey | null;
/** The initial selected key in the collection (uncontrolled). */
defaultSelectedKey?: PickerItemKey;
/** Function to retrieve initial scroll position when opening the picker */
getInitialScrollPosition?: () => Promise<number | null>;
/**
* Handler that is called when the selection change.
* Note that under the hood, this is just an alias for Spectrum's
* `onSelectionChange`. We are renaming for better consistency with other
* components.
*/
onChange?: (key: PickerItemKey) => void;

/** Handler that is called when the picker is scrolled. */
onScroll?: (event: Event) => void;

/**
* Handler that is called when the selection changes.
* @deprecated Use `onChange` instead
Expand Down Expand Up @@ -82,7 +103,10 @@ export function Picker({
tooltip = true,
defaultSelectedKey,
selectedKey,
getInitialScrollPosition,
onChange,
onOpenChange,
onScroll = EMPTY_FUNCTION,
onSelectionChange,
// eslint-disable-next-line camelcase
UNSAFE_className,
Expand All @@ -99,52 +123,116 @@ export function Picker({
);

const renderItem = useCallback(
({ key, content, textValue }: NormalizedPickerItem) => (
// The `textValue` prop gets used to provide the content of `<option>`
// elements that back the Spectrum Picker. These are not visible in the UI,
// but are used for accessibility purposes, so we set to an arbitrary
// 'Empty' value so that they are not empty strings.
<Item
key={key as Key}
textValue={textValue === '' || textValue == null ? 'Empty' : textValue}
>
<PickerItemContent>{content}</PickerItemContent>
{tooltipOptions == null || content === '' ? null : (
<Tooltip options={tooltipOptions}>
{createTooltipContent(content)}
</Tooltip>
)}
</Item>
),
(normalizedItem: NormalizedPickerItem) => {
const key = getPickerItemKey(normalizedItem);
const content = normalizedItem.item?.content ?? '';
const textValue = normalizedItem.item?.textValue ?? '';

return (
<Item
// Note that setting the `key` prop explicitly on `Item` elements
// causes the picker to expect `selectedKey` and `defaultSelectedKey`
// to be strings. It also passes the stringified value of the key to
// `onSelectionChange` handlers` regardless of the actual type of the
// key. We can't really get around setting in order to support Windowed
// data, so we'll need to do some manual conversion of keys to strings
// in other places of this component.
key={key as Key}
// The `textValue` prop gets used to provide the content of `<option>`
// elements that back the Spectrum Picker. These are not visible in the UI,
// but are used for accessibility purposes, so we set to an arbitrary
// 'Empty' value so that they are not empty strings.
textValue={textValue === '' ? 'Empty' : textValue}
>
<>
<PickerItemContent>{content}</PickerItemContent>
{tooltipOptions == null || content === '' ? null : (
<Tooltip options={tooltipOptions}>
{createTooltipContent(content)}
</Tooltip>
)}
</>
</Item>
);
},
[tooltipOptions]
);

const getInitialScrollPositionInternal = useCallback(
() =>
getInitialScrollPosition == null
? getPositionOfSelectedItem({
keyedItems: normalizedItems,
// TODO: #1890 & deephaven-plugins#371 add support for sections and
// items with descriptions since they impact the height calculations
itemHeight: PICKER_ITEM_HEIGHT,
selectedKey,
topOffset: PICKER_TOP_OFFSET,
})
: getInitialScrollPosition(),
[getInitialScrollPosition, normalizedItems, selectedKey]
);

const { ref: scrollRef, onOpenChange: popoverOnOpenChange } =
usePopoverOnScrollRef(
findSpectrumPickerScrollArea,
onScroll,
getInitialScrollPositionInternal
);

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

onOpenChange?.(isOpen);
},
[onOpenChange, popoverOnOpenChange]
);

const onSelectionChangeInternal = useCallback(
(key: PickerItemKey): void => {
// The `key` arg will always be a string due to us setting the `Item` key
// prop in `renderItem`. We need to find the matching item to determine
// the actual key.
const selectedItem = normalizedItems.find(
item => String(getPickerItemKey(item)) === key
);

const actualKey = getPickerItemKey(selectedItem) ?? key;

(onChange ?? onSelectionChange)?.(actualKey);
},
[normalizedItems, onChange, onSelectionChange]
);

return (
<SpectrumPicker
// eslint-disable-next-line react/jsx-props-no-spreading
{...spectrumPickerProps}
// The `ref` prop type defined by React Spectrum is incorrect here
ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
onOpenChange={onOpenChangeInternal}
UNSAFE_className={cl('dh-picker', UNSAFE_className)}
items={normalizedItems}
// Type assertions are necessary for `selectedKey`, `defaultSelectedKey`,
// and `onSelectionChange` due to Spectrum types not accounting for
// `boolean` keys
selectedKey={selectedKey as NormalizedSpectrumPickerProps['selectedKey']}
defaultSelectedKey={
defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey']
}
// Spectrum Picker treats keys as strings if the `key` prop is explicitly
// set on `Item` elements. Since we do this in `renderItem`, we need to
// ensure that `selectedKey` and `defaultSelectedKey` are strings in order
// for selection to work.
selectedKey={selectedKey?.toString()}
defaultSelectedKey={defaultSelectedKey?.toString()}
// `onChange` is just an alias for `onSelectionChange`
onSelectionChange={
(onChange ??
onSelectionChange) as NormalizedSpectrumPickerProps['onSelectionChange']
onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
}
>
{itemOrSection => {
if (isNormalizedPickerSection(itemOrSection)) {
return (
<Section
key={itemOrSection.key}
title={itemOrSection.title}
items={itemOrSection.items}
key={getPickerItemKey(itemOrSection)}
title={itemOrSection.item?.title}
items={itemOrSection.item?.items}
>
{renderItem}
</Section>
Expand Down
Loading
Loading