Skip to content

Commit

Permalink
refactor: replace html selects with Select component (#1637)
Browse files Browse the repository at this point in the history
Replaces all the places we used raw html select components with the
Select component.

This was orignally needed for themeing so we could re-style the Select
component, but we came up with an alternate solution.
  • Loading branch information
dsmmcken authored Nov 13, 2023
1 parent 724c84e commit 93eebff
Show file tree
Hide file tree
Showing 15 changed files with 195 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import memoize from 'memoizee';
import { CSSTransition, TransitionGroup } from 'react-transition-group';
import debounce from 'lodash.debounce';
import classNames from 'classnames';
import { Button, ThemeExport } from '@deephaven/components';
import { Button, Select, ThemeExport } from '@deephaven/components';
import {
DateTimeColumnFormatter,
IntegerColumnFormatter,
Expand Down Expand Up @@ -362,8 +362,8 @@ export class ColumnSpecificSectionContent extends PureComponent<
this.handleFormatRuleChange(i, 'columnName', e.target.value);
const onNameBlur = (): void =>
this.handleFormatRuleChange(i, 'isNewRule', false);
const onTypeChange = (e: ChangeEvent<HTMLSelectElement>): void =>
this.handleFormatRuleChange(i, 'columnType', e.target.value);
const onTypeChange = (eventTargetValue: string): void =>
this.handleFormatRuleChange(i, 'columnType', eventTargetValue);

const ruleError = this.getRuleError(rule);

Expand Down Expand Up @@ -396,14 +396,14 @@ export class ColumnSpecificSectionContent extends PureComponent<
/>

<label htmlFor={columnTypeId}>Column Type</label>
<select
<Select
id={columnTypeId}
className="custom-select"
value={rule.columnType}
onChange={onTypeChange}
>
{columnTypeOptions}
</select>
</Select>
</div>
</div>
<div className="form-row">
Expand Down Expand Up @@ -478,14 +478,14 @@ export class ColumnSpecificSectionContent extends PureComponent<

const value = format.formatString ?? '';
return (
<select
<Select
className={classNames('custom-select', { 'is-invalid': isInvalid })}
value={value}
id={formatId}
onChange={e => {
onChange={eventTargetValue => {
const selectedFormat = DateTimeColumnFormatter.makeFormat(
'',
e.target.value,
eventTargetValue,
DateTimeColumnFormatter.TYPE_GLOBAL
);
this.handleFormatRuleChange(i, 'format', selectedFormat);
Expand All @@ -499,7 +499,7 @@ export class ColumnSpecificSectionContent extends PureComponent<
showTimeZone,
showTSeparator
)}
</select>
</Select>
);
}

Expand Down
22 changes: 10 additions & 12 deletions packages/code-studio/src/settings/FormattingSectionContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { vsRefresh } from '@deephaven/icons';
import memoize from 'memoizee';
import debounce from 'lodash.debounce';
import classNames from 'classnames';
import { Button, Checkbox } from '@deephaven/components';
import { Button, Checkbox, Select } from '@deephaven/components';
import {
IntegerColumnFormatter,
DecimalColumnFormatter,
Expand Down Expand Up @@ -170,12 +170,10 @@ export class FormattingSectionContent extends PureComponent<
this.debouncedCommitChanges();
}

handleDefaultDateTimeFormatChange(
event: ChangeEvent<HTMLSelectElement>
): void {
log.debug('handleDefaultDateTimeFormatChange', event.target.value);
handleDefaultDateTimeFormatChange(value: string): void {
log.debug('handleDefaultDateTimeFormatChange', value);
const update = {
defaultDateTimeFormat: event.target.value,
defaultDateTimeFormat: value,
};
this.setState(update);
this.queueUpdate(update);
Expand Down Expand Up @@ -233,8 +231,8 @@ export class FormattingSectionContent extends PureComponent<
this.queueUpdate(update);
}

handleTimeZoneChange(event: ChangeEvent<HTMLSelectElement>): void {
const update = { timeZone: event.target.value };
handleTimeZoneChange(value: string): void {
const update = { timeZone: value };
this.setState(update);
this.queueUpdate(update);
}
Expand Down Expand Up @@ -364,14 +362,14 @@ export class FormattingSectionContent extends PureComponent<
Time zone
</label>
<div className="col pr-0">
<select
<Select
className="custom-select"
value={timeZone}
onChange={this.handleTimeZoneChange}
id="select-reset-timezone"
>
<TimeZoneOptions />
</select>
</Select>
</div>
<div className="col-1 btn-col">
<Button
Expand All @@ -393,7 +391,7 @@ export class FormattingSectionContent extends PureComponent<
DateTime
</label>
<div className="col pr-0">
<select
<Select
className="custom-select"
value={defaultDateTimeFormat}
onChange={this.handleDefaultDateTimeFormatChange}
Expand All @@ -406,7 +404,7 @@ export class FormattingSectionContent extends PureComponent<
true,
defaultDateTimeFormat
)}
</select>
</Select>
</div>
<div className="col-1 btn-col">
<Button
Expand Down
25 changes: 20 additions & 5 deletions packages/code-studio/src/styleguide/Inputs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,25 +235,38 @@ function Inputs(): React.ReactElement {
<div className="col">
<div className="form-group">
<h5>Selection Menu</h5>
<select defaultValue="0" className="custom-select">
<Select
onChange={v => {
// no-op
}}
defaultValue="0"
className="custom-select"
>
<option disabled value="0">
Custom Selection
</option>
<option value="1">One</option>
<option value="2">Two</option>
<option value="3">Three</option>
</select>
</Select>
</div>

<div className="form-group">
<select defaultValue="0" className="custom-select" disabled>
<Select
onChange={v => {
// no-op
}}
defaultValue="0"
className="custom-select"
disabled
>
<option disabled value="0">
Custom Selection
</option>
<option value="1">One</option>
<option value="2">Two</option>
<option value="3">Three</option>
</select>
</Select>
</div>

<div className="form-group">
Expand Down Expand Up @@ -308,7 +321,9 @@ function Inputs(): React.ReactElement {
labelText="Dropdown"
>
<Select
onChange={newValue => setValidateOption(newValue)}
onChange={eventTargetValue =>
setValidateOption(eventTargetValue)
}
value={validateOption}
>
{VALIDATE_OPTIONS.map(option => (
Expand Down
86 changes: 45 additions & 41 deletions packages/components/src/Select.tsx
Original file line number Diff line number Diff line change
@@ -1,50 +1,54 @@
import React, { useCallback } from 'react';
import classNames from 'classnames';
import { useForwardedRef } from '@deephaven/react-hooks';

export type SelectProps = {
children: React.ReactNode;
onBlur?: React.FocusEventHandler<HTMLSelectElement>;
type baseSelectProps = Omit<React.HTMLProps<HTMLSelectElement>, 'onChange'>;

export type SelectProps = baseSelectProps & {
onChange: (value: string) => void;
className?: string;
defaultValue?: string;
name?: string;
value?: string;
disabled?: boolean;
'data-testid'?: string;
};

function Select({
children,
onBlur,
onChange,
className,
defaultValue,
name,
value,
disabled,
'data-testid': dataTestId,
}: SelectProps): JSX.Element {
const handleChange = useCallback(
event => {
onChange(event.target.value);
},
[onChange]
);

return (
<select
className={classNames('custom-select', className)}
onBlur={onBlur}
onChange={handleChange}
defaultValue={defaultValue}
value={value}
name={name}
disabled={disabled}
data-testid={dataTestId}
>
{children}
</select>
);
}
/**
* A custom select component with styling, which is a wrapper around the
* native select element.
* @param props.onChange returns a string value and not the event
*/

const Select = React.forwardRef<HTMLSelectElement, SelectProps>(
(props, forwardedRef) => {
const {
children,
className,
onChange,
'data-testid': dataTestId,
...rest
} = props;

const ref = useForwardedRef<HTMLSelectElement>(forwardedRef);

const handleChange = useCallback(
(event: React.ChangeEvent<HTMLSelectElement>): void => {
onChange(event.target.value);
},
[onChange]
);

return (
<select
ref={ref}
className={classNames('custom-select', className)}
onChange={handleChange}
data-testid={dataTestId}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{children}
</select>
);
}
);

Select.displayName = 'Select';

export default Select;
10 changes: 5 additions & 5 deletions packages/console/src/csv/CsvInputBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import React, {
} from 'react';
import classNames from 'classnames';
import type { JSZipObject } from 'jszip';
import { Button, Checkbox } from '@deephaven/components';
import { Button, Checkbox, Select } from '@deephaven/components';
import type { IdeSession, Table } from '@deephaven/jsapi-types';
import Log from '@deephaven/log';
import { DbNameValidator } from '@deephaven/utils';
Expand Down Expand Up @@ -285,9 +285,9 @@ class CsvInputBar extends Component<CsvInputBarProps, CsvInputBarState> {
onClose();
}

handleQueryTypeChange(event: ChangeEvent<HTMLSelectElement>): void {
handleQueryTypeChange(eventTargetValue: string): void {
this.setState({
type: event.target.value as keyof typeof CsvFormats.TYPES,
type: eventTargetValue as keyof typeof CsvFormats.TYPES,
});
}

Expand Down Expand Up @@ -319,14 +319,14 @@ class CsvInputBar extends Component<CsvInputBarProps, CsvInputBarState> {
</div>
<div className="form-group">
<label htmlFor="formatSelect">File format</label>
<select
<Select
id="formatSelect"
className="custom-select"
value={type}
onChange={this.handleQueryTypeChange}
>
{TYPE_OPTIONS}
</select>
</Select>
</div>
<Checkbox
className="firstRowHeaders"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ describe('options when source is selected', () => {
onChange.mockClear();

// Enter key should send event immediately
fireEvent.keyPress(getValueSelect(), {
fireEvent.keyDown(getValueSelect(), {
key: 'Enter',
code: 'Enter',
charCode: 13,
Expand All @@ -356,7 +356,7 @@ describe('options when source is selected', () => {
});

it('doesnt fire an event when a key other than `Enter` is pressed', () => {
fireEvent.keyPress(getValueSelect(), {
fireEvent.keyDown(getValueSelect(), {
key: 'a',
code: 'KeyA',
charCode: 97,
Expand Down
Loading

0 comments on commit 93eebff

Please sign in to comment.