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

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<T> {
+    throw new Error(
+      'Not implemented. Please contact oncall if this feature is needed',
+    );
+  }
+
   emitDataEvent(event: DataEvent<T>) {
     this.dataUpdateQueue.push(event);
     // TODO: schedule

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<HTMLDivElement>['style'];
+  style?: CSSProperties;
 };
 
 function renderSplitLayout(

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 (
     <Dropdown overlay={menu} trigger={['click']}>
-      <HeaderButton
+      <Button
+        size="small"
         type="text"
         style={{
+          backgroundColor: theme.backgroundWash,
+          borderRadius: 0,
           visibility: isActive ? 'visible' : 'hidden',
           color: isActive ? theme.primaryColor : theme.disabledColor,
         }}>
         <FilterFilled />
-      </HeaderButton>
+      </Button>
     </Dropdown>
   );
 }

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<T = any> {
   columns: DataTableColumn<T>[];
   dataSource: DataSource<T, any, any>;
   autoScroll?: boolean;
   extraActions?: React.ReactElement;
-  // custom onSearch(text, row) option?
+  onSelect?(record: T | undefined, records: T[]): void;
-  onSelect?(item: T | undefined, items: T[]): void;
+  onRowStyle?(record: T): CSSProperties | undefined;
   // multiselect?: true
   tableManagerRef?: RefObject<TableManager>;
   _testHeight?: number; // exposed for unit testing only
@@ -76,7 +78,13 @@ export interface RenderContext<T = any> {
 export function DataTable<T extends object>(
   props: DataTableProps<T>,
 ): 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<DataSourceVirtualizer | undefined>();
   const tableManager = useDataTableManager(
     dataSource,
@@ -135,7 +143,7 @@ export function DataTable<T extends object>(
 
   const itemRenderer = useCallback(
     function itemRenderer(
-      item: any,
+      record: T,
       index: number,
       renderContext: RenderContext<T>,
     ) {
@@ -143,15 +151,16 @@ export function DataTable<T extends object>(
         <TableRow
           key={index}
           config={renderContext}
-          value={item}
+          record={record}
           itemIndex={index}
           highlighted={
             index === selection.current || selection.items.has(index)
           }
+          style={onRowStyle?.(record)}
         />
       );
     },
-    [selection],
+    [selection, onRowStyle],
   );
 
   /**

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}>
-        <Text strong>
+        <Text type="secondary">
           {column.title ?? <>&nbsp;</>}
           <SortIcons
             direction={sorted}

desktop/flipper-plugin/src/ui/datatable/TableRow.tsx

@@ -7,7 +7,7 @@
  * @format
  */
 
-import React, {memo} from 'react';
+import React, {CSSProperties, memo} from 'react';
 import styled from '@emotion/styled';
 import {theme} from 'flipper-plugin';
 import type {RenderContext} from './DataTable';
@@ -23,7 +23,7 @@ type TableBodyRowContainerProps = {
 
 const backgroundColor = (props: TableBodyRowContainerProps) => {
   if (props.highlighted) {
-    return theme.backgroundTransparentHover;
+    return theme.backgroundWash;
   }
   return undefined;
 };
@@ -47,8 +47,14 @@ const TableBodyRowContainer = styled.div<TableBodyRowContainerProps>(
     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<any>;
   highlighted: boolean;
-  value: any;
+  record: any;
   itemIndex: number;
+  style?: CSSProperties;
 };
 
-export const TableRow = memo(function TableRow(props: Props) {
+export const TableRow = memo(function TableRow({
-  const {config, highlighted, value: row} = props;
+  record,
+  itemIndex,
+  highlighted,
+  style,
+  config,
+}: Props) {
   return (
     <TableBodyRowContainer
       highlighted={highlighted}
-      data-key={row.key}
+      data-key={record.key}
       onMouseDown={(e) => {
-        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)
+            ? (col as any).onRender(record)
-            : normalizeCellValue((row as any)[col.key]);
+            : normalizeCellValue((record as any)[col.key]);
 
           return (
             <TableBodyColumnContainer
-              className="ant-table-cell"
               key={col.key as string}
               multiline={col.wrap}
               justifyContent={col.align ? col.align : 'flex-start'}

desktop/flipper-plugin/src/ui/datatable/__tests__/DataTable.node.tsx

@@ -55,15 +55,15 @@ test('update and append', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-tihkal-TableBodyRowContainer efe0za01"
+        class="css-8pa5c2-TableBodyRowContainer efe0za01"
       >
         <div
-          class="ant-table-cell css-1u65yt0-TableBodyColumnContainer efe0za00"
+          class="css-kkcfb6-TableBodyColumnContainer efe0za00"
         >
           test DataTable
         </div>
         <div
-          class="ant-table-cell css-1u65yt0-TableBodyColumnContainer efe0za00"
+          class="css-kkcfb6-TableBodyColumnContainer efe0za00"
         >
           true
         </div>
@@ -112,15 +112,15 @@ test('column visibility', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-tihkal-TableBodyRowContainer efe0za01"
+        class="css-8pa5c2-TableBodyRowContainer efe0za01"
       >
         <div
-          class="ant-table-cell css-1u65yt0-TableBodyColumnContainer efe0za00"
+          class="css-kkcfb6-TableBodyColumnContainer efe0za00"
         >
           test DataTable
         </div>
         <div
-          class="ant-table-cell css-1u65yt0-TableBodyColumnContainer efe0za00"
+          class="css-kkcfb6-TableBodyColumnContainer efe0za00"
         >
           true
         </div>
@@ -137,10 +137,10 @@ test('column visibility', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-tihkal-TableBodyRowContainer efe0za01"
+        class="css-8pa5c2-TableBodyRowContainer efe0za01"
       >
         <div
-          class="ant-table-cell css-1u65yt0-TableBodyColumnContainer efe0za00"
+          class="css-kkcfb6-TableBodyColumnContainer efe0za00"
         >
           test DataTable
         </div>

desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx

@@ -246,6 +246,10 @@ export function useDataTableManager<T>(
     [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,

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
+      };