[ML] Address review feedback

elastic · Aug 8, 2019 · 7b862b5 · 7b862b5
1 parent c4cddd1
commit 7b862b5
Show file tree

Hide file tree

Showing 10 changed files with 120 additions and 98 deletions.
diff --git a/x-pack/legacy/plugins/ml/public/components/create_job_link_card/create_job_link_card.tsx b/x-pack/legacy/plugins/ml/public/components/create_job_link_card/create_job_link_card.tsx
@@ -6,12 +6,7 @@
 
 import React, { FC } from 'react';
 
-import {
-  // @ts-ignore
-  EuiCard,
-  EuiIcon,
-  IconType,
-} from '@elastic/eui';
+import { EuiCard, EuiIcon, IconType } from '@elastic/eui';
 
 interface Props {
   iconType: IconType;
@@ -22,14 +17,12 @@ interface Props {
 
 // Component for rendering a card which links to the Create Job page, displaying an
 // icon, card title, description and link.
-export const CreateJobLinkCard: FC<Props> = ({ iconType, title, description, onClick }) => {
-  return (
-    <EuiCard
-      layout="horizontal"
-      icon={<EuiIcon size="xl" type={iconType} />}
-      title={title}
-      description={description}
-      onClick={onClick}
-    />
-  );
-};
+export const CreateJobLinkCard: FC<Props> = ({ iconType, title, description, onClick }) => (
+  <EuiCard
+    layout="horizontal"
+    icon={<EuiIcon size="xl" type={iconType} />}
+    title={title}
+    description={description}
+    onClick={onClick}
+  />
+);
diff --git a/x-pack/legacy/plugins/ml/public/data_visualizer/common/field_vis_config.ts b/x-pack/legacy/plugins/ml/public/data_visualizer/common/field_vis_config.ts
@@ -6,7 +6,7 @@
 
 import { ML_JOB_FIELD_TYPES } from '../../../common/constants/field_types';
 
-// The internal representation of the confuguration used to build the visuals
+// The internal representation of the configuration used to build the visuals
 // which display the field information.
 // TODO - type stats
 export interface FieldVisConfig {

diff --git a/x-pack/legacy/plugins/ml/public/data_visualizer/components/actions_panel/actions_panel.tsx b/x-pack/legacy/plugins/ml/public/data_visualizer/components/actions_panel/actions_panel.tsx
@@ -9,25 +9,24 @@ import React, { FC } from 'react';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { i18n } from '@kbn/i18n';
 
-import chrome from 'ui/chrome';
 import { IndexPattern } from 'ui/index_patterns';
 
 import { EuiPanel, EuiSpacer, EuiText, EuiTitle } from '@elastic/eui';
 
+import { useUiChromeContext } from '../../../contexts/ui/use_ui_chrome_context';
 import { CreateJobLinkCard } from '../../../components/create_job_link_card';
 
 interface Props {
   indexPattern: IndexPattern;
 }
 
 export const ActionsPanel: FC<Props> = ({ indexPattern }) => {
+  const basePath = useUiChromeContext().getBasePath();
+
   function openAdvancedJobWizard() {
     // TODO - pass the search string to the advanced job page as well as the index pattern
     //       (add in with new advanced job wizard?)
-    window.open(
-      `${chrome.getBasePath()}/app/ml#/jobs/new_job/advanced?index=${indexPattern}`,
-      '_self'
-    );
+    window.open(`${basePath}/app/ml#/jobs/new_job/advanced?index=${indexPattern}`, '_self');
   }
 
   return (

diff --git a/...egacy/plugins/ml/public/data_visualizer/components/field_data_card/content_types/index.ts b/...egacy/plugins/ml/public/data_visualizer/components/field_data_card/content_types/index.ts
@@ -10,6 +10,7 @@ export { DocumentCountContent } from './document_count_content';
 export { GeoPointContent } from './geo_point_content';
 export { KeywordContent } from './keyword_content';
 export { IpContent } from './ip_content';
+export { NotInDocsContent } from './not_in_docs_content';
 export { NumberContent } from './number_content';
 export { OtherContent } from './other_content';
 export { TextContent } from './text_content';
diff --git a/...l/public/data_visualizer/components/field_data_card/content_types/not_in_docs_content.tsx b/...l/public/data_visualizer/components/field_data_card/content_types/not_in_docs_content.tsx
@@ -0,0 +1,33 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React, { FC, Fragment } from 'react';
+import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiSpacer } from '@elastic/eui';
+
+import { FormattedMessage } from '@kbn/i18n/react';
+
+export const NotInDocsContent: FC = () => (
+  <Fragment>
+    <EuiSpacer size="xxl" />
+    <EuiFlexGroup justifyContent="spaceAround">
+      <EuiFlexItem grow={false}>
+        <EuiFlexGroup gutterSize="xs" justifyContent="spaceAround">
+          <EuiFlexItem grow={false}>
+            <EuiIcon type="alert" />
+          </EuiFlexItem>
+        </EuiFlexGroup>
+        <EuiFlexGroup style={{ padding: '0px 16px', textAlign: 'center' }}>
+          <EuiFlexItem grow={false}>
+            <FormattedMessage
+              id="xpack.ml.fieldDataCard.fieldNotInDocsLabel"
+              defaultMessage="This field does not appear in any documents for the selected time range"
+            />
+          </EuiFlexItem>
+        </EuiFlexGroup>
+      </EuiFlexItem>
+    </EuiFlexGroup>
+  </Fragment>
+);
diff --git a/.../data_visualizer/components/field_data_card/document_count_chart/document_count_chart.tsx b/.../data_visualizer/components/field_data_card/document_count_chart/document_count_chart.tsx
@@ -21,10 +21,11 @@ import {
   Settings,
 } from '@elastic/charts';
 
-import chrome from 'ui/chrome';
 import darkTheme from '@elastic/eui/dist/eui_theme_dark.json';
 import lightTheme from '@elastic/eui/dist/eui_theme_light.json';
 
+import { useUiChromeContext } from '../../../../contexts/ui/use_ui_chrome_context';
+
 export interface DocumentCountChartPoint {
   time: number | string;
   value: number;
@@ -40,16 +41,6 @@ interface Props {
 
 const SPEC_ID = 'document_count';
 
-// TODO - switch to use inline areaSeriesStyle to set series fill once charts version is 8.0.0+
-const IS_DARK_THEME = chrome.getUiSettingsClient().get('theme:darkMode');
-const themeName = IS_DARK_THEME ? darkTheme : lightTheme;
-const EVENT_RATE_COLOR = themeName.euiColorVis2;
-const barSeriesColorValues: DataSeriesColorsValues = {
-  colorValues: [],
-  specId: getSpecId(SPEC_ID),
-};
-const seriesColors = new Map([[barSeriesColorValues, EVENT_RATE_COLOR]]);
-
 export const DocumentCountChart: FC<Props> = ({
   width,
   height,
@@ -68,6 +59,18 @@ export const DocumentCountChart: FC<Props> = ({
 
   const dateFormatter = niceTimeFormatter([timeRangeEarliest, timeRangeLatest]);
 
+  // TODO - switch to use inline areaSeriesStyle to set series fill once charts version is 8.0.0+
+  const IS_DARK_THEME = useUiChromeContext()
+    .getUiSettingsClient()
+    .get('theme:darkMode');
+  const themeName = IS_DARK_THEME ? darkTheme : lightTheme;
+  const EVENT_RATE_COLOR = themeName.euiColorVis2;
+  const barSeriesColorValues: DataSeriesColorsValues = {
+    colorValues: [],
+    specId: getSpecId(SPEC_ID),
+  };
+  const seriesColors = new Map([[barSeriesColorValues, EVENT_RATE_COLOR]]);
+
   return (
     <div style={{ width, height }}>
       <Chart>

diff --git a/...k/legacy/plugins/ml/public/data_visualizer/components/field_data_card/field_data_card.tsx b/...k/legacy/plugins/ml/public/data_visualizer/components/field_data_card/field_data_card.tsx
@@ -4,17 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { FC, Fragment } from 'react';
-import {
-  EuiFlexGroup,
-  EuiFlexItem,
-  EuiIcon,
-  EuiLoadingSpinner,
-  EuiSpacer,
-  EuiText,
-} from '@elastic/eui';
-
-import { FormattedMessage } from '@kbn/i18n/react';
+import React, { FC } from 'react';
 
 import { ML_JOB_FIELD_TYPES } from '../../../../common/constants/field_types';
 
@@ -28,10 +18,12 @@ import {
   GeoPointContent,
   IpContent,
   KeywordContent,
+  NotInDocsContent,
   NumberContent,
   OtherContent,
   TextContent,
 } from './content_types';
+import { LoadingIndicator } from './loading_indicator';
 
 interface Props {
   config: FieldVisConfig;
@@ -41,6 +33,10 @@ export const FieldDataCard: FC<Props> = ({ config }) => {
   const { fieldName, loading, type, existsInDocs } = config;
 
   function getCardContent() {
+    if (existsInDocs === false) {
+      return <NotInDocsContent />;
+    }
+
     switch (type) {
       case ML_JOB_FIELD_TYPES.NUMBER:
         if (fieldName !== undefined) {
@@ -77,49 +73,7 @@ export const FieldDataCard: FC<Props> = ({ config }) => {
       <div className="mlFieldDataCard">
         <FieldTitleBar card={config} />
         <div className="mlFieldDataCard__content">
-          {loading === true && (
-            <Fragment>
-              <EuiSpacer size="xxl" />
-              <EuiFlexGroup justifyContent="spaceAround">
-                <EuiFlexItem grow={false}>
-                  <EuiLoadingSpinner size="l" />
-                </EuiFlexItem>
-              </EuiFlexGroup>
-              <EuiFlexGroup justifyContent="spaceAround">
-                <EuiFlexItem grow={false}>
-                  <EuiText color="subdued">
-                    <FormattedMessage
-                      id="xpack.ml.fieldDataCard.loadingLabel"
-                      defaultMessage="Loading"
-                    />
-                  </EuiText>
-                </EuiFlexItem>
-              </EuiFlexGroup>
-            </Fragment>
-          )}
-          {loading === false && existsInDocs === true && getCardContent()}
-          {loading === false && existsInDocs === false && (
-            <Fragment>
-              <EuiSpacer size="xxl" />
-              <EuiFlexGroup justifyContent="spaceAround">
-                <EuiFlexItem grow={false}>
-                  <EuiFlexGroup gutterSize="xs" justifyContent="spaceAround">
-                    <EuiFlexItem grow={false}>
-                      <EuiIcon type="alert" />
-                    </EuiFlexItem>
-                  </EuiFlexGroup>
-                  <EuiFlexGroup style={{ padding: '0px 16px', textAlign: 'center' }}>
-                    <EuiFlexItem grow={false}>
-                      <FormattedMessage
-                        id="xpack.ml.fieldDataCard.fieldNotInDocsLabel"
-                        defaultMessage="This field does not appear in any documents for the selected time range"
-                      />
-                    </EuiFlexItem>
-                  </EuiFlexGroup>
-                </EuiFlexItem>
-              </EuiFlexGroup>
-            </Fragment>
-          )}
+          {loading === true ? <LoadingIndicator /> : getCardContent()}
         </div>
       </div>
     </div>

diff --git a/...y/plugins/ml/public/data_visualizer/components/field_data_card/loading_indicator/index.ts b/...y/plugins/ml/public/data_visualizer/components/field_data_card/loading_indicator/index.ts
@@ -0,0 +1,7 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+export { LoadingIndicator } from './loading_indicator';
diff --git a/...public/data_visualizer/components/field_data_card/loading_indicator/loading_indicator.tsx b/...public/data_visualizer/components/field_data_card/loading_indicator/loading_indicator.tsx
@@ -0,0 +1,29 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React, { FC, Fragment } from 'react';
+
+import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner, EuiSpacer, EuiText } from '@elastic/eui';
+
+import { FormattedMessage } from '@kbn/i18n/react';
+
+export const LoadingIndicator: FC = () => (
+  <Fragment>
+    <EuiSpacer size="xxl" />
+    <EuiFlexGroup justifyContent="spaceAround">
+      <EuiFlexItem grow={false}>
+        <EuiLoadingSpinner size="l" />
+      </EuiFlexItem>
+    </EuiFlexGroup>
+    <EuiFlexGroup justifyContent="spaceAround">
+      <EuiFlexItem grow={false}>
+        <EuiText color="subdued">
+          <FormattedMessage id="xpack.ml.fieldDataCard.loadingLabel" defaultMessage="Loading" />
+        </EuiText>
+      </EuiFlexItem>
+    </EuiFlexGroup>
+  </Fragment>
+);
diff --git a/...alizer/components/field_data_card/metric_distribution_chart/metric_distribution_chart.tsx b/...alizer/components/field_data_card/metric_distribution_chart/metric_distribution_chart.tsx
@@ -23,11 +23,11 @@ import {
   TooltipValue,
 } from '@elastic/charts';
 
-import chrome from 'ui/chrome';
 import darkTheme from '@elastic/eui/dist/eui_theme_dark.json';
 import lightTheme from '@elastic/eui/dist/eui_theme_light.json';
 
 import { MetricDistributionChartTooltipHeader } from './metric_distribution_chart_tooltip_header';
+import { useUiChromeContext } from '../../../../contexts/ui/use_ui_chrome_context';
 // @ts-ignore
 import { kibanaFieldFormat } from '../../../../formatters/kibana_field_format';
 
@@ -48,15 +48,6 @@ interface Props {
 
 const SPEC_ID = 'metric_distribution';
 
-const IS_DARK_THEME = chrome.getUiSettingsClient().get('theme:darkMode');
-const themeName = IS_DARK_THEME ? darkTheme : lightTheme;
-const EVENT_RATE_COLOR = themeName.euiColorVis1;
-const seriesColorValues: DataSeriesColorsValues = {
-  colorValues: [],
-  specId: getSpecId(SPEC_ID),
-};
-const seriesColors = new Map([[seriesColorValues, EVENT_RATE_COLOR]]);
-
 export const MetricDistributionChart: FC<Props> = ({ width, height, chartData, fieldFormat }) => {
   // This value is shown to label the y axis values in the tooltip.
   // Ideally we wouldn't show these values at all in the tooltip,
@@ -65,6 +56,18 @@ export const MetricDistributionChart: FC<Props> = ({ width, height, chartData, f
     defaultMessage: 'distribution',
   });
 
+  // TODO - switch to use inline areaSeriesStyle to set series fill once charts version is 8.0.0+
+  const IS_DARK_THEME = useUiChromeContext()
+    .getUiSettingsClient()
+    .get('theme:darkMode');
+  const themeName = IS_DARK_THEME ? darkTheme : lightTheme;
+  const EVENT_RATE_COLOR = themeName.euiColorVis1;
+  const seriesColorValues: DataSeriesColorsValues = {
+    colorValues: [],
+    specId: getSpecId(SPEC_ID),
+  };
+  const seriesColors = new Map([[seriesColorValues, EVENT_RATE_COLOR]]);
+
   const headerFormatter: TooltipValueFormatter = (tooltipData: TooltipValue) => {
     const xValue = tooltipData.value;
     const chartPoint: MetricDistributionChartData | undefined = chartData.find(