Make autoScrolling explicit

Summary: Changelog: Added an explicit autoscroll indicator in logs and fixed snapping We got several reports that auto scrolling was to aggressive, so revisited the implementation and the new one is a lot more reliable. Also added an explicit indicator / button to toggle tailing. Exposed ant's active color as well in our theme, as it gives better contrast on the buttons than Flipper purple. Reviewed By: passy Differential Revision: D27397506 fbshipit-source-id: 5e82939de4b2f8b89380bd55009e3fa2a7c10ec9
2021-03-31 03:42:59 -07:00
parent 8cd38a6b49
commit 220ebbc601
9 changed files with 79 additions and 33 deletions
--- a/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx
+++ b/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx
@@ -66,6 +66,7 @@ export default function SandyDesignSystem() {
          <ColorPreview name="textColorPrimary" />
          <ColorPreview name="textColorSecondary" />
          <ColorPreview name="textColorPlaceholder" />
+          <ColorPreview name="textColorActive" />
          <ColorPreview name="disabledColor" />
          <ColorPreview name="backgroundDefault" />
          <ColorPreview name="backgroundWash" />
--- a/desktop/flipper-plugin/src/ui/DataFormatter.tsx
+++ b/desktop/flipper-plugin/src/ui/DataFormatter.tsx
@@ -159,7 +159,7 @@ function TruncateHelper({
        type="text"
        style={truncateButtonStyle}
        icon={collapsed ? <CaretRightOutlined /> : <CaretUpOutlined />}>
-        {`(and ${value.length - maxLength} more...)`}
+        {collapsed ? `and ${value.length - maxLength} more` : 'collapse'}
      </Button>
      <Button
        icon={<CopyOutlined />}
@@ -169,13 +169,13 @@ function TruncateHelper({
        size="small"
        type="text"
        style={truncateButtonStyle}>
-        Copy
+        copy
      </Button>
    </>
  );
 }

 const truncateButtonStyle = {
-  color: theme.textColorPrimary,
+  color: theme.primaryColor,
  marginLeft: 4,
 };
--- a/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx
@@ -12,7 +12,7 @@ import styled from '@emotion/styled';
 import React from 'react';
 import {Button, Checkbox, Dropdown, Menu, Typography, Input} from 'antd';
 import {
-  FilterFilled,
+  FilterOutlined,
  MinusCircleOutlined,
  PlusCircleOutlined,
 } from '@ant-design/icons';
@@ -167,7 +167,7 @@ export function FilterIcon({
  return (
    <Dropdown overlay={menu} trigger={['click']}>
      <FilterButton isActive={isActive}>
-        <FilterFilled />
+        <FilterOutlined />
      </FilterButton>
    </Dropdown>
  );
@@ -176,12 +176,12 @@ export function FilterIcon({
 export const FilterButton = styled.div<{isActive?: boolean}>(({isActive}) => ({
  backgroundColor: theme.backgroundWash,
  visibility: isActive ? 'visible' : 'hidden',
-  color: isActive ? theme.primaryColor : theme.disabledColor,
+  color: isActive ? theme.textColorActive : theme.disabledColor,
  cursor: 'pointer',
  marginRight: 4,
  zIndex: 1,
  '&:hover': {
-    color: theme.primaryColor,
+    color: theme.textColorActive,
    backgroundColor: theme.backgroundWash,
  },
 }));
--- a/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx
@@ -66,6 +66,7 @@ type DataSourceProps<T extends object, C> = {
    total: number,
    offset: number,
  ): void;
+  onUpdateAutoScroll?(autoScroll: boolean): void;
  emptyRenderer?(dataSource: DataSource<T>): React.ReactElement;
  _testHeight?: number; // exposed for unit testing only
 };
@@ -86,6 +87,7 @@ export const DataSourceRenderer: <T extends object, C>(
  onKeyDown,
  virtualizerRef,
  onRangeChange,
+  onUpdateAutoScroll,
  emptyRenderer,
  _testHeight,
 }: DataSourceProps<any, any>) {
@@ -211,7 +213,7 @@ export const DataSourceRenderer: <T extends object, C>(
  useLayoutEffect(function updateWindow() {
    const start = virtualizer.virtualItems[0]?.index ?? 0;
    const end = start + virtualizer.virtualItems.length;
-    if (start !== dataSource.view.windowStart && !followOutput.current) {
+    if (start !== dataSource.view.windowStart && !autoScroll) {
      onRangeChange?.(
        start,
        end,
@@ -225,29 +227,21 @@ export const DataSourceRenderer: <T extends object, C>(
  /**
   * Scrolling
   */
-  // if true, scroll if new items are appended
-  const followOutput = useRef(false);
-  // if true, the next scroll event will be fired as result of a size change,
-  // ignore it
-  const suppressScroll = useRef(false);
-  suppressScroll.current = true;
-
  const onScroll = useCallback(() => {
-    // scroll event is firing as a result of painting new items?
-    if (suppressScroll.current || !autoScroll) {
+    const elem = parentRef.current;
+    if (!elem) {
      return;
    }
-    const elem = parentRef.current!;
-    // make bottom 1/3 of screen sticky
-    if (elem.scrollTop < elem.scrollHeight - elem.clientHeight * 1.3) {
-      followOutput.current = false;
-    } else {
-      followOutput.current = true;
+    const fromEnd = elem.scrollHeight - elem.scrollTop - elem.clientHeight;
+    if (autoScroll && fromEnd >= 1) {
+      onUpdateAutoScroll?.(false);
+    } else if (!autoScroll && fromEnd < 1) {
+      onUpdateAutoScroll?.(true);
    }
-  }, [autoScroll, parentRef]);
+  }, [onUpdateAutoScroll, autoScroll]);

  useLayoutEffect(function scrollToEnd() {
-    if (followOutput.current && autoScroll) {
+    if (autoScroll) {
      virtualizer.scrollToIndex(
        dataSource.view.size - 1,
        /* smooth is not typed by react-virtual, but passed on to the DOM as it should*/
@@ -263,7 +257,6 @@ export const DataSourceRenderer: <T extends object, C>(
   * Render finalization
   */
  useEffect(function renderCompleted() {
-    suppressScroll.current = false;
    renderPending.current = UpdatePrio.NONE;
    lastRender.current = Date.now();
  });
@@ -295,7 +288,7 @@ export const DataSourceRenderer: <T extends object, C>(
   */
  return (
    <RedrawContext.Provider value={redraw}>
-      <TableContainer onScroll={onScroll} ref={parentRef}>
+      <TableContainer ref={parentRef} onScroll={onScroll}>
        {virtualizer.virtualItems.length === 0
          ? emptyRenderer?.(dataSource)
          : null}
--- a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx
@@ -41,7 +41,7 @@ import styled from '@emotion/styled';
 import {theme} from '../theme';
 import {tableContextMenuFactory} from './TableContextMenu';
 import {Typography} from 'antd';
-import {CoffeeOutlined, SearchOutlined} from '@ant-design/icons';
+import {CoffeeOutlined, SearchOutlined, PushpinFilled} from '@ant-design/icons';
 import {useAssertStableRef} from '../../utils/useAssertStableRef';
 import {Formatter} from '../DataFormatter';
 import {usePluginInstance} from '../../plugin/PluginContext';
@@ -115,6 +115,7 @@ export function DataTable<T extends object>(
        onSelect,
        scope,
        virtualizerRef,
+        autoScroll: props.autoScroll,
      }),
  );

@@ -307,6 +308,7 @@ export function DataTable<T extends object>(
          type: 'appliedInitialScroll',
        });
      } else if (selection && selection.current >= 0) {
+        dispatch({type: 'setAutoScroll', autoScroll: false});
        virtualizerRef.current?.scrollToIndex(selection!.current, {
          align: 'auto',
        });
@@ -334,6 +336,15 @@ export function DataTable<T extends object>(
    [],
  );

+  const onUpdateAutoScroll = useCallback(
+    (autoScroll: boolean) => {
+      if (props.autoScroll) {
+        dispatch({type: 'setAutoScroll', autoScroll});
+      }
+    },
+    [props.autoScroll],
+  );
+
  /** Context menu */
  const contexMenu = props._testHeight
    ? undefined
@@ -390,7 +401,7 @@ export function DataTable<T extends object>(
        </Layout.Container>
        <DataSourceRenderer<T, RenderContext<T>>
          dataSource={dataSource}
-          autoScroll={props.autoScroll && !dragging.current}
+          autoScroll={tableState.autoScroll && !dragging.current}
          useFixedRowHeight={!tableState.usesWrapping}
          defaultRowHeight={DEFAULT_ROW_HEIGHT}
          context={renderingConfig}
@@ -398,10 +409,23 @@ export function DataTable<T extends object>(
          onKeyDown={onKeyDown}
          virtualizerRef={virtualizerRef}
          onRangeChange={onRangeChange}
+          onUpdateAutoScroll={onUpdateAutoScroll}
          emptyRenderer={emptyRenderer}
          _testHeight={props._testHeight}
        />
      </Layout.Top>
+      {props.autoScroll && (
+        <AutoScroller>
+          <PushpinFilled
+            style={{
+              color: tableState.autoScroll ? theme.textColorActive : undefined,
+            }}
+            onClick={() => {
+              dispatch({type: 'toggleAutoScroll'});
+            }}
+          />
+        </AutoScroller>
+      )}
      {range && <RangeFinder>{range}</RangeFinder>}
    </Layout.Container>
  );
@@ -436,9 +460,20 @@ function EmptyTable({dataSource}: {dataSource: DataSource<any>}) {
 const RangeFinder = styled.div({
  backgroundColor: theme.backgroundWash,
  position: 'absolute',
-  right: 40,
+  right: 64,
  bottom: 20,
  padding: '4px 8px',
  color: theme.textColorSecondary,
  fontSize: '0.8em',
 });
+
+const AutoScroller = styled.div({
+  backgroundColor: theme.backgroundWash,
+  position: 'absolute',
+  right: 40,
+  bottom: 20,
+  width: 24,
+  padding: '4px 8px',
+  color: theme.textColorSecondary,
+  fontSize: '0.8em',
+});
--- a/desktop/flipper-plugin/src/ui/datatable/DataTableManager.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/DataTableManager.tsx
@@ -40,6 +40,7 @@ type PersistedState = {
  /** The default columns, but normalized */
  columns: Pick<DataTableColumn, 'key' | 'width' | 'filters' | 'visible'>[];
  scrollOffset: number;
+  autoScroll: boolean;
 };

 type Action<Name extends string, Args = {}> = {type: Name} & Args;
@@ -79,7 +80,9 @@ type DataManagerActions<T> =
  | Action<'toggleColumnFilter', {column: keyof T; index: number}>
  | Action<'setColumnFilterFromSelection', {column: keyof T}>
  | Action<'appliedInitialScroll'>
-  | Action<'toggleUseRegex'>;
+  | Action<'toggleUseRegex'>
+  | Action<'toggleAutoScroll'>
+  | Action<'setAutoScroll', {autoScroll: boolean}>;

 type DataManagerConfig<T> = {
  dataSource: DataSource<T>;
@@ -87,6 +90,7 @@ type DataManagerConfig<T> = {
  scope: string;
  onSelect: undefined | ((item: T | undefined, items: T[]) => void);
  virtualizerRef: MutableRefObject<DataSourceVirtualizer | undefined>;
+  autoScroll?: boolean;
 };

 type DataManagerState<T> = {
@@ -99,6 +103,7 @@ type DataManagerState<T> = {
  selection: Selection;
  searchValue: string;
  useRegex: boolean;
+  autoScroll: boolean;
 };

 export type DataTableReducer<T> = Reducer<
@@ -208,6 +213,14 @@ export const dataTableManagerReducer = produce<
      draft.initialOffset = 0;
      break;
    }
+    case 'toggleAutoScroll': {
+      draft.autoScroll = !draft.autoScroll;
+      break;
+    }
+    case 'setAutoScroll': {
+      draft.autoScroll = action.autoScroll;
+      break;
+    }
    default: {
      throw new Error('Unknown action ' + (action as any).type);
    }
@@ -307,6 +320,7 @@ export function createInitialState<T>(
      : emptySelection,
    searchValue: prefs?.search ?? '',
    useRegex: prefs?.useRegex ?? false,
+    autoScroll: prefs?.autoScroll ?? config.autoScroll ?? false,
  };
  // @ts-ignore
  res.config[immerable] = false; // optimization: never proxy anything in config
@@ -382,6 +396,7 @@ export function savePreferences(
      visible: c.visible,
    })),
    scrollOffset,
+    autoScroll: state.autoScroll,
  };
  localStorage.setItem(state.storageKey, JSON.stringify(prefs));
 }
--- a/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx
@@ -72,7 +72,7 @@ const SortIconsContainer = styled.span<{direction?: 'asc' | 'desc'}>(
    cursor: 'pointer',
    color: theme.disabledColor,
    '.ant-table-column-sorter-up:hover, .ant-table-column-sorter-down:hover': {
-      color: theme.primaryColor,
+      color: theme.textColorActive,
    },
  }),
 );
--- a/desktop/flipper-plugin/src/ui/theme.tsx
+++ b/desktop/flipper-plugin/src/ui/theme.tsx
@@ -18,6 +18,7 @@ export const theme = {
  textColorPrimary: 'var(--flipper-text-color-primary)',
  textColorSecondary: 'var(--flipper-text-color-secondary)',
  textColorPlaceholder: 'var(--flipper-text-color-placeholder)',
+  textColorActive: 'var(--light-color-button-active)',
  disabledColor: 'var(--flipper-disabled-color)',
  backgroundDefault: 'var(--flipper-background-default)',
  backgroundWash: 'var(--flipper-background-wash)',
--- a/desktop/static/icons.json
+++ b/desktop/static/icons.json
@@ -369,7 +369,8 @@
  ],
  "code": [
    12,
-    16
+    16,
+    20
  ],
  "undo-outline": [
    16