From dec8e88aeb5c3d0148e1174c502acc398a1df689 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 16 Mar 2021 14:54:53 -0700 Subject: [PATCH] Add row styling Summary: Added styling / coloring to the new logs plugin, to bring it closer to feature completeness. Made the colum headers slightly more compact Also made the API more foolproof by introducing the `useAssertStableRef` hook, that will protect against accidentally passing in props that would invalidate rendering every time. Reviewed By: passy Differential Revision: D26635063 fbshipit-source-id: 60b2af8db3cc3c12d8d25d922cf1735aed91dd2c --- .../src/state/datasource/DataSource.tsx | 10 +++++ desktop/flipper-plugin/src/ui/Layout.tsx | 2 +- .../src/ui/datatable/ColumnFilter.tsx | 14 +++---- .../src/ui/datatable/DataTable.tsx | 21 +++++++--- .../src/ui/datatable/TableHead.tsx | 10 +++-- .../src/ui/datatable/TableRow.tsx | 39 ++++++++++++------- .../ui/datatable/__tests__/DataTable.node.tsx | 16 ++++---- .../src/ui/datatable/useDataTableManager.tsx | 4 ++ .../src/utils/useAssertStableRef.tsx | 29 ++++++++++++++ 9 files changed, 103 insertions(+), 42 deletions(-) create mode 100644 desktop/flipper-plugin/src/utils/useAssertStableRef.tsx diff --git a/desktop/flipper-plugin/src/state/datasource/DataSource.tsx b/desktop/flipper-plugin/src/state/datasource/DataSource.tsx index e2a954f4d..61ac62ab7 100644 --- a/desktop/flipper-plugin/src/state/datasource/DataSource.tsx +++ b/desktop/flipper-plugin/src/state/datasource/DataSource.tsx @@ -328,6 +328,16 @@ export class DataSource< this.rebuildOutput(); } + /** + * Returns a fork of this dataSource, that shares the source data with this dataSource, + * but has it's own FSRW pipeline, to allow multiple views on the same data + */ + fork(): DataSource { + throw new Error( + 'Not implemented. Please contact oncall if this feature is needed', + ); + } + emitDataEvent(event: DataEvent) { this.dataUpdateQueue.push(event); // TODO: schedule diff --git a/desktop/flipper-plugin/src/ui/Layout.tsx b/desktop/flipper-plugin/src/ui/Layout.tsx index f2d43b7c4..4f815d0ac 100644 --- a/desktop/flipper-plugin/src/ui/Layout.tsx +++ b/desktop/flipper-plugin/src/ui/Layout.tsx @@ -140,7 +140,7 @@ type SplitLayoutProps = { center?: boolean; gap?: Spacing; children: [React.ReactNode, React.ReactNode]; - style?: React.HTMLAttributes['style']; + style?: CSSProperties; }; function renderSplitLayout( diff --git a/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx b/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx index 0e52fdf85..6ea29edf1 100644 --- a/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/ColumnFilter.tsx @@ -8,7 +8,6 @@ */ import {useMemo, useState} from 'react'; -import styled from '@emotion/styled'; import React from 'react'; import {theme} from '../theme'; import type {DataTableColumn} from './DataTable'; @@ -19,12 +18,6 @@ import {Layout} from '../Layout'; const {Text} = Typography; -export const HeaderButton = styled(Button)({ - padding: 4, - backgroundColor: theme.backgroundWash, - borderRadius: 0, -}); - export type ColumnFilterHandlers = { onAddColumnFilter(columnId: string, value: string): void; onRemoveColumnFilter(columnId: string, index: number): void; @@ -135,14 +128,17 @@ export function FilterIcon({ return ( - - + ); } diff --git a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx index 69b0fc3b9..0ccd28a05 100644 --- a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx @@ -15,6 +15,7 @@ import React, { useState, RefObject, MutableRefObject, + CSSProperties, } from 'react'; import {TableRow, DEFAULT_ROW_HEIGHT} from './TableRow'; import {DataSource} from '../../state/datasource/DataSource'; @@ -29,14 +30,15 @@ import {theme} from '../theme'; import {tableContextMenuFactory} from './TableContextMenu'; import {Typography} from 'antd'; import {CoffeeOutlined, SearchOutlined} from '@ant-design/icons'; +import {useAssertStableRef} from '../../utils/useAssertStableRef'; interface DataTableProps { columns: DataTableColumn[]; dataSource: DataSource; autoScroll?: boolean; extraActions?: React.ReactElement; - // custom onSearch(text, row) option? - onSelect?(item: T | undefined, items: T[]): void; + onSelect?(record: T | undefined, records: T[]): void; + onRowStyle?(record: T): CSSProperties | undefined; // multiselect?: true tableManagerRef?: RefObject; _testHeight?: number; // exposed for unit testing only @@ -76,7 +78,13 @@ export interface RenderContext { export function DataTable( props: DataTableProps, ): React.ReactElement { - const {dataSource} = props; + const {dataSource, onRowStyle} = props; + useAssertStableRef(dataSource, 'dataSource'); + useAssertStableRef(onRowStyle, 'onRowStyle'); + useAssertStableRef(props.onSelect, 'onRowSelect'); + useAssertStableRef(props.columns, 'columns'); + useAssertStableRef(props._testHeight, '_testHeight'); + const virtualizerRef = useRef(); const tableManager = useDataTableManager( dataSource, @@ -135,7 +143,7 @@ export function DataTable( const itemRenderer = useCallback( function itemRenderer( - item: any, + record: T, index: number, renderContext: RenderContext, ) { @@ -143,15 +151,16 @@ export function DataTable( ); }, - [selection], + [selection, onRowStyle], ); /** diff --git a/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx b/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx index 8224185a0..4c527e0ff 100644 --- a/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/TableHead.tsx @@ -24,7 +24,8 @@ import {Typography} from 'antd'; import {CaretDownFilled, CaretUpFilled} from '@ant-design/icons'; import {Layout} from '../Layout'; import {Sorting, OnColumnResize, SortDirection} from './useDataTableManager'; -import {ColumnFilterHandlers, FilterIcon, HeaderButton} from './ColumnFilter'; +import {ColumnFilterHandlers, FilterIcon} from './ColumnFilter'; +import {DEFAULT_ROW_HEIGHT} from './TableRow'; const {Text} = Typography; @@ -90,14 +91,15 @@ TableHeaderColumnInteractive.displayName = const TableHeadColumnContainer = styled.div<{ width: Width; }>((props) => ({ + // height: DEFAULT_ROW_HEIGHT, flexShrink: props.width === undefined ? 1 : 0, flexGrow: props.width === undefined ? 1 : 0, width: props.width === undefined ? '100%' : props.width, - paddingLeft: 4, + paddingLeft: 8, [`:hover ${SortIconsContainer}`]: { visibility: 'visible', }, - [`&:hover ${HeaderButton}`]: { + [`&:hover button`]: { visibility: 'visible !important' as any, }, })); @@ -172,7 +174,7 @@ function TableHeadColumn({ }} role="button" tabIndex={0}> - + {column.title ?? <> } { if (props.highlighted) { - return theme.backgroundTransparentHover; + return theme.backgroundWash; } return undefined; }; @@ -47,8 +47,14 @@ const TableBodyRowContainer = styled.div( backgroundColor: backgroundColor(props), borderLeft: props.highlighted ? `4px solid ${theme.primaryColor}` - : `4px solid ${theme.backgroundDefault}`, + : `4px solid transparent`, + paddingTop: 1, + borderBottom: `1px solid ${theme.dividerColor}`, minHeight: DEFAULT_ROW_HEIGHT, + lineHeight: `${DEFAULT_ROW_HEIGHT - 2}px`, + '& .anticon': { + lineHeight: `${DEFAULT_ROW_HEIGHT - 2}px`, + }, overflow: 'hidden', width: '100%', flexShrink: 0, @@ -74,7 +80,6 @@ const TableBodyColumnContainer = styled.div<{ wordWrap: props.multiline ? 'break-word' : 'normal', width: props.width, justifyContent: props.justifyContent, - borderBottom: `1px solid ${theme.dividerColor}`, '&::selection': { color: 'inherit', backgroundColor: theme.buttonDefaultBackground, @@ -85,32 +90,38 @@ TableBodyColumnContainer.displayName = 'TableRow:TableBodyColumnContainer'; type Props = { config: RenderContext; highlighted: boolean; - value: any; + record: any; itemIndex: number; + style?: CSSProperties; }; -export const TableRow = memo(function TableRow(props: Props) { - const {config, highlighted, value: row} = props; +export const TableRow = memo(function TableRow({ + record, + itemIndex, + highlighted, + style, + config, +}: Props) { return ( { - props.config.onMouseDown(e, props.value, props.itemIndex); + config.onMouseDown(e, record, itemIndex); }} onMouseEnter={(e) => { - props.config.onMouseEnter(e, props.value, props.itemIndex); - }}> + config.onMouseEnter(e, record, itemIndex); + }} + style={style}> {config.columns .filter((col) => col.visible) .map((col) => { const value = (col as any).onRender - ? (col as any).onRender(row) - : normalizeCellValue((row as any)[col.key]); + ? (col as any).onRender(record) + : normalizeCellValue((record as any)[col.key]); return ( { expect(elem.length).toBe(1); expect(elem[0].parentElement).toMatchInlineSnapshot(`
test DataTable
true
@@ -112,15 +112,15 @@ test('column visibility', async () => { expect(elem.length).toBe(1); expect(elem[0].parentElement).toMatchInlineSnapshot(`
test DataTable
true
@@ -137,10 +137,10 @@ test('column visibility', async () => { expect(elem.length).toBe(1); expect(elem[0].parentElement).toMatchInlineSnapshot(`
test DataTable
diff --git a/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx b/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx index dc8d356d2..24cc9118f 100644 --- a/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx @@ -246,6 +246,10 @@ export function useDataTableManager( [currentFilter, dataSource], ); + // if the component unmounts, we reset the SFRW pipeline to + // avoid wasting resources in the background + useEffect(() => () => dataSource.reset(), [dataSource]); + return { /** The default columns, but normalized */ columns, diff --git a/desktop/flipper-plugin/src/utils/useAssertStableRef.tsx b/desktop/flipper-plugin/src/utils/useAssertStableRef.tsx new file mode 100644 index 000000000..14bfe2f43 --- /dev/null +++ b/desktop/flipper-plugin/src/utils/useAssertStableRef.tsx @@ -0,0 +1,29 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {useRef} from 'react'; + +/** + * This hook will throw in development builds if the value passed in is stable. + * Use this if to make sure consumers aren't creating or changing certain props over time + * (intentionally or accidentally) + */ +export const useAssertStableRef = + process.env.NODE_ENV === 'development' + ? function useAssertStableRef(value: any, prop: string) { + const ref = useRef(value); + if (ref.current !== value) { + throw new Error( + `[useAssertStableRef] An unstable reference was passed to this component as property '${prop}'. For optimization purposes we expect that this prop doesn't change over time. You might want to create the value passed to this prop outside the render closure, store it in useCallback / useMemo / useState, or set a key on the parent component`, + ); + } + } + : function (_value: any, _prop: string) { + // no-op + };