chore: Select component refactoring - PropertiesModal - Iteration 5 (#…

…16445) * Refactor Select * Update tests and checks * Clean up props * Update ariaLabel * Reconcile with master changes * Apply enhancements * Fix lint
apache · Sep 27, 2021 · ab9f8cb · ab9f8cb
1 parent e40ff3a
commit ab9f8cb
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 54 deletions.
diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
@@ -211,8 +211,8 @@ test('Should render all elements inside modal', async () => {
   const props = createProps();
   render(<PropertiesModal {...props} />);
   await waitFor(() => {
-    expect(screen.getAllByRole('textbox')).toHaveLength(4);
-
+    expect(screen.getAllByRole('textbox')).toHaveLength(3);
+    expect(screen.getByRole('combobox')).toBeInTheDocument();
     expect(
       screen.getByRole('heading', { name: 'Basic information' }),
     ).toBeVisible();

diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx
@@ -16,12 +16,12 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useState, useEffect, useCallback } from 'react';
+import React, { useMemo, useState, useEffect, useCallback } from 'react';
 import Modal from 'src/components/Modal';
 import { Row, Col, Input, TextArea } from 'src/common/components';
 import Button from 'src/components/Button';
-import { OptionsType } from 'react-select/src/types';
-import { AsyncSelect } from 'src/components/Select';
+import { Select } from 'src/components';
+import { SelectValue } from 'antd/lib/select';
 import rison from 'rison';
 import { t, SupersetClient } from '@superset-ui/core';
 import Chart, { Slice } from 'src/types/Chart';
@@ -35,11 +35,6 @@ type PropertiesModalProps = {
   onSave: (chart: Chart) => void;
 };
 
-type OwnerOption = {
-  label: string;
-  value: number;
-};
-
 export default function PropertiesModal({
   slice,
   onHide,
@@ -54,7 +49,9 @@ export default function PropertiesModal({
   const [cacheTimeout, setCacheTimeout] = useState(
     slice.cache_timeout != null ? slice.cache_timeout : '',
   );
-  const [owners, setOwners] = useState<OptionsType<OwnerOption> | null>(null);
+  const [selectedOwners, setSelectedOwners] = useState<SelectValue | null>(
+    null,
+  );
 
   function showError({ error, statusText, message }: any) {
     let errorText = error || statusText || t('An error has occurred');
@@ -75,7 +72,7 @@ export default function PropertiesModal({
           endpoint: `/api/v1/chart/${slice.slice_id}`,
         });
         const chart = response.json.result;
-        setOwners(
+        setSelectedOwners(
           chart.owners.map((owner: any) => ({
             value: owner.id,
             label: `${owner.first_name} ${owner.last_name}`,
@@ -89,36 +86,23 @@ export default function PropertiesModal({
     [slice.slice_id],
   );
 
-  // get the owners of this slice
-  useEffect(() => {
-    fetchChartData();
-  }, [fetchChartData]);
-
-  // update name after it's changed in another modal
-  useEffect(() => {
-    setName(slice.slice_name || '');
-  }, [slice.slice_name]);
-
-  const loadOptions = (input = '') => {
-    const query = rison.encode({
-      filter: input,
-    });
-    return SupersetClient.get({
-      endpoint: `/api/v1/chart/related/owners?q=${query}`,
-    }).then(
-      response => {
-        const { result } = response.json;
-        return result.map((item: any) => ({
-          value: item.value,
-          label: item.text,
-        }));
-      },
-      badResponse => {
-        getClientErrorObject(badResponse).then(showError);
-        return [];
-      },
-    );
-  };
+  const loadOptions = useMemo(
+    () => (input = '', page: number, pageSize: number) => {
+      const query = rison.encode({ filter: input, page, page_size: pageSize });
+      return SupersetClient.get({
+        endpoint: `/api/v1/chart/related/owners?q=${query}`,
+      }).then(response => ({
+        data: response.json.result.map(
+          (item: { value: number; text: string }) => ({
+            value: item.value,
+            label: item.text,
+          }),
+        ),
+        totalCount: response.json.count,
+      }));
+    },
+    [],
+  );
 
   const onSubmit = async (event: React.FormEvent) => {
     event.stopPropagation();
@@ -129,8 +113,11 @@ export default function PropertiesModal({
       description: description || null,
       cache_timeout: cacheTimeout || null,
     };
-    if (owners) {
-      payload.owners = owners.map(o => o.value);
+    if (selectedOwners) {
+      payload.owners = (selectedOwners as {
+        value: number;
+        label: string;
+      }[]).map(o => o.value);
     }
     try {
       const res = await SupersetClient.put({
@@ -152,6 +139,18 @@ export default function PropertiesModal({
     setSubmitting(false);
   };
 
+  const ownersLabel = t('Owners');
+
+  // get the owners of this slice
+  useEffect(() => {
+    fetchChartData();
+  }, [fetchChartData]);
+
+  // update name after it's changed in another modal
+  useEffect(() => {
+    setName(slice.slice_name || '');
+  }, [slice.slice_name]);
+
   return (
     <Modal
       show={show}
@@ -236,17 +235,16 @@ export default function PropertiesModal({
               </p>
             </FormItem>
             <h3 style={{ marginTop: '1em' }}>{t('Access')}</h3>
-            <FormItem label={t('Owners')}>
-              <AsyncSelect
-                isMulti
+            <FormItem label={ownersLabel}>
+              <Select
+                ariaLabel={ownersLabel}
+                mode="multiple"
                 name="owners"
-                value={owners || []}
-                loadOptions={loadOptions}
-                defaultOptions // load options on render
-                cacheOptions
-                onChange={setOwners}
-                disabled={!owners}
-                filterOption={null} // options are filtered at the api
+                value={selectedOwners || []}
+                options={loadOptions}
+                onChange={setSelectedOwners}
+                disabled={!selectedOwners}
+                allowClear
               />
               <p className="help-block">
                 {t(