From 26524e3ff1dc6c91279ee9d3474c13b56c8f13de Mon Sep 17 00:00:00 2001
From: Giordano Ricci <me@giordanoricci.com>
Date: Fri, 26 Aug 2022 13:48:51 +0100
Subject: [PATCH] GrafanaUI: Fix styles for invalid selects & DataSourcePicker
 (#53476)

* GrafanaUI: fix styles for invalid select & DataSourcePicker

* Apply suggestions from code review

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

* fix focus issues & tests

* remove unused import

* TypeScript work in progress

* Move react select props to types.ts

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: joshhunt <josh@trtr.co>
---
 .betterer.results                             |  6 +-
 .../src/components/DataSourcePicker.tsx       |  3 +-
 .../src/components/Select/Container.tsx       | 59 --------------
 .../src/components/Select/Select.tsx          |  1 -
 .../src/components/Select/SelectBase.tsx      | 12 +--
 .../src/components/Select/SelectContainer.tsx | 77 ++++++++++---------
 .../grafana-ui/src/components/Select/types.ts | 27 ++++++-
 7 files changed, 77 insertions(+), 108 deletions(-)
 delete mode 100644 packages/grafana-ui/src/components/Select/Container.tsx

diff --git a/.betterer.results b/.betterer.results
index aab4e04aaa4..f5925e1daa7 100644
--- a/.betterer.results
+++ b/.betterer.results
@@ -1612,11 +1612,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "9"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "10"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "11"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "12"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "13"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "14"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "15"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "16"]
+      [0, 0, 0, "Unexpected any. Specify a different type.", "12"]
     ],
     "packages/grafana-ui/src/components/Select/SelectMenu.tsx:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
diff --git a/packages/grafana-runtime/src/components/DataSourcePicker.tsx b/packages/grafana-runtime/src/components/DataSourcePicker.tsx
index 71b5db4e4a9..8e13c6ff9e6 100644
--- a/packages/grafana-runtime/src/components/DataSourcePicker.tsx
+++ b/packages/grafana-runtime/src/components/DataSourcePicker.tsx
@@ -46,6 +46,7 @@ export interface DataSourcePickerProps {
   inputId?: string;
   filter?: (dataSource: DataSourceInstanceSettings) => boolean;
   onClear?: () => void;
+  invalid?: boolean;
 }
 
 /**
@@ -186,7 +187,7 @@ export class DataSourcePicker extends PureComponent<DataSourcePickerProps, DataS
           placeholder={placeholder}
           noOptionsMessage="No datasources found"
           value={value ?? null}
-          invalid={!!error}
+          invalid={Boolean(error) || Boolean(this.props.invalid)}
           getOptionLabel={(o) => {
             if (o.meta && isUnsignedPluginSignature(o.meta.signature) && o !== value) {
               return (
diff --git a/packages/grafana-ui/src/components/Select/Container.tsx b/packages/grafana-ui/src/components/Select/Container.tsx
deleted file mode 100644
index 33eacc870bd..00000000000
--- a/packages/grafana-ui/src/components/Select/Container.tsx
+++ /dev/null
@@ -1,59 +0,0 @@
-import { css, cx } from '@emotion/css';
-import React from 'react';
-import { components, ContainerProps, GroupBase } from 'react-select';
-
-import { GrafanaTheme2 } from '@grafana/data';
-
-import { stylesFactory } from '../../themes';
-import { useTheme2 } from '../../themes/ThemeContext';
-import { focusCss } from '../../themes/mixins';
-import { sharedInputStyle } from '../Forms/commonStyles';
-import { getInputStyles } from '../Input/Input';
-
-export const SelectContainer = <Option, isMulti extends boolean, Group extends GroupBase<Option>>(
-  props: ContainerProps<Option, isMulti, Group> & { isFocused: boolean }
-) => {
-  const { isDisabled, isFocused, children } = props;
-
-  const theme = useTheme2();
-  const styles = getSelectContainerStyles(theme, isFocused, isDisabled);
-
-  return (
-    <components.SelectContainer {...props} className={cx(styles.wrapper, props.className)}>
-      {children}
-    </components.SelectContainer>
-  );
-};
-
-const getSelectContainerStyles = stylesFactory((theme: GrafanaTheme2, focused: boolean, disabled: boolean) => {
-  const styles = getInputStyles({ theme, invalid: false });
-
-  return {
-    wrapper: cx(
-      styles.wrapper,
-      sharedInputStyle(theme, false),
-      focused &&
-        css`
-          ${focusCss(theme.v1)}
-        `,
-      disabled && styles.inputDisabled,
-      css`
-        position: relative;
-        box-sizing: border-box;
-        display: flex;
-        flex-direction: row;
-        flex-wrap: wrap;
-        align-items: center;
-        justify-content: space-between;
-
-        min-height: 32px;
-        height: auto;
-        max-width: 100%;
-
-        /* Input padding is applied to the InputControl so the menu is aligned correctly */
-        padding: 0;
-        cursor: ${disabled ? 'not-allowed' : 'pointer'};
-      `
-    ),
-  };
-});
diff --git a/packages/grafana-ui/src/components/Select/Select.tsx b/packages/grafana-ui/src/components/Select/Select.tsx
index 89698678ae9..dc13eca2a19 100644
--- a/packages/grafana-ui/src/components/Select/Select.tsx
+++ b/packages/grafana-ui/src/components/Select/Select.tsx
@@ -18,7 +18,6 @@ export function MultiSelect<T>(props: MultiSelectCommonProps<T>) {
 export interface AsyncSelectProps<T> extends Omit<SelectCommonProps<T>, 'options'>, SelectAsyncProps<T> {
   // AsyncSelect has options stored internally. We cannot enable plain values as we don't have access to the fetched options
   value?: SelectableValue<T> | null;
-  invalid?: boolean;
 }
 
 export function AsyncSelect<T>(props: AsyncSelectProps<T>) {
diff --git a/packages/grafana-ui/src/components/Select/SelectBase.tsx b/packages/grafana-ui/src/components/Select/SelectBase.tsx
index 28a41837458..5bf9b932b7a 100644
--- a/packages/grafana-ui/src/components/Select/SelectBase.tsx
+++ b/packages/grafana-ui/src/components/Select/SelectBase.tsx
@@ -317,28 +317,28 @@ export function SelectBase<T>({
               />
             );
           },
-          LoadingIndicator(props: any) {
-            return <Spinner inline={true} />;
+          LoadingIndicator() {
+            return <Spinner inline />;
           },
-          LoadingMessage(props: any) {
+          LoadingMessage() {
             return <div className={styles.loadingMessage}>{loadingMessage}</div>;
           },
-          NoOptionsMessage(props: any) {
+          NoOptionsMessage() {
             return (
               <div className={styles.loadingMessage} aria-label="No options provided">
                 {noOptionsMessage}
               </div>
             );
           },
-          DropdownIndicator(props: any) {
+          DropdownIndicator(props) {
             return <DropdownIndicator isOpen={props.selectProps.menuIsOpen} />;
           },
           SingleValue(props: any) {
             return <SingleValue {...props} disabled={disabled} />;
           },
+          SelectContainer,
           MultiValueContainer: MultiValueContainer,
           MultiValueRemove: MultiValueRemove,
-          SelectContainer,
           ...components,
         }}
         styles={selectStyles}
diff --git a/packages/grafana-ui/src/components/Select/SelectContainer.tsx b/packages/grafana-ui/src/components/Select/SelectContainer.tsx
index c7d500e9706..68845cdafe3 100644
--- a/packages/grafana-ui/src/components/Select/SelectContainer.tsx
+++ b/packages/grafana-ui/src/components/Select/SelectContainer.tsx
@@ -10,19 +10,24 @@ import { focusCss } from '../../themes/mixins';
 import { sharedInputStyle } from '../Forms/commonStyles';
 import { getInputStyles } from '../Input/Input';
 
-// isFocus prop is actually available, but its not in the types for the version we have.
-export interface SelectContainerProps<Option, isMulti extends boolean, Group extends GroupBase<Option>>
-  extends BaseContainerProps<Option, isMulti, Group> {
-  isFocused: boolean;
-}
+import { CustomComponentProps } from './types';
+
+// prettier-ignore
+export type SelectContainerProps<Option, isMulti extends boolean, Group extends GroupBase<Option>> =
+  BaseContainerProps<Option, isMulti, Group> & CustomComponentProps<Option, isMulti, Group>;
 
 export const SelectContainer = <Option, isMulti extends boolean, Group extends GroupBase<Option>>(
   props: SelectContainerProps<Option, isMulti, Group>
 ) => {
-  const { isDisabled, isFocused, children } = props;
+  const {
+    isDisabled,
+    isFocused,
+    children,
+    selectProps: { invalid = false },
+  } = props;
 
   const theme = useTheme2();
-  const styles = getSelectContainerStyles(theme, isFocused, isDisabled);
+  const styles = getSelectContainerStyles(theme, isFocused, isDisabled, invalid);
 
   return (
     <components.SelectContainer {...props} className={cx(styles.wrapper, props.className)}>
@@ -31,35 +36,37 @@ export const SelectContainer = <Option, isMulti extends boolean, Group extends G
   );
 };
 
-const getSelectContainerStyles = stylesFactory((theme: GrafanaTheme2, focused: boolean, disabled: boolean) => {
-  const styles = getInputStyles({ theme, invalid: false });
+const getSelectContainerStyles = stylesFactory(
+  (theme: GrafanaTheme2, focused: boolean, disabled: boolean, invalid: boolean) => {
+    const styles = getInputStyles({ theme, invalid });
 
-  return {
-    wrapper: cx(
-      styles.wrapper,
-      sharedInputStyle(theme, false),
-      focused &&
+    return {
+      wrapper: cx(
+        styles.wrapper,
+        sharedInputStyle(theme, invalid),
+        focused &&
+          css`
+            ${focusCss(theme.v1)}
+          `,
+        disabled && styles.inputDisabled,
         css`
-          ${focusCss(theme.v1)}
-        `,
-      disabled && styles.inputDisabled,
-      css`
-        position: relative;
-        box-sizing: border-box;
-        /* The display property is set by the styles prop in SelectBase because it's dependant on the width prop  */
-        flex-direction: row;
-        flex-wrap: wrap;
-        align-items: stretch;
-        justify-content: space-between;
+          position: relative;
+          box-sizing: border-box;
+          /* The display property is set by the styles prop in SelectBase because it's dependant on the width prop  */
+          flex-direction: row;
+          flex-wrap: wrap;
+          align-items: stretch;
+          justify-content: space-between;
 
-        min-height: 32px;
-        height: auto;
-        max-width: 100%;
+          min-height: 32px;
+          height: auto;
+          max-width: 100%;
 
-        /* Input padding is applied to the InputControl so the menu is aligned correctly */
-        padding: 0;
-        cursor: ${disabled ? 'not-allowed' : 'pointer'};
-      `
-    ),
-  };
-});
+          /* Input padding is applied to the InputControl so the menu is aligned correctly */
+          padding: 0;
+          cursor: ${disabled ? 'not-allowed' : 'pointer'};
+        `
+      ),
+    };
+  }
+);
diff --git a/packages/grafana-ui/src/components/Select/types.ts b/packages/grafana-ui/src/components/Select/types.ts
index b7bd318463d..2149ba2723c 100644
--- a/packages/grafana-ui/src/components/Select/types.ts
+++ b/packages/grafana-ui/src/components/Select/types.ts
@@ -1,5 +1,10 @@
 import React from 'react';
-import { ActionMeta as SelectActionMeta, GroupBase, OptionsOrGroups } from 'react-select';
+import {
+  ActionMeta as SelectActionMeta,
+  CommonProps as ReactSelectCommonProps,
+  GroupBase,
+  OptionsOrGroups,
+} from 'react-select';
 
 import { SelectableValue } from '@grafana/data';
 
@@ -103,10 +108,13 @@ export interface MultiSelectCommonProps<T> extends Omit<SelectCommonProps<T>, 'o
   onChange: (item: Array<SelectableValue<T>>) => {} | void;
 }
 
+// This is the type of *our* SelectBase component, not ReactSelect's prop, although
+// they should be mostly compatible.
 export interface SelectBaseProps<T> extends SelectCommonProps<T>, SelectAsyncProps<T> {
   invalid?: boolean;
 }
 
+// This is used for the `renderControl` prop on *our* SelectBase component
 export interface CustomControlProps<T> {
   ref: React.Ref<any>;
   isOpen: boolean;
@@ -133,3 +141,20 @@ export type SelectOptions<T = any> =
   | Array<SelectableValue<T> | SelectableOptGroup<T> | Array<SelectableOptGroup<T>>>;
 
 export type FormatOptionLabelMeta<T> = { context: string; inputValue: string; selectValue: Array<SelectableValue<T>> };
+
+// This is the type of `selectProps` our custom components (like SelectContainer, etc) recieve
+// It's slightly different to the base react select props because we pass in additional props directly to
+// react select
+export type ReactSelectProps<Option, IsMulti extends boolean, Group extends GroupBase<Option>> = ReactSelectCommonProps<
+  Option,
+  IsMulti,
+  Group
+>['selectProps'] & {
+  invalid: boolean;
+};
+
+// Use this type when implementing custom components for react select.
+// See SelectContainerProps in SelectContainer.tsx
+export interface CustomComponentProps<Option, isMulti extends boolean, Group extends GroupBase<Option>> {
+  selectProps: ReactSelectProps<Option, isMulti, Group>;
+}