From d903a862d2d08385f551ea792cdb6e710fe85c06 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 4 May 2021 13:49:11 -0700 Subject: [PATCH] Use DataTable as list base Summary: Changelog: Standardized DataList component This diff standardizes the DataList component, by reusing the DataList. This is done to be able to take full advantage of all its features like virtualisation, keyboard support, datasource support, etc. Also cleaned up DataTable properties a bit, by prefixing all flags with `enableXXX` and setting clear defaults Reviewed By: passy Differential Revision: D28119721 fbshipit-source-id: b7b241ea18d788bfa035389cc8c6ae7ea95ecadb --- desktop/flipper-plugin/src/ui/DataList.tsx | 204 +++++++++++------- desktop/flipper-plugin/src/ui/Layout.tsx | 1 + .../flipper-plugin/src/ui/MasterDetail.tsx | 2 +- .../src/ui/data-table/DataSourceRenderer.tsx | 3 +- .../src/ui/data-table/DataTable.tsx | 156 ++++++++------ .../data-table/StaticDataSourceRenderer.tsx | 2 +- .../src/ui/data-table/TableRow.tsx | 20 +- desktop/plugins/public/cpu/index.tsx | 4 +- .../public/databases/DatabaseStructure.tsx | 4 +- desktop/plugins/public/logs/index.tsx | 2 +- docs/tutorial/js-custom.mdx | 3 +- 11 files changed, 244 insertions(+), 157 deletions(-) diff --git a/desktop/flipper-plugin/src/ui/DataList.tsx b/desktop/flipper-plugin/src/ui/DataList.tsx index f6fc06764..4b8690df7 100644 --- a/desktop/flipper-plugin/src/ui/DataList.tsx +++ b/desktop/flipper-plugin/src/ui/DataList.tsx @@ -7,11 +7,29 @@ * @format */ -import React, {useCallback, memo} from 'react'; +import React, { + useCallback, + memo, + createRef, + useEffect, + useMemo, + useState, +} from 'react'; import {DataFormatter} from './DataFormatter'; import {Layout} from './Layout'; -import {theme} from './theme'; import {Typography} from 'antd'; +import { + DataTable, + DataTableColumn, + DataTableProps, + ItemRenderer, +} from './data-table/DataTable'; +import {RightOutlined} from '@ant-design/icons'; +import {theme} from './theme'; +import styled from '@emotion/styled'; +import {DataTableManager} from './data-table/DataTableManager'; +import {Atom, createState} from '../state/atom'; +import {useAssertStableRef} from '../utils/useAssertStableRef'; const {Text} = Typography; @@ -21,7 +39,7 @@ interface Item { description?: string; } -interface DataListProps { +interface DataListBaseProps { /** * Defines the styling of the component. By default shows a list, but alternatively the items can be displayed in a drop down */ @@ -34,11 +52,11 @@ interface DataListProps { /** * The current selection */ - value?: string /* | Atom*/; + selection: Atom; /** * Handler that is fired if selection is changed */ - onSelect?(id: string, value: T): void; + onSelect?(id: string | undefined, value: T | undefined): void; className?: string; style?: React.CSSProperties; /** @@ -48,105 +66,143 @@ interface DataListProps { /** * Custom render function. By default the component will render the `title` in bold and description (if any) below it */ - onRenderItem?: (item: T, selected: boolean) => React.ReactElement; + onRenderItem?: ItemRenderer; + /** + * Show a right arrow by default + */ + enableArrow?: boolean; } +export type DataListProps = DataListBaseProps & + // Some table props are set by DataList instead, so override them + Omit, 'records' | 'dataSource' | 'columns' | 'onSelect'>; + export const DataList: React.FC> = function DataList< T extends Item >({ - // type, - scrollable, - value, + selection: baseSelection, onSelect, className, style, items, onRenderItem, + enableArrow, + ...tableProps }: DataListProps) { + // if a tableManagerRef is provided, we piggy back on that same ref + // eslint-disable-next-line + const tableManagerRef = tableProps.tableManagerRef ?? createRef>(); + + useAssertStableRef(baseSelection, 'selection'); + // create local selection atom if none provided + // eslint-disable-next-line + const selection = baseSelection ?? useState(() => createState())[0]; + const handleSelect = useCallback( - (key: string, item: T) => { - onSelect?.(key, item); + (item: T | undefined) => { + selection.set(item?.id); }, - [onSelect], + [selection], ); - const renderedItems = items.map((item) => ( - - )); + const dataListColumns: DataTableColumn[] = useMemo( + () => [ + { + key: 'id' as const, + wrap: true, + onRender(item: T, selected: boolean, index: number) { + return onRenderItem ? ( + onRenderItem(item, selected, index) + ) : ( + + ); + }, + }, + ], + [onRenderItem, enableArrow], + ); - return scrollable ? ( - - {renderedItems} - - ) : ( - - {renderedItems} + useEffect( + function updateSelection() { + return selection.subscribe((valueFromAtom) => { + const m = tableManagerRef.current; + if (!m) { + return; + } + if (!valueFromAtom && m.getSelectedItem()) { + m.clearSelection(); + } else if (valueFromAtom && m.getSelectedItem()?.id !== valueFromAtom) { + // find valueFromAtom in the selection + m.selectItemById(valueFromAtom); + } + }); + }, + [selection, tableManagerRef], + ); + + return ( + + + {...tableProps} + tableManagerRef={tableManagerRef} + records={items} + recordsKey="id" + columns={dataListColumns} + onSelect={handleSelect} + /> ); }; DataList.defaultProps = { type: 'default', - scrollable: false, - onRenderItem: defaultItemRenderer, + scrollable: true, + enableSearchbar: false, + enableColumnHeaders: false, + enableArrow: true, }; -function defaultItemRenderer(item: Item, _selected: boolean) { - return ; -} - -const DataListItemWrapper = memo( +const DataListItem = memo( ({ - item, - onRenderItem, - onSelect, - selected, + title, + description, + enableArrow, }: { - item: Item; - onRenderItem: typeof defaultItemRenderer; - onSelect: (id: string, item: Item) => void; - selected: boolean; + // TODO: add icon support + title: string; + description?: string; + enableArrow?: boolean; }) => { return ( - { - onSelect(item.id, item); - }}> - {onRenderItem(item, selected)} - + + + + {DataFormatter.format(title)} + + {description != null && ( + + {DataFormatter.format(description)} + + )} + + {enableArrow && ( + + + + )} + ); }, ); -const DataListItem = memo( - ({title, description}: {title: string; description?: string}) => { - return ( - <> - {DataFormatter.format(title)} - {description != null && ( - {DataFormatter.format(description)} - )} - - ); +const ArrowWrapper = styled.div({ + flex: 0, + paddingLeft: theme.space.small, + '.anticon': { + lineHeight: '14px', }, -); +}); diff --git a/desktop/flipper-plugin/src/ui/Layout.tsx b/desktop/flipper-plugin/src/ui/Layout.tsx index 0659d3e2f..a49887b50 100644 --- a/desktop/flipper-plugin/src/ui/Layout.tsx +++ b/desktop/flipper-plugin/src/ui/Layout.tsx @@ -73,6 +73,7 @@ const Container = styled.div( gap: normalizeSpace(gap, theme.space.small), minWidth: shrink ? 0 : undefined, + maxWidth: shrink ? '100%' : undefined, boxSizing: 'border-box', width, height, diff --git a/desktop/flipper-plugin/src/ui/MasterDetail.tsx b/desktop/flipper-plugin/src/ui/MasterDetail.tsx index b8e91ab87..d0cff7076 100644 --- a/desktop/flipper-plugin/src/ui/MasterDetail.tsx +++ b/desktop/flipper-plugin/src/ui/MasterDetail.tsx @@ -194,7 +194,7 @@ export function MasterDetail({ const table = ( - autoScroll + enableAutoScroll {...tableProps} dataSource={dataSource as any} records={records!} diff --git a/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx b/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx index 23ed9b539..e28138504 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx @@ -68,8 +68,7 @@ type DataSourceProps = { offset: number, ): void; onUpdateAutoScroll?(autoScroll: boolean): void; - emptyRenderer?(dataSource: DataSource): React.ReactElement; - _testHeight?: number; // exposed for unit testing only + emptyRenderer?: null | ((dataSource: DataSource) => React.ReactElement); }; /** diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx index e24ec1e7a..65a5a2742 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx @@ -51,30 +51,45 @@ import {useInUnitTest} from '../../utils/useInUnitTest()'; interface DataTableBaseProps { columns: DataTableColumn[]; - - autoScroll?: boolean; + enableSearchbar?: boolean; + enableAutoScroll?: boolean; + enableColumnHeaders?: boolean; + enableMultiSelect?: boolean; + // if set (the default) will grow and become scrollable. Otherwise will use natural size + scrollable?: boolean; extraActions?: React.ReactElement; onSelect?(record: T | undefined, records: T[]): void; onRowStyle?(record: T): CSSProperties | undefined; - // multiselect?: true tableManagerRef?: RefObject | undefined>; // Actually we want a MutableRefObject, but that is not what React.createRef() returns, and we don't want to put the burden on the plugin dev to cast it... onCopyRows?(records: T[]): string; onContextMenu?: (selection: undefined | T) => React.ReactElement; - searchbar?: boolean; - scrollable?: boolean; + onRenderEmpty?: + | null + | ((dataSource?: DataSource) => React.ReactElement); } +export type ItemRenderer = ( + item: T, + selected: boolean, + index: number, +) => React.ReactNode; + type DataTableInput = - | {dataSource: DataSource; records?: undefined} | { - records: T[]; + dataSource: DataSource; + records?: undefined; + recordsKey?: undefined; + } + | { + records: readonly T[]; + recordsKey?: keyof T; dataSource?: undefined; }; export type DataTableColumn = { key: keyof T & string; // possible future extension: getValue(row) (and free-form key) to support computed columns - onRender?: (row: T) => React.ReactNode; + onRender?: (row: T, selected: boolean, index: number) => React.ReactNode; formatters?: Formatter[] | Formatter; title?: string; width?: number | Percentage | undefined; // undefined: use all remaining width @@ -89,7 +104,7 @@ export type DataTableColumn = { }[]; }; -export interface RenderContext { +export interface TableRowRenderContext { columns: DataTableColumn[]; onMouseEnter( e: React.MouseEvent, @@ -101,6 +116,7 @@ export interface RenderContext { item: T, itemId: number, ): void; + onRowStyle?(item: T): React.CSSProperties | undefined; } export type DataTableProps = DataTableInput & DataTableBaseProps; @@ -116,6 +132,7 @@ export function DataTable( useAssertStableRef(props.columns, 'columns'); useAssertStableRef(onCopyRows, 'onCopyRows'); useAssertStableRef(onContextMenu, 'onContextMenu'); + const isUnitTest = useInUnitTest(); // eslint-disable-next-line @@ -131,7 +148,7 @@ export function DataTable( onSelect, scope, virtualizerRef, - autoScroll: props.autoScroll, + autoScroll: props.enableAutoScroll, }), ); @@ -154,7 +171,7 @@ export function DataTable( [columns], ); - const renderingConfig = useMemo>(() => { + const renderingConfig = useMemo>(() => { let startIndex = 0; return { columns: visibleColumns, @@ -185,14 +202,15 @@ export function DataTable( document.addEventListener('mouseup', onStopDragSelecting); } }, + onRowStyle, }; - }, [visibleColumns, tableManager]); + }, [visibleColumns, tableManager, onRowStyle]); const itemRenderer = useCallback( function itemRenderer( record: T, index: number, - renderContext: RenderContext, + renderContext: TableRowRenderContext, ) { return ( ( const onUpdateAutoScroll = useCallback( (autoScroll: boolean) => { - if (props.autoScroll) { + if (props.enableAutoScroll) { dispatch({type: 'setAutoScroll', autoScroll}); } }, - [props.autoScroll], + [props.enableAutoScroll], ); /** Context menu */ @@ -409,7 +427,7 @@ export function DataTable( const header = ( - {props.searchbar !== false && ( + {props.enableSearchbar && ( ( extraActions={props.extraActions} /> )} - + )} + + ); + + const emptyRenderer = + props.onRenderEmpty === undefined + ? props.onRenderEmpty + : props.onRenderEmpty; + const mainSection = props.scrollable ? ( + + {header} + > + dataSource={dataSource} + autoScroll={tableState.autoScroll && !dragging.current} + useFixedRowHeight={!tableState.usesWrapping} + defaultRowHeight={DEFAULT_ROW_HEIGHT} + context={renderingConfig} + itemRenderer={itemRenderer} + onKeyDown={onKeyDown} + virtualizerRef={virtualizerRef} + onRangeChange={onRangeChange} + onUpdateAutoScroll={onUpdateAutoScroll} + emptyRenderer={emptyRenderer} + /> + + ) : ( + + {header} + > + dataSource={dataSource} + useFixedRowHeight={!tableState.usesWrapping} + defaultRowHeight={DEFAULT_ROW_HEIGHT} + context={renderingConfig} + itemRenderer={itemRenderer} + onKeyDown={onKeyDown} + emptyRenderer={emptyRenderer} /> ); - const mainSection = - props.scrollable !== false ? ( - - {header} - > - dataSource={dataSource} - autoScroll={tableState.autoScroll && !dragging.current} - useFixedRowHeight={!tableState.usesWrapping} - defaultRowHeight={DEFAULT_ROW_HEIGHT} - context={renderingConfig} - itemRenderer={itemRenderer} - onKeyDown={onKeyDown} - virtualizerRef={virtualizerRef} - onRangeChange={onRangeChange} - onUpdateAutoScroll={onUpdateAutoScroll} - emptyRenderer={emptyRenderer} - /> - - ) : ( - - {header} - > - dataSource={dataSource} - useFixedRowHeight={!tableState.usesWrapping} - defaultRowHeight={DEFAULT_ROW_HEIGHT} - context={renderingConfig} - itemRenderer={itemRenderer} - onKeyDown={onKeyDown} - emptyRenderer={emptyRenderer} - /> - - ); - return ( - + {mainSection} - {props.autoScroll && ( + {props.enableAutoScroll && ( ( ); } +DataTable.defaultProps = { + scrollable: true, + enableSearchbar: true, + enableAutoScroll: false, + enableColumnHeaders: true, + eanbleMultiSelect: true, + onRenderEmpty: emptyRenderer, +} as Partial>; + /* eslint-disable react-hooks/rules-of-hooks */ function normalizeDataSourceInput(props: DataTableInput): DataSource { if (props.dataSource) { @@ -491,7 +523,7 @@ function normalizeDataSourceInput(props: DataTableInput): DataSource { } if (props.records) { const [dataSource] = useState(() => { - const ds = new DataSource(undefined); + const ds = new DataSource(props.recordsKey); syncRecordsToDataSource(ds, props.records); return ds; }); @@ -507,7 +539,7 @@ function normalizeDataSourceInput(props: DataTableInput): DataSource { } /* eslint-enable */ -function syncRecordsToDataSource(ds: DataSource, records: T[]) { +function syncRecordsToDataSource(ds: DataSource, records: readonly T[]) { const startTime = Date.now(); ds.clear(); // TODO: optimize in the case we're only dealing with appends or replacements diff --git a/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx b/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx index 8929fc0f0..f4db20609 100644 --- a/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx @@ -34,7 +34,7 @@ type DataSourceProps = { defaultRowHeight: number; onKeyDown?: React.KeyboardEventHandler; onUpdateAutoScroll?(autoScroll: boolean): void; - emptyRenderer?(dataSource: DataSource): React.ReactElement; + emptyRenderer?: null | ((dataSource: DataSource) => React.ReactElement); }; /** diff --git a/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx b/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx index 092bfcfd0..804d5724d 100644 --- a/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx @@ -10,7 +10,7 @@ import React, {CSSProperties, memo} from 'react'; import styled from '@emotion/styled'; import {theme} from '../theme'; -import type {RenderContext} from './DataTable'; +import type {TableRowRenderContext} from './DataTable'; import {Width} from '../../utils/widthUtils'; import {DataFormatter} from '../DataFormatter'; @@ -92,37 +92,35 @@ const TableBodyColumnContainer = styled.div<{ })); TableBodyColumnContainer.displayName = 'TableRow:TableBodyColumnContainer'; -type Props = { - config: RenderContext; +type TableRowProps = { + config: TableRowRenderContext; highlighted: boolean; - record: any; + record: T; itemIndex: number; style?: CSSProperties; }; -export const TableRow = memo(function TableRow({ +export const TableRow = memo(function TableRow({ record, itemIndex, highlighted, - style, config, -}: Props) { +}: TableRowProps) { return ( { config.onMouseDown(e, record, itemIndex); }} onMouseEnter={(e) => { config.onMouseEnter(e, record, itemIndex); }} - style={style}> + style={config.onRowStyle?.(record)}> {config.columns .filter((col) => col.visible) .map((col) => { - const value = (col as any).onRender - ? (col as any).onRender(record) + const value = col.onRender + ? (col as any).onRender(record, highlighted, itemIndex) // TODO: ever used? : DataFormatter.format((record as any)[col.key], col.formatters); return ( diff --git a/desktop/plugins/public/cpu/index.tsx b/desktop/plugins/public/cpu/index.tsx index 42e6aab56..1cfb25009 100644 --- a/desktop/plugins/public/cpu/index.tsx +++ b/desktop/plugins/public/cpu/index.tsx @@ -445,7 +445,7 @@ export function Component() { records={sidebarRows(id)} columns={cpuSidebarColumns} scrollable={false} - searchbar={false} + enableSearchbar={false} /> @@ -511,7 +511,7 @@ export function Component() { scrollable={false} onSelect={setSelected} onRowStyle={getRowStyle} - searchbar={false} + enableSearchbar={false} /> {renderCPUSidebar()} {renderThermalSidebar()} diff --git a/desktop/plugins/public/databases/DatabaseStructure.tsx b/desktop/plugins/public/databases/DatabaseStructure.tsx index 6399761bd..e759bcb3f 100644 --- a/desktop/plugins/public/databases/DatabaseStructure.tsx +++ b/desktop/plugins/public/databases/DatabaseStructure.tsx @@ -63,12 +63,12 @@ export default React.memo((props: {structure: Structure}) => { records={rowObjs} columns={columnObjs} - searchbar={false} + enableSearchbar={false} /> records={indexRowObjs} columns={indexColumnObjs} - searchbar={false} + enableSearchbar={false} /> ); diff --git a/desktop/plugins/public/logs/index.tsx b/desktop/plugins/public/logs/index.tsx index 178b1f0ce..6c0a21f10 100644 --- a/desktop/plugins/public/logs/index.tsx +++ b/desktop/plugins/public/logs/index.tsx @@ -226,7 +226,7 @@ export function Component() { dataSource={plugin.rows} columns={plugin.columns} - autoScroll + enableAutoScroll onRowStyle={getRowStyle} extraActions={ plugin.isConnected ? ( diff --git a/docs/tutorial/js-custom.mdx b/docs/tutorial/js-custom.mdx index dd165592c..665a57f61 100644 --- a/docs/tutorial/js-custom.mdx +++ b/docs/tutorial/js-custom.mdx @@ -254,7 +254,8 @@ The benefit of `useValue(instance.rows)` over using `rows.get()`, is that the fi Since both `usePlugin` and `useValue` are hooks, they usual React rules for them apply; they need to be called unconditionally. So it is recommended to put them at the top of your component body. Both hooks can not only be used in the root `Component`, but also in any other component in your plugin component tree. -So it is not necessary to grab all the data at the root, or pass down the `instance` to all child components. +So it is not necessary to grab all the data at the root and pass it down using props. +Using `useValue` as deep in the component tree as possible will benefit performance. Finally (`(4)`) we render the data we have. The details have been left out here, as from here it is just idiomatic React code. The source of the other `MammalCard` component can be found [here](https://github.com/facebook/flipper/blob/master/desktop/plugins/public/seamammals/src/index.tsx#L113-L165).