From 220ebbc6010d7133868332592762a751a23731de Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 31 Mar 2021 03:42:59 -0700 Subject: [PATCH] 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 --- .../src/sandy-chrome/SandyDesignSystem.tsx | 1 + .../flipper-plugin/src/ui/DataFormatter.tsx | 6 +-- .../src/ui/datatable/ColumnFilter.tsx | 8 ++-- .../src/ui/datatable/DataSourceRenderer.tsx | 33 ++++++--------- .../src/ui/datatable/DataTable.tsx | 41 +++++++++++++++++-- .../src/ui/datatable/DataTableManager.tsx | 17 +++++++- .../src/ui/datatable/TableHead.tsx | 2 +- desktop/flipper-plugin/src/ui/theme.tsx | 1 + desktop/static/icons.json | 3 +- 9 files changed, 79 insertions(+), 33 deletions(-) diff --git a/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx b/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx index 83f2ceb9c..3f06c9d90 100644 --- a/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx +++ b/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx @@ -66,6 +66,7 @@ export default function SandyDesignSystem() { + diff --git a/desktop/flipper-plugin/src/ui/DataFormatter.tsx b/desktop/flipper-plugin/src/ui/DataFormatter.tsx index 62da1686e..cdddbfa6d 100644 --- 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 ? : }> - {`(and ${value.length - maxLength} more...)`} + {collapsed ? `and ${value.length - maxLength} more` : 'collapse'} ); } const truncateButtonStyle = { - color: theme.textColorPrimary, + color: theme.primaryColor, marginLeft: 4, }; diff --git a/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx b/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx index 3236c02fe..73a2f2adf 100644 --- 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 ( - + ); @@ -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, }, })); diff --git a/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx b/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx index 93e3c4b42..e7535b14a 100644 --- a/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx @@ -66,6 +66,7 @@ type DataSourceProps = { total: number, offset: number, ): void; + onUpdateAutoScroll?(autoScroll: boolean): void; emptyRenderer?(dataSource: DataSource): React.ReactElement; _testHeight?: number; // exposed for unit testing only }; @@ -86,6 +87,7 @@ export const DataSourceRenderer: ( onKeyDown, virtualizerRef, onRangeChange, + onUpdateAutoScroll, emptyRenderer, _testHeight, }: DataSourceProps) { @@ -211,7 +213,7 @@ export const DataSourceRenderer: ( 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: ( /** * 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: ( * Render finalization */ useEffect(function renderCompleted() { - suppressScroll.current = false; renderPending.current = UpdatePrio.NONE; lastRender.current = Date.now(); }); @@ -295,7 +288,7 @@ export const DataSourceRenderer: ( */ return ( - + {virtualizer.virtualItems.length === 0 ? emptyRenderer?.(dataSource) : null} diff --git a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx index 66611dd62..eee940748 100644 --- 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( onSelect, scope, virtualizerRef, + autoScroll: props.autoScroll, }), ); @@ -307,6 +308,7 @@ export function DataTable( 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( [], ); + 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( > 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( onKeyDown={onKeyDown} virtualizerRef={virtualizerRef} onRangeChange={onRangeChange} + onUpdateAutoScroll={onUpdateAutoScroll} emptyRenderer={emptyRenderer} _testHeight={props._testHeight} /> + {props.autoScroll && ( + + { + dispatch({type: 'toggleAutoScroll'}); + }} + /> + + )} {range && {range}} ); @@ -436,9 +460,20 @@ function EmptyTable({dataSource}: {dataSource: DataSource}) { 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', +}); diff --git a/desktop/flipper-plugin/src/ui/datatable/DataTableManager.tsx b/desktop/flipper-plugin/src/ui/datatable/DataTableManager.tsx index dc06622d6..657b01f0c 100644 --- 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[]; scrollOffset: number; + autoScroll: boolean; }; type Action = {type: Name} & Args; @@ -79,7 +80,9 @@ type DataManagerActions = | 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 = { dataSource: DataSource; @@ -87,6 +90,7 @@ type DataManagerConfig = { scope: string; onSelect: undefined | ((item: T | undefined, items: T[]) => void); virtualizerRef: MutableRefObject; + autoScroll?: boolean; }; type DataManagerState = { @@ -99,6 +103,7 @@ type DataManagerState = { selection: Selection; searchValue: string; useRegex: boolean; + autoScroll: boolean; }; export type DataTableReducer = 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( : 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)); } diff --git a/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx b/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx index dba43d18d..21543fdf7 100644 --- 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, }, }), ); diff --git a/desktop/flipper-plugin/src/ui/theme.tsx b/desktop/flipper-plugin/src/ui/theme.tsx index c427e42c3..96d6b0dcd 100644 --- 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)', diff --git a/desktop/static/icons.json b/desktop/static/icons.json index b3b8d1552..ea1ae1d42 100644 --- a/desktop/static/icons.json +++ b/desktop/static/icons.json @@ -369,7 +369,8 @@ ], "code": [ 12, - 16 + 16, + 20 ], "undo-outline": [ 16