Skip to content

Commit 026c101

Browse files
bminglesbmingles
authored
feat: Picker - Item description support (#1855)
- Normalized keys for item / section now remain undefined if not provided. Same for item `textValue` - Proper rendering of items with descriptions - Fixed tooltips for items with descriptions. Also changed default placement to `right` - isElementOfType util - Added SpectrumThemeProvider to Popper Supports deephaven/deephaven-plugins/issues/293
1 parent aa34ace commit 026c101

15 files changed

Lines changed: 234 additions & 86 deletions

File tree

‎packages/code-studio/src/styleguide/Pickers.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,23 @@ export function Pickers(): JSX.Element {
5757
<Text>Complex Ccc</Text>
5858
</Item>
5959
</Section>
60-
<Section>
60+
<Section key="Key B">
6161
<Item>Item Ddd</Item>
6262
<Item>Item Eee</Item>
6363
<Item textValue="Complex Fff">
6464
<PersonIcon />
6565
<Text>Complex Fff</Text>
6666
</Item>
67+
<Item key="Ggg">
68+
<PersonIcon />
69+
<Text>Label</Text>
70+
<Text slot="description">Description</Text>
71+
</Item>
72+
<Item key="Hhh">
73+
<PersonIcon />
74+
<Text>Label that causes overflow</Text>
75+
<Text slot="description">Description that causes overflow</Text>
76+
</Item>
6777
</Section>
6878
</Picker>
6979
</Flex>

‎packages/code-studio/src/styleguide/SamplesMenu.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export function SamplesMenu(): JSX.Element {
5656
);
5757

5858
const spectrumComparisonSamples = document.querySelector(
59-
`#${SPECTRUM_COMPARISON_SAMPLES_ID}`
59+
`#sample-section-${SPECTRUM_COMPARISON_SAMPLES_ID}`
6060
);
6161

6262
document

‎packages/code-studio/src/styleguide/StyleGuide.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ function StyleGuide(): React.ReactElement {
104104
<Buttons />
105105
<Progress />
106106
<Inputs />
107+
<Pickers />
107108
<ItemListInputs />
108109
<DraggableLists />
109110
<TimeSliderInputs />
@@ -112,7 +113,6 @@ function StyleGuide(): React.ReactElement {
112113
<ContextMenus />
113114
<DropdownMenus />
114115
<Navigations />
115-
<Pickers />
116116
<Tooltips />
117117
<Icons />
118118
<Editors />

‎packages/components/scss/BaseStyleSheet.scss‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@
88

99
color: var(--dh-color-text);
1010

11-
.spectrum-theme-provider,
12-
[class*='_spectrum--'] {
11+
.spectrum-theme-provider {
12+
// This is important for portals with rounded corners (e.g. Popover) so that
13+
// the underlying background color shows.
14+
--dh-spectrum-theme-provider-bg: unset;
15+
16+
background-color: var(--dh-spectrum-theme-provider-bg);
1317
color: var(--dh-color-text);
1418
}
1519
}

‎packages/components/src/popper/Popper.scss‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,39 +59,60 @@ $animation-offset: 10px;
5959
transition: all $transition;
6060
}
6161

62-
.popper-container[x-placement^='top'] > .popper-transition-enter,
63-
.popper-container[x-placement^='top'] > .popper-transition-exit {
62+
.popper-container[x-placement^='top']
63+
> .spectrum-theme-provider
64+
.popper-transition-enter,
65+
.popper-container[x-placement^='top']
66+
> .spectrum-theme-provider
67+
.popper-transition-exit {
6468
transform: scale($animation-scale) translate(0, $animation-offset);
6569
}
6670

67-
.popper-container[x-placement^='right'] > .popper-transition-enter,
68-
.popper-container[x-placement^='right'] > .popper-transition-exit {
71+
.popper-container[x-placement^='right']
72+
> .spectrum-theme-provider
73+
.popper-transition-enter,
74+
.popper-container[x-placement^='right']
75+
> .spectrum-theme-provider
76+
.popper-transition-exit {
6977
transform: scale($animation-scale) translate(-$animation-offset, 0);
7078
}
7179

72-
.popper-container[x-placement^='bottom'] > .popper-transition-enter,
73-
.popper-container[x-placement^='bottom'] > .popper-transition-exit {
80+
.popper-container[x-placement^='bottom']
81+
> .spectrum-theme-provider
82+
.popper-transition-enter,
83+
.popper-container[x-placement^='bottom']
84+
> .spectrum-theme-provider
85+
.popper-transition-exit {
7486
transform: scale($animation-scale) translate(0, -$animation-offset);
7587
}
7688

77-
.popper-container[x-placement^='left'] > .popper-transition-enter,
78-
.popper-container[x-placement^='left'] > .popper-transition-exit {
89+
.popper-container[x-placement^='left']
90+
> .spectrum-theme-provider
91+
.popper-transition-enter,
92+
.popper-container[x-placement^='left']
93+
> .spectrum-theme-provider
94+
.popper-transition-exit {
7995
transform: scale($animation-scale) translate($animation-offset, 0);
8096
}
8197

82-
.popper-container[x-placement^='top'] > .popper-transition-enter-active.popper,
98+
.popper-container[x-placement^='top']
99+
> .spectrum-theme-provider
100+
.popper-transition-enter-active.popper,
83101
.popper-container[x-placement^='right']
84-
> .popper-transition-enter-active.popper,
102+
> .spectrum-theme-provider
103+
.popper-transition-enter-active.popper,
85104
.popper-container[x-placement^='bottom']
86-
> .popper-transition-enter-active.popper,
105+
> .spectrum-theme-provider
106+
.popper-transition-enter-active.popper,
87107
.popper-container[x-placement^='left']
88-
> .popper-transition-enter-active.popper {
108+
> .spectrum-theme-provider
109+
.popper-transition-enter-active.popper {
89110
opacity: 1;
90111
transform: none;
91112
transition: all $transition ease-out;
92113
}
93114

94-
.popper-container[x-placement^='top'] > .popper {
115+
.popper-container[x-placement^='top'] > .spectrum-theme-provider .popper {
95116
margin-bottom: $arrow-width;
96117

97118
.popper-arrow {
@@ -105,7 +126,7 @@ $animation-offset: 10px;
105126
}
106127
}
107128

108-
.popper-container[x-placement^='bottom'] > .popper {
129+
.popper-container[x-placement^='bottom'] > .spectrum-theme-provider .popper {
109130
margin-top: $arrow-width;
110131

111132
.popper-arrow {
@@ -119,7 +140,7 @@ $animation-offset: 10px;
119140
}
120141
}
121142

122-
.popper-container[x-placement^='right'] > .popper {
143+
.popper-container[x-placement^='right'] > .spectrum-theme-provider .popper {
123144
margin-left: $arrow-width;
124145

125146
.popper-arrow {
@@ -133,7 +154,7 @@ $animation-offset: 10px;
133154
}
134155
}
135156

136-
.popper-container[x-placement^='left'] > .popper {
157+
.popper-container[x-placement^='left'] > .spectrum-theme-provider .popper {
137158
margin-right: $arrow-width;
138159

139160
.popper-arrow {

‎packages/components/src/popper/Popper.tsx‎

Lines changed: 45 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import PopperJs, { PopperOptions, ReferenceObject } from 'popper.js';
2323
import PropTypes from 'prop-types';
2424
import ThemeExport from '../ThemeExport';
2525
import './Popper.scss';
26+
import { SpectrumThemeProvider } from '../theme/SpectrumThemeProvider';
27+
28+
const POPPER_CLASS_NAME = 'popper';
2629

2730
interface PopperProps {
2831
options: PopperOptions;
@@ -166,12 +169,17 @@ class Popper extends Component<PopperProps, PopperState> {
166169
// delayed due to scheduleUpdate
167170
cancelAnimationFrame(this.rAF);
168171
this.rAF = window.requestAnimationFrame(() => {
169-
// for blur on close to work, focus needs to be on or within the popper
170-
if (closeOnBlur && !this.element.contains(document.activeElement)) {
171-
// only set focus, if a focus isn't already set within
172-
const elem = this.element.firstElementChild;
173-
if (elem instanceof HTMLElement) {
174-
elem.focus(); // first child of the portal element
172+
// If the current focus is not on the .popper or one of its descendants,
173+
// set the focus to the .popper element. This is necessary for close on
174+
// blur to work.
175+
if (closeOnBlur) {
176+
const popperEl = this.element.querySelector(`.${POPPER_CLASS_NAME}`);
177+
178+
if (
179+
popperEl instanceof HTMLElement &&
180+
!popperEl.contains(document.activeElement)
181+
) {
182+
popperEl.focus();
175183
}
176184
}
177185
});
@@ -244,33 +252,39 @@ class Popper extends Component<PopperProps, PopperState> {
244252
const { show } = this.state;
245253

246254
return (
247-
<CSSTransition
248-
in={show}
249-
timeout={timeout}
250-
classNames="popper-transition"
251-
onEntered={this.handleEnter}
252-
onExited={this.handleExit}
253-
>
254-
<div
255-
onClick={e => {
256-
// stop click events from escaping popper
257-
e.stopPropagation();
258-
}}
259-
onKeyDown={e => {
260-
if (e.key === 'Escape') this.hide();
261-
}}
262-
className={classNames('popper', { interactive }, className)}
263-
onBlur={closeOnBlur ? this.handleBlur : undefined}
264-
tabIndex={closeOnBlur ? -1 : undefined}
265-
role="presentation"
255+
<SpectrumThemeProvider isPortal>
256+
<CSSTransition
257+
in={show}
258+
timeout={timeout}
259+
classNames="popper-transition"
260+
onEntered={this.handleEnter}
261+
onExited={this.handleExit}
266262
>
267-
<div className="popper-content">
268-
{children}
269-
{/* eslint-disable-next-line react/no-unknown-property */}
270-
<div className="popper-arrow" x-arrow="" />
263+
<div
264+
onClick={e => {
265+
// stop click events from escaping popper
266+
e.stopPropagation();
267+
}}
268+
onKeyDown={e => {
269+
if (e.key === 'Escape') this.hide();
270+
}}
271+
className={classNames(
272+
POPPER_CLASS_NAME,
273+
{ interactive },
274+
className
275+
)}
276+
onBlur={closeOnBlur ? this.handleBlur : undefined}
277+
tabIndex={closeOnBlur ? -1 : undefined}
278+
role="presentation"
279+
>
280+
<div className="popper-content">
281+
{children}
282+
{/* eslint-disable-next-line react/no-unknown-property */}
283+
<div className="popper-arrow" x-arrow="" />
284+
</div>
271285
</div>
272-
</div>
273-
</CSSTransition>
286+
</CSSTransition>
287+
</SpectrumThemeProvider>
274288
);
275289
}
276290

‎packages/components/src/spectrum/picker/Picker.tsx‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { Key, useCallback, useMemo } from 'react';
2-
import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
1+
import { Key, ReactNode, useCallback, useMemo } from 'react';
2+
import { Flex, Picker as SpectrumPicker, Text } from '@adobe/react-spectrum';
3+
import { isElementOfType } from '@deephaven/react-hooks';
34
import cl from 'classnames';
45
import { Tooltip } from '../../popper';
56
import {
@@ -50,6 +51,26 @@ export type PickerProps = {
5051
| 'defaultSelectedKey'
5152
>;
5253

54+
/**
55+
* Create tooltip content optionally wrapping with a Flex column for array
56+
* content. This is needed for Items containing description `Text` elements.
57+
*/
58+
function createTooltipContent(content: ReactNode) {
59+
if (typeof content === 'boolean') {
60+
return String(content);
61+
}
62+
63+
if (Array.isArray(content)) {
64+
return (
65+
<Flex direction="column" alignItems="start">
66+
{content.filter(node => isElementOfType(node, Text))}
67+
</Flex>
68+
);
69+
}
70+
71+
return content;
72+
}
73+
5374
/**
5475
* Picker component for selecting items from a list of items. Items can be
5576
* provided via the `items` prop or as children. Each item can be a string,
@@ -84,11 +105,14 @@ export function Picker({
84105
// elements that back the Spectrum Picker. These are not visible in the UI,
85106
// but are used for accessibility purposes, so we set to an arbitrary
86107
// 'Empty' value so that they are not empty strings.
87-
<Item key={key as Key} textValue={textValue === '' ? 'Empty' : textValue}>
108+
<Item
109+
key={key as Key}
110+
textValue={textValue === '' || textValue == null ? 'Empty' : textValue}
111+
>
88112
<PickerItemContent>{content}</PickerItemContent>
89113
{tooltipOptions == null || content === '' ? null : (
90114
<Tooltip options={tooltipOptions}>
91-
{typeof content === 'boolean' ? String(content) : content}
115+
{createTooltipContent(content)}
92116
</Tooltip>
93117
)}
94118
</Item>

‎packages/components/src/spectrum/picker/PickerItemContent.tsx‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
import { isValidElement, ReactNode } from 'react';
1+
import { Children, cloneElement, isValidElement, ReactNode } from 'react';
22
import { Text } from '@adobe/react-spectrum';
3+
import cl from 'classnames';
4+
import { isElementOfType } from '@deephaven/react-hooks';
35
import stylesCommon from '../../SpectrumComponent.module.scss';
46

57
export interface PickerItemContentProps {
@@ -12,25 +14,46 @@ export interface PickerItemContentProps {
1214
*/
1315
export function PickerItemContent({
1416
children: content,
15-
}: PickerItemContentProps): JSX.Element {
17+
}: PickerItemContentProps): JSX.Element | null {
1618
if (isValidElement(content)) {
1719
return content;
1820
}
1921

22+
/* eslint-disable no-param-reassign */
2023
if (content === '') {
2124
// Prevent the item height from collapsing when the content is empty
22-
// eslint-disable-next-line no-param-reassign
23-
content = <>&nbsp;</>;
24-
}
25-
26-
if (typeof content === 'boolean') {
25+
content = '\xa0'; // Non-breaking space
26+
} else if (typeof content === 'boolean') {
2727
// Boolean values need to be stringified to render
28-
// eslint-disable-next-line no-param-reassign
2928
content = String(content);
29+
} else if (Array.isArray(content)) {
30+
// For cases where there are multiple `Text` children, add a css class to
31+
// handle overflow. The primary use case for multiple text nodes is when a
32+
// description is provided for an item. e.g.
33+
// <Item textValue="Some Text">
34+
// <SomeIcon />
35+
// <Text>Some Label</Text>
36+
// <Text slot="description">Some Description</Text>
37+
// </Item>
38+
content = Children.map(content, (el, i) =>
39+
isElementOfType(el, Text)
40+
? cloneElement(el, {
41+
...el.props,
42+
UNSAFE_className: cl(
43+
el.props.UNSAFE_className,
44+
stylesCommon.spectrumEllipsis
45+
),
46+
})
47+
: el
48+
);
3049
}
50+
/* eslint-enable no-param-reassign */
3151

32-
return (
52+
return typeof content === 'string' || typeof content === 'number' ? (
3353
<Text UNSAFE_className={stylesCommon.spectrumEllipsis}>{content}</Text>
54+
) : (
55+
// eslint-disable-next-line react/jsx-no-useless-fragment
56+
<>{content}</>
3457
);
3558
}
3659

0 commit comments

Comments
 (0)
⚡