[SIEM] Fix bugs on resizeable (#38714)

* fix resizeable * - refactored `calculateDelta` function * few unit tests * - added a unit test to verify that dragging is disabled during a column resize * cleanup * - calcuate mouse movement manually for Safari-only (movementX is more accurate, and thus preferred)
2019-06-11 22:11:28 -04:00 · 2019-06-11 22:11:28 -04:00 · 4b4b079b2d
parent f1a4552e9e
commit 4b4b079b2d
7 changed files with 239 additions and 129 deletions
--- a/x-pack/plugins/siem/public/components/resize_handle/index.test.tsx
+++ b/x-pack/plugins/siem/public/components/resize_handle/index.test.tsx
@ -14,9 +14,9 @@ import { TestProviders } from '../../mock/test_providers';
 import {
  addGlobalResizeCursorStyleToBody,
  globalResizeCursorClassName,
-  isResizing,
  removeGlobalResizeCursorStyleFromBody,
  Resizeable,
+  calculateDeltaX,
 } from '.';
 import { CellResizeHandle } from './styled_handles';

@ -118,39 +118,17 @@ describe('Resizeable', () => {
      });
    });

-    describe('#isResizing', () => {
-      test('it returns true when the global-resize-cursor is present on the body', () => {
-        mount(
-          <TestProviders>
-            <Resizeable
-              handle={<CellResizeHandle data-test-subj="test-resize-handle" />}
-              height="100%"
-              id="test"
-              onResize={jest.fn()}
-              render={() => <></>}
-            />
-          </TestProviders>
-        );
-
-        addGlobalResizeCursorStyleToBody();
-
-        expect(isResizing()).toEqual(true);
+    describe('#calculateDeltaX', () => {
+      test('it returns 0 when prevX isEqual 0', () => {
+        expect(calculateDeltaX({ prevX: 0, screenX: 189 })).toEqual(0);
      });

-      test('it returns false when the global-resize-cursor is NOT present on the body', () => {
-        mount(
-          <TestProviders>
-            <Resizeable
-              handle={<CellResizeHandle data-test-subj="test-resize-handle" />}
-              height="100%"
-              id="test"
-              onResize={jest.fn()}
-              render={() => <></>}
-            />
-          </TestProviders>
-        );
+      test('it returns positive difference when screenX > prevX', () => {
+        expect(calculateDeltaX({ prevX: 10, screenX: 189 })).toEqual(179);
+      });

-        expect(isResizing()).toEqual(false);
+      test('it returns negative difference when prevX > screenX ', () => {
+        expect(calculateDeltaX({ prevX: 199, screenX: 189 })).toEqual(-10);
      });
    });
  });
--- a/x-pack/plugins/siem/public/components/resize_handle/index.tsx
+++ b/x-pack/plugins/siem/public/components/resize_handle/index.tsx
@ -6,7 +6,7 @@

 import * as React from 'react';
 import { fromEvent, Observable, Subscription } from 'rxjs';
-import { mergeMap, takeUntil } from 'rxjs/operators';
+import { concatMap, takeUntil } from 'rxjs/operators';
 import styled, { injectGlobal } from 'styled-components';

 export type OnResize = (
@ -22,6 +22,12 @@ export type OnResize = (
 export const resizeCursorStyle = 'col-resize';
 export const globalResizeCursorClassName = 'global-resize-cursor';

+/** This polyfill is for Safari only. `movementX` is more accurate and "feels" better, so only use this function on Safari */
+export const calculateDeltaX = ({ prevX, screenX }: { prevX: number; screenX: number }) =>
+  prevX !== 0 ? screenX - prevX : 0;
+
+const isSafari = /^((?!chrome|android|crios|fxios|Firefox).)*safari/i.test(navigator.userAgent);
+
 // eslint-disable-next-line no-unused-expressions
 injectGlobal`
 .${globalResizeCursorClassName} {
@ -47,6 +53,10 @@ interface Props {
  onResize: OnResize;
 }

+interface State {
+  isResizing: boolean;
+}
+
 const ResizeHandleContainer = styled.div<{ height?: string }>`
  cursor: ${resizeCursorStyle};
  ${({ height }) => (height != null ? `height: ${height}` : '')}
@ -60,12 +70,11 @@ export const removeGlobalResizeCursorStyleFromBody = () => {
  document.body.classList.remove(globalResizeCursorClassName);
 };

-export const isResizing = () => document.body.className.includes(globalResizeCursorClassName);
-
-export class Resizeable extends React.PureComponent<Props> {
-  private drag$: Observable<Event> | null;
-  private ref: React.RefObject<HTMLElement>;
+export class Resizeable extends React.PureComponent<Props, State> {
+  private drag$: Observable<MouseEvent> | null;
  private dragSubscription: Subscription | null;
+  private prevX: number = 0;
+  private ref: React.RefObject<HTMLElement>;
  private upSubscription: Subscription | null;

  constructor(props: Props) {
@ -76,18 +85,26 @@ export class Resizeable extends React.PureComponent<Props> {
    this.drag$ = null;
    this.dragSubscription = null;
    this.upSubscription = null;
+
+    this.state = {
+      isResizing: false,
+    };
  }

  public componentDidMount() {
    const { id, onResize } = this.props;

-    const move$ = fromEvent(document, 'mousemove');
-    const down$ = fromEvent(this.ref.current!, 'mousedown');
-    const up$ = fromEvent(document, 'mouseup');
+    const move$ = fromEvent<MouseEvent>(document, 'mousemove');
+    const down$ = fromEvent<MouseEvent>(this.ref.current!, 'mousedown');
+    const up$ = fromEvent<MouseEvent>(document, 'mouseup');

-    this.drag$ = down$.pipe(mergeMap(() => move$.pipe(takeUntil(up$))));
+    this.drag$ = down$.pipe(concatMap(() => move$.pipe(takeUntil(up$))));
    this.dragSubscription = this.drag$.subscribe(e => {
-      const delta = (e as MouseEvent).movementX;
+      const delta = isSafari ? this.calculateDelta(e) : e.movementX;
+
+      if (!this.state.isResizing) {
+        this.setState({ isResizing: true });
+      }

      onResize({ id, delta });

@ -95,8 +112,10 @@ export class Resizeable extends React.PureComponent<Props> {
    });

    this.upSubscription = up$.subscribe(() => {
-      if (isResizing()) {
+      if (this.state.isResizing) {
        removeGlobalResizeCursorStyleFromBody();
+
+        this.setState({ isResizing: false });
      }
    });
  }
@ -116,7 +135,7 @@ export class Resizeable extends React.PureComponent<Props> {

    return (
      <>
-        {render(isResizing())}
+        {render(this.state.isResizing)}
        <ResizeHandleContainer
          data-test-subj="resize-handle-container"
          height={height}
@ -127,4 +146,12 @@ export class Resizeable extends React.PureComponent<Props> {
      </>
    );
  }
+
+  private calculateDelta = (e: MouseEvent) => {
+    const deltaX = calculateDeltaX({ prevX: this.prevX, screenX: e.screenX });
+
+    this.prevX = e.screenX;
+
+    return deltaX;
+  };
 }
--- a/x-pack/plugins/siem/public/components/resize_handle/is_resizing.tsx
+++ b/x-pack/plugins/siem/public/components/resize_handle/is_resizing.tsx
@ -0,0 +1,16 @@
+/*
+ * 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 { useState } from 'react';
+
+export const isContainerResizing = () => {
+  const [isResizing, setIsResizing] = useState(false);
+
+  return {
+    isResizing,
+    setIsResizing,
+  };
+};
--- a/x-pack/plugins/siem/public/components/timeline/body/column_headers/header/index.test.tsx
+++ b/x-pack/plugins/siem/public/components/timeline/body/column_headers/header/index.test.tsx
@ -37,6 +37,7 @@ describe('Header', () => {
        onColumnRemoved={jest.fn()}
        onColumnResized={jest.fn()}
        onColumnSorted={jest.fn()}
+        setIsResizing={jest.fn()}
        sort={sort}
        timelineId={timelineId}
      />
@ -54,6 +55,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={jest.fn()}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -77,6 +79,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={jest.fn()}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -105,6 +108,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={jest.fn()}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -129,6 +133,7 @@ describe('Header', () => {
              onColumnRemoved={jest.fn()}
              onColumnResized={jest.fn()}
              onColumnSorted={jest.fn()}
+              setIsResizing={jest.fn()}
              sort={sort}
              timelineId={timelineId}
            />
@ -156,6 +161,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={mockOnColumnSorted}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -184,6 +190,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={mockOnColumnSorted}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -209,6 +216,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={mockOnColumnSorted}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -234,6 +242,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={mockOnColumnSorted}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -347,6 +356,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={jest.fn()}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -370,6 +380,7 @@ describe('Header', () => {
            onColumnRemoved={jest.fn()}
            onColumnResized={jest.fn()}
            onColumnSorted={jest.fn()}
+            setIsResizing={jest.fn()}
            sort={sort}
            timelineId={timelineId}
          />
@ -379,4 +390,26 @@ describe('Header', () => {
      expect(wrapper.find('[data-test-subj="header-tooltip"]').exists()).toEqual(true);
    });
  });
+
+  describe('setIsResizing', () => {
+    test('setIsResizing have been call when it renders actions', () => {
+      const mockSetIsResizing = jest.fn();
+      mount(
+        <TestProviders>
+          <Header
+            header={columnHeader}
+            isLoading={false}
+            onColumnRemoved={jest.fn()}
+            onColumnResized={jest.fn()}
+            onColumnSorted={jest.fn()}
+            setIsResizing={mockSetIsResizing}
+            sort={sort}
+            timelineId={timelineId}
+          />
+        </TestProviders>
+      );
+
+      expect(mockSetIsResizing).toHaveBeenCalled();
+    });
+  });
 });
--- a/x-pack/plugins/siem/public/components/timeline/body/column_headers/header/index.tsx
+++ b/x-pack/plugins/siem/public/components/timeline/body/column_headers/header/index.tsx
@ -61,6 +61,7 @@ interface Props {
  onColumnResized: OnColumnResized;
  onColumnSorted: OnColumnSorted;
  onFilterChange?: OnFilterChange;
+  setIsResizing: (isResizing: boolean) => void;
  sort: Sort;
  timelineId: string;
 }
@ -93,7 +94,16 @@ export class Header extends React.PureComponent<Props> {
  }

  private renderActions = (isResizing: boolean) => {
-    const { header, isLoading, sort, onColumnRemoved, onFilterChange = noop } = this.props;
+    const {
+      header,
+      isLoading,
+      onColumnRemoved,
+      onFilterChange = noop,
+      setIsResizing,
+      sort,
+    } = this.props;
+
+    setIsResizing(isResizing);

    return (
      <HeaderFlexItem grow={false} width={`${header.width - CELL_RESIZE_HANDLE_WIDTH}px`}>
--- a/x-pack/plugins/siem/public/components/timeline/body/column_headers/index.test.tsx
+++ b/x-pack/plugins/siem/public/components/timeline/body/column_headers/index.test.tsx
@ -17,6 +17,14 @@ import { TestProviders } from '../../../../mock/test_providers';

 import { ColumnHeaders } from '.';

+jest.mock('../../../resize_handle/is_resizing', () => ({
+  ...jest.requireActual('../../../resize_handle/is_resizing'),
+  isContainerResizing: () => ({
+    isResizing: true,
+    setIsResizing: jest.fn(),
+  }),
+}));
+
 describe('ColumnHeaders', () => {
  describe('rendering', () => {
    const sort: Sort = {
@ -101,5 +109,35 @@ describe('ColumnHeaders', () => {
        ).toContain(h.id);
      });
    });
+
+    test('it disables dragging during a column resize', () => {
+      const wrapper = mount(
+        <TestProviders>
+          <ColumnHeaders
+            actionsColumnWidth={ACTIONS_COLUMN_WIDTH}
+            browserFields={mockBrowserFields}
+            columnHeaders={defaultHeaders}
+            isLoading={false}
+            minWidth={1000}
+            onColumnSorted={jest.fn()}
+            onColumnRemoved={jest.fn()}
+            onColumnResized={jest.fn()}
+            onUpdateColumns={jest.fn()}
+            showEventsSelect={false}
+            sort={sort}
+            timelineId={'test'}
+          />
+        </TestProviders>
+      );
+
+      defaultHeaders.forEach(h => {
+        expect(
+          wrapper
+            .find('[data-test-subj="draggable"]')
+            .first()
+            .prop('isDragDisabled')
+        ).toBe(true);
+      });
+    });
  });
 });
--- a/x-pack/plugins/siem/public/components/timeline/body/column_headers/index.tsx
+++ b/x-pack/plugins/siem/public/components/timeline/body/column_headers/index.tsx
@ -34,6 +34,7 @@ import { ColumnHeader } from './column_header';
 import { EventsSelect } from './events_select';
 import { Header } from './header';
 import { FIELD_BROWSER_HEIGHT, FIELD_BROWSER_WIDTH } from '../../../fields_browser/helpers';
+import { isContainerResizing } from '../../../resize_handle/is_resizing';

 const ActionsContainer = styled.div<{ actionsColumnWidth: number }>`
  overflow: hidden;
@ -93,90 +94,97 @@ export const ColumnHeaders = pure<Props>(
    sort,
    timelineId,
    minWidth,
-  }) => (
-    <ColumnHeadersContainer data-test-subj="column-headers" minWidth={minWidth}>
-      <ColumnHeadersFlexGroup
-        alignItems="center"
-        data-test-subj="column-headers-group"
-        gutterSize="none"
-      >
-        <EuiFlexItem data-test-subj="actions-item" grow={false}>
-          <ActionsContainer
-            actionsColumnWidth={actionsColumnWidth}
-            data-test-subj="actions-container"
-          >
-            <EuiFlexGroup gutterSize="none">
-              {showEventsSelect && (
-                <EventsSelectContainer grow={false}>
-                  <EventsSelect checkState="unchecked" timelineId={timelineId} />
-                </EventsSelectContainer>
-              )}
-              <EuiFlexItem grow={true}>
-                <StatefulFieldsBrowser
-                  browserFields={browserFields}
-                  columnHeaders={columnHeaders}
-                  data-test-subj="field-browser"
-                  height={FIELD_BROWSER_HEIGHT}
-                  isLoading={isLoading}
-                  onUpdateColumns={onUpdateColumns}
-                  timelineId={timelineId}
-                  width={FIELD_BROWSER_WIDTH}
-                />
-              </EuiFlexItem>
-            </EuiFlexGroup>
-          </ActionsContainer>
-        </EuiFlexItem>
+  }) => {
+    const { isResizing, setIsResizing } = isContainerResizing();

-        <EuiFlexItem data-test-subj="headers-item" grow={false}>
-          <DroppableWrapper
-            droppableId={`${droppableTimelineColumnsPrefix}${timelineId}`}
-            height={COLUMN_HEADERS_HEIGHT}
-            isDropDisabled={false}
-            type={DRAG_TYPE_FIELD}
-          >
-            <EuiFlexGroup data-test-subj="headers-group" gutterSize="none">
-              {columnHeaders.map((header, i) => (
-                <EuiFlexItem grow={false} key={header.id}>
-                  <Draggable
-                    draggableId={getDraggableFieldId({
-                      contextId: `timeline-column-headers-${timelineId}`,
-                      fieldId: header.id,
-                    })}
-                    index={i}
-                    type={DRAG_TYPE_FIELD}
-                  >
-                    {(provided, snapshot) => (
-                      <div
-                        {...provided.draggableProps}
-                        {...provided.dragHandleProps}
-                        ref={provided.innerRef}
-                        data-test-subj="draggable-header"
-                      >
-                        {!snapshot.isDragging ? (
-                          <Header
-                            timelineId={timelineId}
-                            header={header}
-                            isLoading={isLoading}
-                            onColumnRemoved={onColumnRemoved}
-                            onColumnResized={onColumnResized}
-                            onColumnSorted={onColumnSorted}
-                            onFilterChange={onFilterChange}
-                            sort={sort}
-                          />
-                        ) : (
-                          <DragEffects>
-                            <DraggableFieldBadge fieldId={header.id} />
-                          </DragEffects>
-                        )}
-                      </div>
-                    )}
-                  </Draggable>
+    return (
+      <ColumnHeadersContainer data-test-subj="column-headers" minWidth={minWidth}>
+        <ColumnHeadersFlexGroup
+          alignItems="center"
+          data-test-subj="column-headers-group"
+          gutterSize="none"
+        >
+          <EuiFlexItem data-test-subj="actions-item" grow={false}>
+            <ActionsContainer
+              actionsColumnWidth={actionsColumnWidth}
+              data-test-subj="actions-container"
+            >
+              <EuiFlexGroup gutterSize="none">
+                {showEventsSelect && (
+                  <EventsSelectContainer grow={false}>
+                    <EventsSelect checkState="unchecked" timelineId={timelineId} />
+                  </EventsSelectContainer>
+                )}
+                <EuiFlexItem grow={true}>
+                  <StatefulFieldsBrowser
+                    browserFields={browserFields}
+                    columnHeaders={columnHeaders}
+                    data-test-subj="field-browser"
+                    height={FIELD_BROWSER_HEIGHT}
+                    isLoading={isLoading}
+                    onUpdateColumns={onUpdateColumns}
+                    timelineId={timelineId}
+                    width={FIELD_BROWSER_WIDTH}
+                  />
                </EuiFlexItem>
-              ))}
-            </EuiFlexGroup>
-          </DroppableWrapper>
-        </EuiFlexItem>
-      </ColumnHeadersFlexGroup>
-    </ColumnHeadersContainer>
-  )
+              </EuiFlexGroup>
+            </ActionsContainer>
+          </EuiFlexItem>
+
+          <EuiFlexItem data-test-subj="headers-item" grow={false}>
+            <DroppableWrapper
+              droppableId={`${droppableTimelineColumnsPrefix}${timelineId}`}
+              height={COLUMN_HEADERS_HEIGHT}
+              isDropDisabled={false}
+              type={DRAG_TYPE_FIELD}
+            >
+              <EuiFlexGroup data-test-subj="headers-group" gutterSize="none">
+                {columnHeaders.map((header, i) => (
+                  <EuiFlexItem grow={false} key={header.id}>
+                    <Draggable
+                      data-test-subj="draggable"
+                      draggableId={getDraggableFieldId({
+                        contextId: `timeline-column-headers-${timelineId}`,
+                        fieldId: header.id,
+                      })}
+                      index={i}
+                      type={DRAG_TYPE_FIELD}
+                      isDragDisabled={isResizing}
+                    >
+                      {(provided, snapshot) => (
+                        <div
+                          {...provided.draggableProps}
+                          {...provided.dragHandleProps}
+                          ref={provided.innerRef}
+                          data-test-subj="draggable-header"
+                        >
+                          {!snapshot.isDragging ? (
+                            <Header
+                              timelineId={timelineId}
+                              header={header}
+                              isLoading={isLoading}
+                              onColumnRemoved={onColumnRemoved}
+                              onColumnResized={onColumnResized}
+                              onColumnSorted={onColumnSorted}
+                              onFilterChange={onFilterChange}
+                              setIsResizing={setIsResizing}
+                              sort={sort}
+                            />
+                          ) : (
+                            <DragEffects>
+                              <DraggableFieldBadge fieldId={header.id} />
+                            </DragEffects>
+                          )}
+                        </div>
+                      )}
+                    </Draggable>
+                  </EuiFlexItem>
+                ))}
+              </EuiFlexGroup>
+            </DroppableWrapper>
+          </EuiFlexItem>
+        </ColumnHeadersFlexGroup>
+      </ColumnHeadersContainer>
+    );
+  }
 );