From 14fd0061e4c009ec537d8a4e748a8bb26b85a99b Mon Sep 17 00:00:00 2001
From: Dima Arnautov <dmitrii.arnautov@elastic.co>
Date: Fri, 17 Jul 2020 17:05:53 +0200
Subject: [PATCH] [ML] improve annotation flyout performance

---
 .../annotations/annotation_flyout/index.tsx   | 270 ++++++++++--------
 1 file changed, 148 insertions(+), 122 deletions(-)

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 2196f3d6cc6d2..84abe3ed8a821 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, Fragment, FC, ReactNode } from 'react';
+import React, { Component, FC, ReactNode, useCallback } from 'react';
 import useObservable from 'react-use/lib/useObservable';
 import * as Rx from 'rxjs';
 import { cloneDeep } from 'lodash';
@@ -28,6 +28,7 @@ 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,
@@ -60,7 +61,6 @@ interface Entity {
 }
 
 interface Props {
-  annotation: AnnotationState;
   chartDetails: {
     entityData: { entities: Entity[] };
     functionLabel: string;
@@ -70,25 +70,39 @@ interface Props {
 }
 
 interface State {
+  annotationState: AnnotationState | null;
   isDeleteModalVisible: boolean;
   applyAnnotationToSeries: boolean;
 }
 
-class AnnotationFlyoutUI extends Component<CommonProps & Props> {
+export class AnnotationFlyoutUI extends Component<CommonProps & Props> {
   public state: State = {
     isDeleteModalVisible: false,
     applyAnnotationToSeries: true,
+    annotationState: null,
   };
 
   public annotationSub: Rx.Subscription | null = null;
 
+  componentDidMount() {
+    this.annotationSub = annotation$.subscribe((v) => {
+      this.setState({
+        annotationState: v,
+      });
+    });
+  }
+
+  componentWillUnmount() {
+    this.annotationSub!.unsubscribe();
+  }
+
   public annotationTextChangeHandler = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
-    if (this.props.annotation === null) {
+    if (this.state.annotationState === null) {
       return;
     }
 
     annotation$.next({
-      ...this.props.annotation,
+      ...this.state.annotationState,
       annotation: e.target.value,
     });
   };
@@ -102,21 +116,21 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
   };
 
   public deleteHandler = async () => {
-    const { annotation } = this.props;
+    const { annotationState } = this.state;
     const toastNotifications = getToastNotifications();
 
-    if (annotation === null || annotation._id === undefined) {
+    if (annotationState === null || annotationState._id === undefined) {
       return;
     }
 
     try {
-      await ml.annotations.deleteAnnotation(annotation._id);
+      await ml.annotations.deleteAnnotation(annotationState._id);
       toastNotifications.addSuccess(
         i18n.translate(
           'xpack.ml.timeSeriesExplorer.timeSeriesChart.deletedAnnotationNotificationMessage',
           {
             defaultMessage: 'Deleted annotation for job with ID {jobId}.',
-            values: { jobId: annotation.job_id },
+            values: { jobId: annotationState.job_id },
           }
         )
       );
@@ -127,7 +141,7 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
           {
             defaultMessage:
               'An error occurred deleting the annotation for job with ID {jobId}: {error}',
-            values: { jobId: annotation.job_id, error: JSON.stringify(err) },
+            values: { jobId: annotationState.job_id, error: JSON.stringify(err) },
           }
         )
       );
@@ -145,13 +159,13 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
   public validateAnnotationText = () => {
     // Validates the entered text, returning an array of error messages
     // for display in the form. An empty array is returned if the text is valid.
-    const { annotation } = this.props;
+    const { annotationState } = this.state;
     const errors: string[] = [];
-    if (annotation === null) {
+    if (annotationState === null) {
       return errors;
     }
 
-    if (annotation.annotation.trim().length === 0) {
+    if (annotationState.annotation.trim().length === 0) {
       errors.push(
         i18n.translate('xpack.ml.timeSeriesExplorer.annotationFlyout.noAnnotationTextError', {
           defaultMessage: 'Enter annotation text',
@@ -159,7 +173,7 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
       );
     }
 
-    const textLength = annotation.annotation.length;
+    const textLength = annotationState.annotation.length;
     if (textLength > ANNOTATION_MAX_LENGTH_CHARS) {
       const charsOver = textLength - ANNOTATION_MAX_LENGTH_CHARS;
       errors.push(
@@ -178,7 +192,8 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
   };
 
   public saveOrUpdateAnnotation = () => {
-    const { annotation: originalAnnotation, chartDetails, detectorIndex } = this.props;
+    const { annotationState: originalAnnotation } = this.state;
+    const { chartDetails, detectorIndex } = this.props;
     if (originalAnnotation === null) {
       return;
     }
@@ -262,14 +277,12 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
   };
 
   public render(): ReactNode {
-    const { annotation, detectors, detectorIndex } = this.props;
-    const { isDeleteModalVisible } = this.state;
+    const { detectors, detectorIndex } = this.props;
+    const { annotationState, isDeleteModalVisible } = this.state;
 
-    if (annotation === null) {
-      return null;
-    }
+    if (!annotationState) return null;
 
-    const isExistingAnnotation = typeof annotation._id !== 'undefined';
+    const isExistingAnnotation = typeof annotationState._id !== 'undefined';
 
     // Check the length of the text is within the max length limit,
     // and warn if the length is approaching the limit.
@@ -279,14 +292,16 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
     let helpText = null;
     if (
       isInvalid === false &&
-      annotation.annotation.length > ANNOTATION_MAX_LENGTH_CHARS * lengthRatioToShowWarning
+      annotationState.annotation.length > ANNOTATION_MAX_LENGTH_CHARS * lengthRatioToShowWarning
     ) {
       helpText = i18n.translate(
         'xpack.ml.timeSeriesExplorer.annotationFlyout.approachingMaxLengthWarning',
         {
           defaultMessage:
             '{charsRemaining, number} {charsRemaining, plural, one {character} other {characters}} remaining',
-          values: { charsRemaining: ANNOTATION_MAX_LENGTH_CHARS - annotation.annotation.length },
+          values: {
+            charsRemaining: ANNOTATION_MAX_LENGTH_CHARS - annotationState.annotation.length,
+          },
         }
       );
     }
@@ -295,127 +310,138 @@ class AnnotationFlyoutUI extends Component<CommonProps & Props> {
       detector && 'detector_description' in detector ? detector.detector_description : '';
 
     return (
-      <Fragment>
-        <EuiFlyout onClose={this.cancelEditingHandler} size="m" aria-labelledby="Add annotation">
-          <EuiFlyoutHeader hasBorder>
-            <EuiTitle size="s">
-              <h2 id="mlAnnotationFlyoutTitle">
-                {isExistingAnnotation ? (
-                  <FormattedMessage
-                    id="xpack.ml.timeSeriesExplorer.annotationFlyout.editAnnotationTitle"
-                    defaultMessage="Edit annotation"
-                  />
-                ) : (
-                  <FormattedMessage
-                    id="xpack.ml.timeSeriesExplorer.annotationFlyout.addAnnotationTitle"
-                    defaultMessage="Add annotation"
-                  />
-                )}
-              </h2>
-            </EuiTitle>
-          </EuiFlyoutHeader>
-          <EuiFlyoutBody>
-            <AnnotationDescriptionList
-              annotation={annotation}
-              detectorDescription={detectorDescription}
+      <>
+        <EuiFlyoutBody>
+          <AnnotationDescriptionList
+            annotation={annotationState}
+            detectorDescription={detectorDescription}
+          />
+          <EuiSpacer size="m" />
+          <EuiFormRow
+            label={
+              <FormattedMessage
+                id="xpack.ml.timeSeriesExplorer.annotationFlyout.annotationTextLabel"
+                defaultMessage="Annotation text"
+              />
+            }
+            fullWidth
+            helpText={helpText}
+            isInvalid={isInvalid}
+            error={validationErrors}
+          >
+            <EuiTextArea
+              fullWidth
+              isInvalid={isInvalid}
+              onChange={this.annotationTextChangeHandler}
+              placeholder="..."
+              value={annotationState.annotation}
             />
-            <EuiSpacer size="m" />
-            <EuiFormRow
+          </EuiFormRow>
+          <EuiFormRow>
+            <EuiCheckbox
+              id={'xpack.ml.annotationFlyout.applyToPartition'}
               label={
                 <FormattedMessage
-                  id="xpack.ml.timeSeriesExplorer.annotationFlyout.annotationTextLabel"
-                  defaultMessage="Annotation text"
+                  id="xpack.ml.annotationFlyout.applyToPartitionTextLabel"
+                  defaultMessage="Apply annotation to this series"
                 />
               }
-              fullWidth
-              helpText={helpText}
-              isInvalid={isInvalid}
-              error={validationErrors}
-            >
-              <EuiTextArea
-                fullWidth
-                isInvalid={isInvalid}
-                onChange={this.annotationTextChangeHandler}
-                placeholder="..."
-                value={annotation.annotation}
-              />
-            </EuiFormRow>
-            <EuiFormRow>
-              <EuiCheckbox
-                id={'xpack.ml.annotationFlyout.applyToPartition'}
-                label={
+              checked={this.state.applyAnnotationToSeries}
+              onChange={() =>
+                this.setState({
+                  applyAnnotationToSeries: !this.state.applyAnnotationToSeries,
+                })
+              }
+            />
+          </EuiFormRow>
+        </EuiFlyoutBody>
+        <EuiFlyoutFooter>
+          <EuiFlexGroup justifyContent="spaceBetween">
+            <EuiFlexItem grow={false}>
+              <EuiButtonEmpty iconType="cross" onClick={this.cancelEditingHandler} flush="left">
+                <FormattedMessage
+                  id="xpack.ml.timeSeriesExplorer.annotationFlyout.cancelButtonLabel"
+                  defaultMessage="Cancel"
+                />
+              </EuiButtonEmpty>
+            </EuiFlexItem>
+            <EuiFlexItem grow={false}>
+              {isExistingAnnotation && (
+                <EuiButtonEmpty color="danger" onClick={this.deleteConfirmHandler}>
                   <FormattedMessage
-                    id="xpack.ml.annotationFlyout.applyToPartitionTextLabel"
-                    defaultMessage="Apply annotation to this series"
+                    id="xpack.ml.timeSeriesExplorer.annotationFlyout.deleteButtonLabel"
+                    defaultMessage="Delete"
                   />
-                }
-                checked={this.state.applyAnnotationToSeries}
-                onChange={() =>
-                  this.setState({
-                    applyAnnotationToSeries: !this.state.applyAnnotationToSeries,
-                  })
-                }
-              />
-            </EuiFormRow>
-          </EuiFlyoutBody>
-          <EuiFlyoutFooter>
-            <EuiFlexGroup justifyContent="spaceBetween">
-              <EuiFlexItem grow={false}>
-                <EuiButtonEmpty iconType="cross" onClick={this.cancelEditingHandler} flush="left">
+                </EuiButtonEmpty>
+              )}
+            </EuiFlexItem>
+            <EuiFlexItem grow={false}>
+              <EuiButton fill isDisabled={isInvalid === true} onClick={this.saveOrUpdateAnnotation}>
+                {isExistingAnnotation ? (
                   <FormattedMessage
-                    id="xpack.ml.timeSeriesExplorer.annotationFlyout.cancelButtonLabel"
-                    defaultMessage="Cancel"
+                    id="xpack.ml.timeSeriesExplorer.annotationFlyout.updateButtonLabel"
+                    defaultMessage="Update"
+                  />
+                ) : (
+                  <FormattedMessage
+                    id="xpack.ml.timeSeriesExplorer.annotationFlyout.createButtonLabel"
+                    defaultMessage="Create"
                   />
-                </EuiButtonEmpty>
-              </EuiFlexItem>
-              <EuiFlexItem grow={false}>
-                {isExistingAnnotation && (
-                  <EuiButtonEmpty color="danger" onClick={this.deleteConfirmHandler}>
-                    <FormattedMessage
-                      id="xpack.ml.timeSeriesExplorer.annotationFlyout.deleteButtonLabel"
-                      defaultMessage="Delete"
-                    />
-                  </EuiButtonEmpty>
                 )}
-              </EuiFlexItem>
-              <EuiFlexItem grow={false}>
-                <EuiButton
-                  fill
-                  isDisabled={isInvalid === true}
-                  onClick={this.saveOrUpdateAnnotation}
-                >
-                  {isExistingAnnotation ? (
-                    <FormattedMessage
-                      id="xpack.ml.timeSeriesExplorer.annotationFlyout.updateButtonLabel"
-                      defaultMessage="Update"
-                    />
-                  ) : (
-                    <FormattedMessage
-                      id="xpack.ml.timeSeriesExplorer.annotationFlyout.createButtonLabel"
-                      defaultMessage="Create"
-                    />
-                  )}
-                </EuiButton>
-              </EuiFlexItem>
-            </EuiFlexGroup>
-          </EuiFlyoutFooter>
-        </EuiFlyout>
+              </EuiButton>
+            </EuiFlexItem>
+          </EuiFlexGroup>
+        </EuiFlyoutFooter>
         <DeleteAnnotationModal
           cancelAction={this.closeDeleteModal}
           deleteAction={this.deleteHandler}
           isVisible={isDeleteModalVisible}
         />
-      </Fragment>
+      </>
     );
   }
 }
 
 export const AnnotationFlyout: FC<any> = (props) => {
-  const annotationProp = useObservable(annotation$);
+  const annotationProp = useObservable(
+    annotation$.pipe(
+      distinctUntilChanged((prev, curr) => {
+        // prevent re-rendering
+        return prev !== null && curr !== null;
+      })
+    )
+  );
 
-  if (annotationProp === undefined) {
+  const cancelEditingHandler = useCallback(() => {
+    annotation$.next(null);
+  }, []);
+
+  if (annotationProp === undefined || annotationProp === null) {
     return null;
   }
 
-  return <AnnotationFlyoutUI annotation={annotationProp} {...props} />;
+  const isExistingAnnotation = typeof annotationProp._id !== 'undefined';
+
+  return (
+    <EuiFlyout onClose={cancelEditingHandler} size="m" aria-labelledby="Add annotation">
+      <EuiFlyoutHeader hasBorder>
+        <EuiTitle size="s">
+          <h2 id="mlAnnotationFlyoutTitle">
+            {isExistingAnnotation ? (
+              <FormattedMessage
+                id="xpack.ml.timeSeriesExplorer.annotationFlyout.editAnnotationTitle"
+                defaultMessage="Edit annotation"
+              />
+            ) : (
+              <FormattedMessage
+                id="xpack.ml.timeSeriesExplorer.annotationFlyout.addAnnotationTitle"
+                defaultMessage="Add annotation"
+              />
+            )}
+          </h2>
+        </EuiTitle>
+      </EuiFlyoutHeader>
+      <AnnotationFlyoutUI {...props} />
+    </EuiFlyout>
+  );
 };