Fix selection behavior (#341)

#341
pinterest · Dec 8, 2020 · ea94821 · ea94821
1 parent 94b5862
commit ea94821
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 14 deletions.
diff --git a/datahub/webapp/__tests__/ui/Select.test.js b/datahub/webapp/__tests__/ui/Select.test.js
@@ -1,13 +1,52 @@
 import React from 'react';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
 import toJson from 'enzyme-to-json';
 
-import { Select } from '../../ui/Select/Select';
+import { Select, makeSelectOptions } from '../../ui/Select/Select';
 
 it('renders without crashing', () => {
     shallow(<Select value="test" onChange={() => null} />);
 });
 
+describe('Select behavior', () => {
+    it('selects the correct field', () => {
+        const onChangeMock = jest.fn();
+        const wrapper = mount(
+            <Select
+                value="test"
+                onChange={(event) => onChangeMock(event.target.value)}
+            >
+                {makeSelectOptions(['test', 'test1', 'test2'])}
+            </Select>
+        );
+        wrapper.find('select').instance().selectedIndex = 2;
+        wrapper.find('select').simulate('change');
+
+        expect(wrapper.find('option')).toHaveLength(3);
+        expect(onChangeMock).toHaveBeenCalledTimes(1);
+        expect(onChangeMock).toHaveBeenCalledWith('test2');
+    });
+
+    it('deselects correctly', () => {
+        const onChangeMock = jest.fn();
+        const wrapper = mount(
+            <Select
+                value="test"
+                onChange={(event) => onChangeMock(event.target.value)}
+                withDeselect
+            >
+                {makeSelectOptions(['test', 'test1', 'test2'])}
+            </Select>
+        );
+        wrapper.find('select').instance().selectedIndex = 0;
+        wrapper.find('select').simulate('change');
+
+        expect(wrapper.find('option')).toHaveLength(4);
+        expect(onChangeMock).toHaveBeenCalledTimes(1);
+        expect(onChangeMock).toHaveBeenCalledWith('');
+    });
+});
+
 describe('matches enzyme snapshots', () => {
     it('matches snapshot', () => {
         const wrapper = shallow(<Select value="test" onChange={() => null} />);

diff --git a/datahub/webapp/components/AppAdmin/AdminQueryEngine.tsx b/datahub/webapp/components/AppAdmin/AdminQueryEngine.tsx
@@ -121,7 +121,7 @@ export const AdminQueryEngine: React.FunctionComponent<IProps> = ({
                 language: queryEngine.language,
                 executor: queryEngine.executor,
                 executor_params: queryEngine.executor_params,
-                metastore_id: queryEngine.metastore_id,
+                metastore_id: queryEngine.metastore_id ?? null,
                 status_checker: queryEngine.status_checker,
             });
 
@@ -290,6 +290,12 @@ export const AdminQueryEngine: React.FunctionComponent<IProps> = ({
                                         value: metastore.name,
                                     })
                                 )}
+                                onChange={(value) => {
+                                    onChange(
+                                        'metastore_id',
+                                        value !== '' ? value : null
+                                    );
+                                }}
                                 withDeselect
                             />
                             <SimpleField

diff --git a/datahub/webapp/components/DataTableTags/TableTagSelect.tsx b/datahub/webapp/components/DataTableTags/TableTagSelect.tsx
@@ -84,6 +84,7 @@ export const TableTagSelect: React.FunctionComponent<IProps> = ({
                         }
                     },
                 }}
+                clearAfterSelect
             />
         </div>
     );

diff --git a/datahub/webapp/ui/Select/Select.tsx b/datahub/webapp/ui/Select/Select.tsx
@@ -2,6 +2,7 @@ import React from 'react';
 import classNames from 'classnames';
 import './Select.scss';
 
+export const DESELECT_VALUE = '';
 export interface ISelectProps {
     name?: string;
     disabled?: boolean;
@@ -12,25 +13,23 @@ export interface ISelectProps {
     transparent?: boolean;
 
     withDeselect?: boolean;
-    deselectValue?: string;
 }
 
 export const Select: React.FunctionComponent<ISelectProps> = ({
     name,
     disabled,
     value,
     onChange,
-    className,
+    className = '',
     children,
 
     fullWidth,
     transparent,
 
     withDeselect,
-    deselectValue,
 }) => {
     const deselectOption = withDeselect ? (
-        <option value={deselectValue} key="">
+        <option value={DESELECT_VALUE} key="">
             {'Deselect'}
         </option>
     ) : null;
@@ -39,7 +38,7 @@ export const Select: React.FunctionComponent<ISelectProps> = ({
         <select
             name={name}
             disabled={disabled || false}
-            value={value ?? deselectValue}
+            value={value ?? DESELECT_VALUE}
             onChange={onChange}
         >
             {deselectOption}
@@ -65,11 +64,6 @@ export const Select: React.FunctionComponent<ISelectProps> = ({
     );
 };
 
-Select.defaultProps = {
-    className: '',
-    deselectValue: '',
-};
-
 export type IOptions = Array<
     | {
           key: any;

diff --git a/datahub/webapp/ui/SimpleReactSelect/SimpleReactSelect.tsx b/datahub/webapp/ui/SimpleReactSelect/SimpleReactSelect.tsx
@@ -18,6 +18,9 @@ export interface ISimpleReactSelectProps<T> {
     withDeselect?: boolean;
     isDisabled?: boolean;
     selectProps?: Partial<ReactSelectProps<T>>;
+
+    // Clear selection user picks value
+    clearAfterSelect?: boolean;
 }
 
 const reactSelectStyle = makeReactSelectStyle(true);
@@ -26,10 +29,21 @@ export function SimpleReactSelect<T>({
     options,
     value,
     onChange,
-    withDeselect = false,
     isDisabled,
+
     selectProps = {},
+    withDeselect = false,
+    clearAfterSelect = false,
 }: ISimpleReactSelectProps<T>) {
+    const overrideSelectProps = useMemo(() => {
+        const override: Partial<ReactSelectProps<T>> = {};
+        if (clearAfterSelect) {
+            override.value = null;
+        }
+
+        return override;
+    }, [clearAfterSelect]);
+
     const computedOptions = useMemo(
         () =>
             (options || []).map((option) =>
@@ -64,6 +78,7 @@ export function SimpleReactSelect<T>({
             isClearable={withDeselect}
             menuPlacement={'auto'}
             {...selectProps}
+            {...overrideSelectProps}
         />
     );
 }