feat: Adds a helper text option to the Select component (apache#21269)

Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
preset-io · Sep 2, 2022 · fbe9807 · fbe9807
1 parent 60db0bb
commit fbe9807
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 46 deletions.
diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
@@ -206,13 +206,7 @@ test('sort the options using a custom sort comparator', async () => {
     option1: typeof OPTIONS[0],
     option2: typeof OPTIONS[0],
   ) => option1.gender.localeCompare(option2.gender);
-  render(
-    <AsyncSelect
-      {...defaultProps}
-      options={loadOptions}
-      sortComparator={sortComparator}
-    />,
-  );
+  render(<AsyncSelect {...defaultProps} sortComparator={sortComparator} />);
   await open();
   const options = await findAllSelectOptions();
   const optionsPage = OPTIONS.slice(0, defaultProps.pageSize);
@@ -294,7 +288,7 @@ test('searches for label or value', async () => {
 });
 
 test('search order exact and startWith match first', async () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   await open();
   await type('Her');
   expect(await findSelectOption('Guilherme')).toBeInTheDocument();
@@ -333,7 +327,7 @@ test('same case should be ranked to the top', async () => {
 });
 
 test('ignores special keys when searching', async () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   await type('{shift}');
   expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
 });
@@ -434,7 +428,7 @@ test('clear all the values', async () => {
 });
 
 test('does not add a new option if allowNewOptions is false', async () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   await open();
   await type(NEW_OPTION);
   expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
@@ -469,7 +463,7 @@ test('adds the null option when selected in multiple mode', async () => {
 });
 
 test('renders the select with default props', () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   expect(getSelect()).toBeInTheDocument();
 });
 
@@ -485,7 +479,7 @@ test('opens the select without any data', async () => {
 });
 
 test('displays the loading indicator when opening', async () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   await waitFor(() => {
     userEvent.click(getSelect());
     expect(screen.getByText(LOADING)).toBeInTheDocument();
@@ -494,17 +488,15 @@ test('displays the loading indicator when opening', async () => {
 });
 
 test('makes a selection in single mode', async () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   const optionText = 'Emma';
   await open();
   userEvent.click(await findSelectOption(optionText));
   expect(await findSelectValue()).toHaveTextContent(optionText);
 });
 
 test('multiple selections in multiple mode', async () => {
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} mode="multiple" />,
-  );
+  render(<AsyncSelect {...defaultProps} mode="multiple" />);
   await open();
   const [firstOption, secondOption] = OPTIONS;
   userEvent.click(await findSelectOption(firstOption.label));
@@ -516,9 +508,7 @@ test('multiple selections in multiple mode', async () => {
 
 test('changes the selected item in single mode', async () => {
   const onChange = jest.fn();
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} onChange={onChange} />,
-  );
+  render(<AsyncSelect {...defaultProps} onChange={onChange} />);
   await open();
   const [firstOption, secondOption] = OPTIONS;
   userEvent.click(await findSelectOption(firstOption.label));
@@ -542,9 +532,7 @@ test('changes the selected item in single mode', async () => {
 });
 
 test('deselects an item in multiple mode', async () => {
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} mode="multiple" />,
-  );
+  render(<AsyncSelect {...defaultProps} mode="multiple" />);
   await open();
   const option3 = OPTIONS[2];
   const option8 = OPTIONS[7];
@@ -578,18 +566,14 @@ test('deselects an item in multiple mode', async () => {
 });
 
 test('adds a new option if none is available and allowNewOptions is true', async () => {
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} allowNewOptions />,
-  );
+  render(<AsyncSelect {...defaultProps} allowNewOptions />);
   await open();
   await type(NEW_OPTION);
   expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
 });
 
 test('does not add a new option if the option already exists', async () => {
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} allowNewOptions />,
-  );
+  render(<AsyncSelect {...defaultProps} allowNewOptions />);
   const option = OPTIONS[0].label;
   await open();
   await type(option);
@@ -602,32 +586,21 @@ test('does not add a new option if the option already exists', async () => {
 });
 
 test('shows "No data" when allowNewOptions is false and a new option is entered', async () => {
-  render(
-    <AsyncSelect
-      {...defaultProps}
-      options={loadOptions}
-      allowNewOptions={false}
-      showSearch
-    />,
-  );
+  render(<AsyncSelect {...defaultProps} allowNewOptions={false} showSearch />);
   await open();
   await type(NEW_OPTION);
   expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
 });
 
 test('does not show "No data" when allowNewOptions is true and a new option is entered', async () => {
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} allowNewOptions />,
-  );
+  render(<AsyncSelect {...defaultProps} allowNewOptions />);
   await open();
   await type(NEW_OPTION);
   expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument();
 });
 
 test('sets a initial value in single mode', async () => {
-  render(
-    <AsyncSelect {...defaultProps} options={loadOptions} value={OPTIONS[0]} />,
-  );
+  render(<AsyncSelect {...defaultProps} value={OPTIONS[0]} />);
   expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label);
 });
 
@@ -636,7 +609,6 @@ test('sets a initial value in multiple mode', async () => {
     <AsyncSelect
       {...defaultProps}
       mode="multiple"
-      options={loadOptions}
       value={[OPTIONS[0], OPTIONS[1]]}
     />,
   );
@@ -646,7 +618,7 @@ test('sets a initial value in multiple mode', async () => {
 });
 
 test('searches for matches in both loaded and unloaded pages', async () => {
-  render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+  render(<AsyncSelect {...defaultProps} />);
   await open();
   await type('and');
 
@@ -750,6 +722,20 @@ test('fires a new request if all values have not been fetched', async () => {
   expect(mock).toHaveBeenCalledTimes(2);
 });
 
+test('does not render a helper text by default', async () => {
+  render(<AsyncSelect {...defaultProps} />);
+  await open();
+  expect(screen.queryByRole('note')).not.toBeInTheDocument();
+});
+
+test('renders a helper text when one is provided', async () => {
+  const helperText = 'Helper text';
+  render(<AsyncSelect {...defaultProps} helperText={helperText} />);
+  await open();
+  expect(screen.getByRole('note')).toBeInTheDocument();
+  expect(screen.queryByText(helperText)).toBeInTheDocument();
+});
+
 /*
  TODO: Add tests that require scroll interaction. Needs further investigation.
  - Fetches more data when scrolling and more data is available

diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx
@@ -102,6 +102,11 @@ export interface AsyncSelectProps extends PickedSelectProps {
    * Can be any ReactNode.
    */
   header?: ReactNode;
+  /**
+   * It adds a helper text on top of the Select options
+   * with additional context to help with the interaction.
+   */
+  helperText?: string;
   /**
    * It fires a request against the server after
    * the first interaction and not on render.
@@ -182,6 +187,9 @@ const StyledSelect = styled(AntdSelect)`
     .ant-select-arrow .anticon:not(.ant-select-suffix) {
       pointer-events: none;
     }
+    .ant-select-dropdown {
+      padding: 0;
+    }
   `}
 `;
 
@@ -224,6 +232,16 @@ const StyledLoadingText = styled.div`
   `}
 `;
 
+const StyledHelperText = styled.div`
+  ${({ theme }) => `
+    padding: ${theme.gridUnit * 2}px ${theme.gridUnit * 3}px;
+    color: ${theme.colors.grayscale.base};
+    font-size: ${theme.typography.sizes.s}px;
+    cursor: default;
+    border-bottom: 1px solid ${theme.colors.grayscale.light2};
+  `}
+`;
+
 const MAX_TAG_COUNT = 4;
 const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
 const DEFAULT_PAGE_SIZE = 100;
@@ -297,6 +315,7 @@ const AsyncSelect = forwardRef(
       fetchOnlyOnSearch,
       filterOption = true,
       header = null,
+      helperText,
       invertSelection = false,
       lazyLoading = true,
       loading,
@@ -612,7 +631,16 @@ const AsyncSelect = forwardRef(
       if (isLoading && fullSelectOptions.length === 0) {
         return <StyledLoadingText>{t('Loading...')}</StyledLoadingText>;
       }
-      return error ? <Error error={error} /> : originNode;
+      return error ? (
+        <Error error={error} />
+      ) : (
+        <>
+          {helperText && (
+            <StyledHelperText role="note">{helperText}</StyledHelperText>
+          )}
+          {originNode}
+        </>
+      );
     };
 
     // use a function instead of component since every rerender of the

diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
@@ -535,6 +535,20 @@ test('triggers getPopupContainer if passed', async () => {
   expect(getPopupContainer).toHaveBeenCalled();
 });
 
+test('does not render a helper text by default', async () => {
+  render(<Select {...defaultProps} />);
+  await open();
+  expect(screen.queryByRole('note')).not.toBeInTheDocument();
+});
+
+test('renders a helper text when one is provided', async () => {
+  const helperText = 'Helper text';
+  render(<Select {...defaultProps} helperText={helperText} />);
+  await open();
+  expect(screen.getByRole('note')).toBeInTheDocument();
+  expect(screen.queryByText(helperText)).toBeInTheDocument();
+});
+
 /*
  TODO: Add tests that require scroll interaction. Needs further investigation.
  - Fetches more data when scrolling and more data is available

diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
@@ -84,6 +84,11 @@ export interface SelectProps extends PickedSelectProps {
    * Can be any ReactNode.
    */
   header?: ReactNode;
+  /**
+   * It adds a helper text on top of the Select options
+   * with additional context to help with the interaction.
+   */
+  helperText?: string;
   /**
    * It defines whether the Select should allow for the
    * selection of multiple options or single.
@@ -139,6 +144,9 @@ const StyledSelect = styled(AntdSelect)`
     .ant-select-arrow .anticon:not(.ant-select-suffix) {
       pointer-events: none;
     }
+    .ant-select-dropdown {
+      padding: 0;
+    }
   `}
 `;
 
@@ -162,6 +170,16 @@ const StyledLoadingText = styled.div`
   `}
 `;
 
+const StyledHelperText = styled.div`
+  ${({ theme }) => `
+    padding: ${theme.gridUnit * 2}px ${theme.gridUnit * 3}px;
+    color: ${theme.colors.grayscale.base};
+    font-size: ${theme.typography.sizes.s}px;
+    cursor: default;
+    border-bottom: 1px solid ${theme.colors.grayscale.light2};
+  `}
+`;
+
 const MAX_TAG_COUNT = 4;
 const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
 const EMPTY_OPTIONS: OptionsType = [];
@@ -224,6 +242,7 @@ const Select = forwardRef(
       ariaLabel,
       filterOption = true,
       header = null,
+      helperText,
       invertSelection = false,
       labelInValue = false,
       loading,
@@ -401,7 +420,14 @@ const Select = forwardRef(
       if (isLoading && fullSelectOptions.length === 0) {
         return <StyledLoadingText>{t('Loading...')}</StyledLoadingText>;
       }
-      return originNode;
+      return (
+        <>
+          {helperText && (
+            <StyledHelperText role="note">{helperText}</StyledHelperText>
+          )}
+          {originNode}
+        </>
+      );
     };
 
     // use a function instead of component since every rerender of the