From 7a7057eba7270042a230ff4ac4f2404145357312 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen <43350163+qn895@users.noreply.github.com> Date: Wed, 18 Nov 2020 10:49:26 -0600 Subject: [PATCH] [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (#83216) --- .../__snapshots__/index.test.tsx.snap | 3 - .../annotation_flyout/index.test.tsx | 122 ++++++++++++------ .../annotations/annotation_flyout/index.tsx | 61 +++++---- .../annotations_table.test.js.snap | 6 +- .../annotations_table/annotations_table.js | 22 ++-- .../ml/ml_annotation_updates_context.ts | 14 ++ .../application/routing/routes/explorer.tsx | 9 +- .../application/routing/routes/jobs_list.tsx | 9 +- .../routing/routes/timeseriesexplorer.tsx | 16 ++- .../services/annotations_service.test.tsx | 19 ++- .../services/annotations_service.tsx | 26 +++- .../timeseries_chart/timeseries_chart.d.ts | 8 +- .../timeseries_chart/timeseries_chart.js | 27 ++-- .../timeseries_chart_annotations.ts | 14 +- .../timeseries_chart_with_tooltip.tsx | 6 +- .../timeseriesexplorer/timeseriesexplorer.js | 1 - 16 files changed, 244 insertions(+), 119 deletions(-) delete mode 100644 x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/__snapshots__/index.test.tsx.snap create mode 100644 x-pack/plugins/ml/public/application/contexts/ml/ml_annotation_updates_context.ts diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/__snapshots__/index.test.tsx.snap b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/__snapshots__/index.test.tsx.snap deleted file mode 100644 index dba73c246c3d0..0000000000000 --- a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/__snapshots__/index.test.tsx.snap +++ /dev/null @@ -1,3 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`AnnotationFlyout Initialization. 1`] = `""`; diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx index a4d2cd6b091a8..5ad175e2792b7 100644 --- a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx +++ b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx @@ -5,58 +5,102 @@ */ import useObservable from 'react-use/lib/useObservable'; - import mockAnnotations from '../annotations_table/__mocks__/mock_annotations.json'; - import React from 'react'; -import { mountWithIntl, shallowWithIntl } from '@kbn/test/jest'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; import { Annotation } from '../../../../../common/types/annotations'; -import { annotation$ } from '../../../services/annotations_service'; +import { AnnotationUpdatesService } from '../../../services/annotations_service'; import { AnnotationFlyout } from './index'; +import { MlAnnotationUpdatesContext } from '../../../contexts/ml/ml_annotation_updates_context'; + +jest.mock('../../../util/dependency_cache', () => ({ + getToastNotifications: () => ({ addSuccess: jest.fn(), addDanger: jest.fn() }), +})); + +const MlAnnotationUpdatesContextProvider = ({ + annotationUpdatesService, + children, +}: { + annotationUpdatesService: AnnotationUpdatesService; + children: React.ReactElement; +}) => { + return ( + + {children} + + ); +}; + +const ObservableComponent = (props: any) => { + const { annotationUpdatesService } = props; + const annotationProp = useObservable(annotationUpdatesService!.isAnnotationInitialized$()); + if (annotationProp === undefined) { + return null; + } + return ( + + ); +}; describe('AnnotationFlyout', () => { - test('Initialization.', () => { - const wrapper = shallowWithIntl(); - expect(wrapper).toMatchSnapshot(); + let annotationUpdatesService: AnnotationUpdatesService | null = null; + beforeEach(() => { + annotationUpdatesService = new AnnotationUpdatesService(); }); - test('Update button is disabled with empty annotation', () => { + test('Update button is disabled with empty annotation', async () => { const annotation = mockAnnotations[1] as Annotation; - annotation$.next(annotation); - - // useObservable wraps the observable in a new component - const ObservableComponent = (props: any) => { - const annotationProp = useObservable(annotation$); - if (annotationProp === undefined) { - return null; - } - return ; - }; - - const wrapper = mountWithIntl(); - const updateBtn = wrapper.find('EuiButton').first(); - expect(updateBtn.prop('isDisabled')).toEqual(true); + + annotationUpdatesService!.setValue(annotation); + + const { getByTestId } = render( + + + + ); + const updateBtn = getByTestId('annotationFlyoutUpdateButton'); + expect(updateBtn).toBeDisabled(); }); - test('Error displayed and update button displayed if annotation text is longer than max chars', () => { + test('Error displayed and update button displayed if annotation text is longer than max chars', async () => { const annotation = mockAnnotations[2] as Annotation; - annotation$.next(annotation); - - // useObservable wraps the observable in a new component - const ObservableComponent = (props: any) => { - const annotationProp = useObservable(annotation$); - if (annotationProp === undefined) { - return null; - } - return ; - }; - - const wrapper = mountWithIntl(); - const updateBtn = wrapper.find('EuiButton').first(); - expect(updateBtn.prop('isDisabled')).toEqual(true); - - expect(wrapper.find('EuiFormErrorText')).toHaveLength(1); + annotationUpdatesService!.setValue(annotation); + + const { getByTestId } = render( + + + + ); + const updateBtn = getByTestId('annotationFlyoutUpdateButton'); + expect(updateBtn).toBeDisabled(); + await waitFor(() => { + const errorText = screen.queryByText(/characters above maximum length/); + expect(errorText).not.toBe(undefined); + }); + }); + + test('Flyout disappears when annotation is updated', async () => { + const annotation = mockAnnotations[0] as Annotation; + + annotationUpdatesService!.setValue(annotation); + + const { getByTestId } = render( + + + + ); + const updateBtn = getByTestId('annotationFlyoutUpdateButton'); + expect(updateBtn).not.toBeDisabled(); + expect(screen.queryByTestId('mlAnnotationFlyout')).toBeInTheDocument(); + + await fireEvent.click(updateBtn); + expect(screen.queryByTestId('mlAnnotationFlyout')).not.toBeInTheDocument(); }); }); diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx index 84abe3ed8a821..88996772f49d6 100644 --- a/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx +++ b/x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Component, FC, ReactNode, useCallback } from 'react'; +import React, { Component, FC, ReactNode, useCallback, useContext } from 'react'; import useObservable from 'react-use/lib/useObservable'; import * as Rx from 'rxjs'; import { cloneDeep } from 'lodash'; @@ -28,15 +28,14 @@ import { import { CommonProps } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; -import { distinctUntilChanged } from 'rxjs/operators'; import { ANNOTATION_MAX_LENGTH_CHARS, ANNOTATION_EVENT_USER, } from '../../../../../common/constants/annotations'; import { - annotation$, annotationsRefreshed, AnnotationState, + AnnotationUpdatesService, } from '../../../services/annotations_service'; import { AnnotationDescriptionList } from '../annotation_description_list'; import { DeleteAnnotationModal } from '../delete_annotation_modal'; @@ -48,6 +47,7 @@ import { } from '../../../../../common/types/annotations'; import { PartitionFieldsType } from '../../../../../common/types/anomalies'; import { PARTITION_FIELDS } from '../../../../../common/constants/anomalies'; +import { MlAnnotationUpdatesContext } from '../../../contexts/ml/ml_annotation_updates_context'; interface ViewableDetector { index: number; @@ -67,6 +67,7 @@ interface Props { }; detectorIndex: number; detectors: ViewableDetector[]; + annotationUpdatesService: AnnotationUpdatesService; } interface State { @@ -85,7 +86,8 @@ export class AnnotationFlyoutUI extends Component { public annotationSub: Rx.Subscription | null = null; componentDidMount() { - this.annotationSub = annotation$.subscribe((v) => { + const { annotationUpdatesService } = this.props; + this.annotationSub = annotationUpdatesService.update$().subscribe((v) => { this.setState({ annotationState: v, }); @@ -100,15 +102,17 @@ export class AnnotationFlyoutUI extends Component { if (this.state.annotationState === null) { return; } + const { annotationUpdatesService } = this.props; - annotation$.next({ + annotationUpdatesService.setValue({ ...this.state.annotationState, annotation: e.target.value, }); }; public cancelEditingHandler = () => { - annotation$.next(null); + const { annotationUpdatesService } = this.props; + annotationUpdatesService.setValue(null); }; public deleteConfirmHandler = () => { @@ -148,7 +152,10 @@ export class AnnotationFlyoutUI extends Component { } this.closeDeleteModal(); - annotation$.next(null); + + const { annotationUpdatesService } = this.props; + + annotationUpdatesService.setValue(null); annotationsRefreshed(); }; @@ -193,7 +200,8 @@ export class AnnotationFlyoutUI extends Component { public saveOrUpdateAnnotation = () => { const { annotationState: originalAnnotation } = this.state; - const { chartDetails, detectorIndex } = this.props; + const { chartDetails, detectorIndex, annotationUpdatesService } = this.props; + if (originalAnnotation === null) { return; } @@ -218,8 +226,7 @@ export class AnnotationFlyoutUI extends Component { } // Mark the annotation created by `user` if and only if annotation is being created, not updated annotation.event = annotation.event ?? ANNOTATION_EVENT_USER; - - annotation$.next(null); + annotationUpdatesService.setValue(null); ml.annotations .indexAnnotation(annotation) @@ -356,16 +363,16 @@ export class AnnotationFlyoutUI extends Component { - + - + - + {isExistingAnnotation && ( { )} - + {isExistingAnnotation ? ( { } export const AnnotationFlyout: FC = (props) => { - const annotationProp = useObservable( - annotation$.pipe( - distinctUntilChanged((prev, curr) => { - // prevent re-rendering - return prev !== null && curr !== null; - }) - ) - ); + const annotationUpdatesService = useContext(MlAnnotationUpdatesContext); + const annotationProp = useObservable(annotationUpdatesService.isAnnotationInitialized$()); const cancelEditingHandler = useCallback(() => { - annotation$.next(null); + annotationUpdatesService.setValue(null); }, []); if (annotationProp === undefined || annotationProp === null) { @@ -423,7 +429,12 @@ export const AnnotationFlyout: FC = (props) => { const isExistingAnnotation = typeof annotationProp._id !== 'undefined'; return ( - +

@@ -441,7 +452,7 @@ export const AnnotationFlyout: FC = (props) => {

- + ); }; diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotations_table/__snapshots__/annotations_table.test.js.snap b/x-pack/plugins/ml/public/application/components/annotations/annotations_table/__snapshots__/annotations_table.test.js.snap index 114a6b235d1ad..0c6fa6669c2eb 100644 --- a/x-pack/plugins/ml/public/application/components/annotations/annotations_table/__snapshots__/annotations_table.test.js.snap +++ b/x-pack/plugins/ml/public/application/components/annotations/annotations_table/__snapshots__/annotations_table.test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`AnnotationsTable Initialization with annotations prop. 1`] = ` - annotation$.next(originalAnnotation ?? annotation)} + onClick={() => annotationUpdatesService.setValue(originalAnnotation ?? annotation)} iconType="pencil" aria-label={editAnnotationsTooltipAriaLabelText} /> @@ -693,4 +694,7 @@ class AnnotationsTableUI extends Component { } } -export const AnnotationsTable = withKibana(AnnotationsTableUI); +export const AnnotationsTable = withKibana((props) => { + const annotationUpdatesService = useContext(MlAnnotationUpdatesContext); + return ; +}); diff --git a/x-pack/plugins/ml/public/application/contexts/ml/ml_annotation_updates_context.ts b/x-pack/plugins/ml/public/application/contexts/ml/ml_annotation_updates_context.ts new file mode 100644 index 0000000000000..37dea3029c8ad --- /dev/null +++ b/x-pack/plugins/ml/public/application/contexts/ml/ml_annotation_updates_context.ts @@ -0,0 +1,14 @@ +/* + * 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 { createContext } from 'react'; +import { AnnotationUpdatesService } from '../../services/annotations_service'; + +export type MlAnnotationUpdatesContextValue = AnnotationUpdatesService; + +export const MlAnnotationUpdatesContext = createContext( + new AnnotationUpdatesService() +); diff --git a/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx index cb6944e0ecf05..b91a5bd4a1aa4 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FC, useEffect, useState, useCallback } from 'react'; +import React, { FC, useEffect, useState, useCallback, useMemo } from 'react'; import useObservable from 'react-use/lib/useObservable'; import { i18n } from '@kbn/i18n'; @@ -34,6 +34,8 @@ import { getBreadcrumbWithUrlForApp } from '../breadcrumbs'; import { useTimefilter } from '../../contexts/kibana'; import { isViewBySwimLaneData } from '../../explorer/swimlane_container'; import { JOB_ID } from '../../../../common/constants/anomalies'; +import { MlAnnotationUpdatesContext } from '../../contexts/ml/ml_annotation_updates_context'; +import { AnnotationUpdatesService } from '../../services/annotations_service'; export const explorerRouteFactory = ( navigateToPath: NavigateToPath, @@ -59,10 +61,13 @@ const PageWrapper: FC = ({ deps }) => { jobs: mlJobService.loadJobsWrapper, jobsWithTimeRange: () => ml.jobs.jobsWithTimerange(getDateFormatTz()), }); + const annotationUpdatesService = useMemo(() => new AnnotationUpdatesService(), []); return ( - + + + ); }; diff --git a/x-pack/plugins/ml/public/application/routing/routes/jobs_list.tsx b/x-pack/plugins/ml/public/application/routing/routes/jobs_list.tsx index 2863e59508e35..d91ec27d9a505 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/jobs_list.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/jobs_list.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useEffect, FC } from 'react'; +import React, { useEffect, FC, useMemo } from 'react'; import useObservable from 'react-use/lib/useObservable'; import { i18n } from '@kbn/i18n'; @@ -19,6 +19,8 @@ import { basicResolvers } from '../resolvers'; import { JobsPage } from '../../jobs/jobs_list'; import { useTimefilter } from '../../contexts/kibana'; import { getBreadcrumbWithUrlForApp } from '../breadcrumbs'; +import { AnnotationUpdatesService } from '../../services/annotations_service'; +import { MlAnnotationUpdatesContext } from '../../contexts/ml/ml_annotation_updates_context'; export const jobListRouteFactory = (navigateToPath: NavigateToPath, basePath: string): MlRoute => ({ path: '/jobs', @@ -57,10 +59,13 @@ const PageWrapper: FC = ({ deps }) => { setGlobalState({ refreshInterval }); timefilter.setRefreshInterval(refreshInterval); }, []); + const annotationUpdatesService = useMemo(() => new AnnotationUpdatesService(), []); return ( - + + + ); }; diff --git a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx index 9331fdc04b7bb..2653781ce1a30 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx @@ -5,7 +5,7 @@ */ import { isEqual } from 'lodash'; -import React, { FC, useCallback, useEffect, useState } from 'react'; +import React, { FC, useCallback, useEffect, useMemo, useState } from 'react'; import usePrevious from 'react-use/lib/usePrevious'; import moment from 'moment'; @@ -39,7 +39,8 @@ import { basicResolvers } from '../resolvers'; import { getBreadcrumbWithUrlForApp } from '../breadcrumbs'; import { useTimefilter } from '../../contexts/kibana'; import { useToastNotificationService } from '../../services/toast_notification_service'; - +import { AnnotationUpdatesService } from '../../services/annotations_service'; +import { MlAnnotationUpdatesContext } from '../../contexts/ml/ml_annotation_updates_context'; export const timeSeriesExplorerRouteFactory = ( navigateToPath: NavigateToPath, basePath: string @@ -64,13 +65,16 @@ const PageWrapper: FC = ({ deps }) => { jobs: mlJobService.loadJobsWrapper, jobsWithTimeRange: () => ml.jobs.jobsWithTimerange(getDateFormatTz()), }); + const annotationUpdatesService = useMemo(() => new AnnotationUpdatesService(), []); return ( - + + + ); }; diff --git a/x-pack/plugins/ml/public/application/services/annotations_service.test.tsx b/x-pack/plugins/ml/public/application/services/annotations_service.test.tsx index 2ba54d243ed1b..969748acc6af8 100644 --- a/x-pack/plugins/ml/public/application/services/annotations_service.test.tsx +++ b/x-pack/plugins/ml/public/application/services/annotations_service.test.tsx @@ -7,20 +7,29 @@ import mockAnnotations from '../components/annotations/annotations_table/__mocks__/mock_annotations.json'; import { Annotation } from '../../../common/types/annotations'; -import { annotation$, annotationsRefresh$, annotationsRefreshed } from './annotations_service'; - +import { + annotationsRefresh$, + annotationsRefreshed, + AnnotationUpdatesService, +} from './annotations_service'; describe('annotations_service', () => { - test('annotation$', () => { + let annotationUpdatesService: AnnotationUpdatesService | null = null; + + beforeEach(() => { + annotationUpdatesService = new AnnotationUpdatesService(); + }); + + test('annotationUpdatesService', () => { const subscriber = jest.fn(); - annotation$.subscribe(subscriber); + annotationUpdatesService!.update$().subscribe(subscriber); // the subscriber should have been triggered with the initial value of null expect(subscriber.mock.calls).toHaveLength(1); expect(subscriber.mock.calls[0][0]).toBe(null); const annotation = mockAnnotations[0] as Annotation; - annotation$.next(annotation); + annotationUpdatesService!.setValue(annotation); // the subscriber should have been triggered with the updated annotation value expect(subscriber.mock.calls).toHaveLength(2); diff --git a/x-pack/plugins/ml/public/application/services/annotations_service.tsx b/x-pack/plugins/ml/public/application/services/annotations_service.tsx index 6493770156cb8..208c3b6ca5827 100644 --- a/x-pack/plugins/ml/public/application/services/annotations_service.tsx +++ b/x-pack/plugins/ml/public/application/services/annotations_service.tsx @@ -4,8 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { BehaviorSubject } from 'rxjs'; - +import { BehaviorSubject, Observable } from 'rxjs'; +import { distinctUntilChanged } from 'rxjs/operators'; import { Annotation } from '../../../common/types/annotations'; /* @@ -79,3 +79,25 @@ export const annotation$ = new BehaviorSubject(null); */ export const annotationsRefresh$ = new BehaviorSubject(Date.now()); export const annotationsRefreshed = () => annotationsRefresh$.next(Date.now()); + +export class AnnotationUpdatesService { + private _annotation$: BehaviorSubject = new BehaviorSubject( + null + ); + + public update$() { + return this._annotation$.asObservable(); + } + public isAnnotationInitialized$(): Observable { + return this._annotation$.asObservable().pipe( + distinctUntilChanged((prev, curr) => { + // prevent re-rendering + return prev !== null && curr !== null; + }) + ); + } + + public setValue(annotation: AnnotationState) { + this._annotation$.next(annotation); + } +} diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.d.ts b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.d.ts index 04b666b4fc684..f58a399f5e3de 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.d.ts +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.d.ts @@ -10,6 +10,7 @@ import React from 'react'; import { Annotation } from '../../../../../common/types/annotations'; import { CombinedJob } from '../../../../../common/types/anomaly_detection_jobs'; import { ChartTooltipService } from '../../../components/chart_tooltip'; +import { AnnotationState, AnnotationUpdatesService } from '../../../services/annotations_service'; interface Props { selectedJob: CombinedJob; @@ -47,6 +48,11 @@ interface TimeseriesChartProps { tooltipService: object; } -declare class TimeseriesChart extends React.Component { +interface TimeseriesChartIntProps { + annotationUpdatesService: AnnotationUpdatesService; + annotationProps: AnnotationState; +} + +declare class TimeseriesChart extends React.Component { focusXScale: d3.scale.Ordinal<{}, number>; } diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js index b2d054becbb1a..6f2beb8fe9067 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js @@ -10,7 +10,7 @@ */ import PropTypes from 'prop-types'; -import React, { Component } from 'react'; +import React, { Component, useContext } from 'react'; import useObservable from 'react-use/lib/useObservable'; import { isEqual, reduce, each, get } from 'lodash'; import d3 from 'd3'; @@ -21,7 +21,6 @@ import { getSeverityWithLow, getMultiBucketImpactLabel, } from '../../../../../common/util/anomaly_utils'; -import { annotation$ } from '../../../services/annotations_service'; import { formatValue } from '../../../formatters/format_value'; import { LINE_CHART_ANOMALY_RADIUS, @@ -51,7 +50,7 @@ import { unhighlightFocusChartAnnotation, ANNOTATION_MIN_WIDTH, } from './timeseries_chart_annotations'; -import { distinctUntilChanged } from 'rxjs/operators'; +import { MlAnnotationUpdatesContext } from '../../../contexts/ml/ml_annotation_updates_context'; const focusZoomPanelHeight = 25; const focusChartHeight = 310; @@ -571,7 +570,6 @@ class TimeseriesChartIntl extends Component { } renderFocusChart() { - console.log('renderFocusChart'); const { focusAggregationInterval, focusAnnotationData: focusAnnotationDataOriginalPropValue, @@ -742,7 +740,8 @@ class TimeseriesChartIntl extends Component { this.focusXScale, showAnnotations, showFocusChartTooltip, - hideFocusChartTooltip + hideFocusChartTooltip, + this.props.annotationUpdatesService ); // disable brushing (creation of annotations) when annotations aren't shown @@ -1800,17 +1799,17 @@ class TimeseriesChartIntl extends Component { } export const TimeseriesChart = (props) => { - const annotationProp = useObservable( - annotation$.pipe( - distinctUntilChanged((prev, curr) => { - // prevent re-rendering - return prev !== null && curr !== null; - }) - ) - ); + const annotationUpdatesService = useContext(MlAnnotationUpdatesContext); + const annotationProp = useObservable(annotationUpdatesService.isAnnotationInitialized$()); if (annotationProp === undefined) { return null; } - return ; + return ( + + ); }; diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_annotations.ts b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_annotations.ts index bd86d07dcd8b7..8757fbad19df3 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_annotations.ts +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_annotations.ts @@ -13,13 +13,14 @@ import { Dictionary } from '../../../../../common/types/common'; import { TimeseriesChart } from './timeseries_chart'; -import { annotation$ } from '../../../services/annotations_service'; +import { AnnotationUpdatesService } from '../../../services/annotations_service'; export const ANNOTATION_MASK_ID = 'mlAnnotationMask'; // getAnnotationBrush() is expected to be called like getAnnotationBrush.call(this) // so it gets passed on the context of the component it gets called from. export function getAnnotationBrush(this: TimeseriesChart) { + const { annotationUpdatesService } = this.props; const focusXScale = this.focusXScale; const annotateBrush = d3.svg.brush().x(focusXScale).on('brushend', brushend.bind(this)); @@ -35,7 +36,7 @@ export function getAnnotationBrush(this: TimeseriesChart) { const endTimestamp = extent[1].getTime(); if (timestamp === endTimestamp) { - annotation$.next(null); + annotationUpdatesService.setValue(null); return; } @@ -47,7 +48,7 @@ export function getAnnotationBrush(this: TimeseriesChart) { type: ANNOTATION_TYPE.ANNOTATION, }; - annotation$.next(annotation); + annotationUpdatesService.setValue(annotation); } return annotateBrush; @@ -105,7 +106,8 @@ export function renderAnnotations( focusXScale: TimeseriesChart['focusXScale'], showAnnotations: boolean, showFocusChartTooltip: (d: Annotation, t: object) => {}, - hideFocusChartTooltip: () => void + hideFocusChartTooltip: () => void, + annotationUpdatesService: AnnotationUpdatesService ) { const upperRectMargin = ANNOTATION_UPPER_RECT_MARGIN; const upperTextMargin = ANNOTATION_UPPER_TEXT_MARGIN; @@ -153,9 +155,9 @@ export function renderAnnotations( // clear a possible existing annotation set up for editing before setting the new one. // this needs to be done explicitly here because a new annotation created using the brush tool // could still be present in the chart. - annotation$.next(null); + annotationUpdatesService.setValue(null); // set the actual annotation and trigger the flyout - annotation$.next(d); + annotationUpdatesService.setValue(d); }); rects diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx index 89e7d292dbdf2..23e7740dd048e 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart_with_tooltip.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FC, useEffect, useState, useCallback } from 'react'; +import React, { FC, useEffect, useState, useCallback, useContext } from 'react'; import { i18n } from '@kbn/i18n'; import { MlTooltipComponent } from '../../../components/chart_tooltip'; import { TimeseriesChart } from './timeseries_chart'; @@ -16,6 +16,7 @@ import { useMlKibana, useNotifications } from '../../../contexts/kibana'; import { getBoundsRoundedToInterval } from '../../../util/time_buckets'; import { ANNOTATION_EVENT_USER } from '../../../../../common/constants/annotations'; import { getControlsForDetector } from '../../get_controls_for_detector'; +import { MlAnnotationUpdatesContext } from '../../../contexts/ml/ml_annotation_updates_context'; interface TimeSeriesChartWithTooltipsProps { bounds: any; @@ -50,6 +51,8 @@ export const TimeSeriesChartWithTooltips: FC = }, } = useMlKibana(); + const annotationUpdatesService = useContext(MlAnnotationUpdatesContext); + const [annotationData, setAnnotationData] = useState([]); const showAnnotationErrorToastNotification = useCallback((error?: string) => { @@ -123,6 +126,7 @@ export const TimeSeriesChartWithTooltips: FC = {(tooltipService) => ( {fieldNamesWithEmptyValues.length > 0 && (