Chandler feedback round 1

elastic · Apr 2, 2019 · 8e941e9 · 8e941e9
1 parent 8664ea1
commit 8e941e9
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 25 deletions.
diff --git a/src/components/selectable/selectable.tsx b/src/components/selectable/selectable.tsx
@@ -4,15 +4,14 @@ import React, {
   ReactNode,
   createRef,
   Fragment,
+  ReactElement,
 } from 'react';
 import classNames from 'classnames';
 import { CommonProps, Omit } from '../common';
 import { EuiSelectableSearch } from './selectable_search';
 import { EuiSelectableMessage } from './selectable_message';
 import { EuiSelectableList } from './selectable_list';
-// @ts-ignore
 import { EuiLoadingChart } from '../loading';
-// @ts-ignore
 import { getMatchingOptions } from './matching_options';
 import { comboBoxKeyCodes } from '../../services';
 import { TAB } from '../../services/key_codes';
@@ -40,10 +39,15 @@ export type EuiSelectableProps = Omit<
 > &
   CommonProps & {
     /**
-     * Function that returns the `list` node and then
+     * Function that takes the `list` node and then
      * the `search` node (if `searchable` is applied)
      */
-    children?: (list: ReactNode, search: ReactNode) => ReactNode;
+    children?: (
+      list: ReactElement<
+        typeof EuiSelectableMessage | typeof EuiSelectableList
+      >,
+      search: ReactElement<EuiSelectableSearch> | undefined
+    ) => ReactNode;
     /**
      * Array or Option objects, see docs for props
      */
@@ -89,7 +93,7 @@ export type EuiSelectableProps = Omit<
      * Custom render function for each option.
      * Returns (option, searchValue)
      */
-    renderOption?: (option: Option, searchValue?: string) => {};
+    renderOption?: (option: Option, searchValue: string) => {};
   };
 
 export interface EuiSelectableState {
@@ -278,7 +282,7 @@ export class EuiSelectable extends Component<
           <br />
           <p>
             <EuiI18n
-              token="euiComboBoxOptionsList.loadingOptions"
+              token="euiSelectable.loadingOptions"
               default="Loading options"
             />
           </p>
@@ -288,7 +292,7 @@ export class EuiSelectable extends Component<
       messageContent = (
         <p>
           <EuiI18n
-            token="euiComboBoxOptionsList.noMatchingOptions"
+            token="euiSelectable.noMatchingOptions"
             default="{searchValue} doesn't match any options"
             values={{ searchValue: <strong>{searchValue}</strong> }}
           />
@@ -298,7 +302,7 @@ export class EuiSelectable extends Component<
       messageContent = (
         <p>
           <EuiI18n
-            token="euiComboBoxOptionsList.noAvailableOptions"
+            token="euiSelectable.noAvailableOptions"
             default="No options available"
           />
         </p>

diff --git a/src/components/selectable/selectable_list/selectable_list.tsx b/src/components/selectable/selectable_list/selectable_list.tsx
@@ -1,9 +1,8 @@
-import React, { Component, HTMLAttributes } from 'react';
+import React, { Component, HTMLAttributes, ReactNode } from 'react';
 import classNames from 'classnames';
 import { CommonProps } from '../../common';
 import { List, AutoSizer, ListProps } from 'react-virtualized';
-// @ts-ignore
-import { htmlIdGenerator, comboBoxKeyCodes } from '../../../services';
+import { htmlIdGenerator } from '../../../services';
 import { EuiSelectableListItem } from './selectable_list_item';
 // @ts-ignore
 import { EuiHighlight } from '../../highlight';
@@ -15,7 +14,7 @@ export type EuiSelectableSingleOptionProps = 'always' | boolean;
 export type EuiSelectableOptionsListProps = HTMLAttributes<HTMLDivElement> &
   CommonProps & {
     /**
-     * The index of the option to be highlighted as pseudo-focused.
+     * The index of the option to be highlighted as pseudo-focused;
      * Good for use when only one selection is allowed and needing to open
      * directly to that option
      */
@@ -34,8 +33,8 @@ export type EuiSelectableOptionsListProps = HTMLAttributes<HTMLDivElement> &
      */
     virtualizedProps?: ListProps;
     /**
-     * Adds a border around the list to indicate the bounds.
-     * Useful when the list scrolls, otherwise use your own container.
+     * Adds a border around the list to indicate the bounds;
+     * Useful when the list scrolls, otherwise use your own container
      */
     bordered?: boolean;
   };
@@ -58,10 +57,10 @@ export type EuiSelectableListProps = EuiSelectableOptionsListProps & {
    */
   onOptionClick: (options: Option[]) => void;
   /**
-   * Custom render for the label portion of the option.
-   * Returns (option, searchValue)
+   * Custom render for the label portion of the option;
+   * Takes (option, searchValue), returns ReactNode
    */
-  renderOption?: (option: Option, searchValue?: string) => {};
+  renderOption?: (option: Option, searchValue: string) => ReactNode;
   /**
    * Sets the max height in pixels or pass `full` to allow
    * the whole group to fill the height of its container and
@@ -112,7 +111,7 @@ export class EuiSelectableList extends Component<EuiSelectableListProps> {
     const heightIsFull = forcedHeight === 'full';
 
     let calculatedHeight: any = !heightIsFull && forcedHeight;
-    let isOverflowing: boolean = false;
+    let isOverflowing = false;
 
     // If calculatedHeight is still undefined, then calculate it
     if (!calculatedHeight && !heightIsFull) {
@@ -149,7 +148,7 @@ export class EuiSelectableList extends Component<EuiSelectableListProps> {
               width={width}
               height={calculatedHeight || height}
               rowCount={optionArray.length}
-              rowHeight={Number(rowHeight)}
+              rowHeight={rowHeight}
               scrollToIndex={activeOptionIndex}
               {...virtualizedProps}
               rowRenderer={({ key, index, style }) => {

diff --git a/src/components/selectable/selectable_list/selectable_list_item.tsx b/src/components/selectable/selectable_list/selectable_list_item.tsx
@@ -47,7 +47,6 @@ export class EuiSelectableListItem extends Component<
 
   constructor(props: EuiSelectableListItemProps) {
     super(props);
-    this.state = { hasFocus: false };
   }
 
   render() {

diff --git a/src/components/selectable/selectable_search/selectable_search.tsx b/src/components/selectable/selectable_search/selectable_search.tsx
@@ -1,14 +1,13 @@
-import React, { Component, HTMLAttributes } from 'react';
+import React, { Component } from 'react';
 import classNames from 'classnames';
-import { CommonProps, Omit } from '../../common';
+import { CommonProps, Omit, PropsOf } from '../../common';
 // @ts-ignore
 import { EuiFieldSearch } from '../../form/field_search';
-// @ts-ignore
 import { getMatchingOptions } from '../matching_options';
 import { Option } from '../types';
 
 export type EuiSelectableSearchProps = Omit<
-  HTMLAttributes<HTMLDivElement>,
+  PropsOf<EuiFieldSearch>,
   'onChange'
 > &
   CommonProps & {

diff --git a/src/components/selectable/types.tsx b/src/components/selectable/types.tsx
@@ -27,5 +27,5 @@ export interface Option extends CommonProps {
    * Node to add to the far right of the item
    */
   append?: React.ReactNode;
-  ref?: () => void;
+  ref?: (optionIndex: number) => void;
 }