From 0aadb862ee700e39a1d190617423885257fbecdb Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 16 Jun 2021 07:14:02 -0700 Subject: [PATCH] Use DataList Summary: With new abstraction, `DataList` matches what the plugin trying to render. Should fix: https://fb.workplace.com/groups/flippersupport/permalink/1145431339270856/ Changelog: [MobileConfig] Fix issues with scrolling not working and several other improvements Reviewed By: cekkaewnumchai Differential Revision: D28314408 fbshipit-source-id: 4d8fbe3d8e868f737750203cd568d94bae8b4108 --- desktop/flipper-plugin/src/ui/DataList.tsx | 285 +++++++++--------- .../src/ui/data-table/DataTable.tsx | 19 +- .../src/ui/data-table/DataTableManager.tsx | 9 +- .../flipper-plugin/src/utils/useLatestRef.tsx | 24 ++ .../src/utils/useMakeStableCallback.tsx | 34 +++ .../plugins/public/crash_reporter/Crashes.tsx | 2 +- .../ManageMockResponsePanel.tsx | 2 +- 7 files changed, 231 insertions(+), 144 deletions(-) create mode 100644 desktop/flipper-plugin/src/utils/useLatestRef.tsx create mode 100644 desktop/flipper-plugin/src/utils/useMakeStableCallback.tsx diff --git a/desktop/flipper-plugin/src/ui/DataList.tsx b/desktop/flipper-plugin/src/ui/DataList.tsx index b8322eb28..3bdd2cda4 100644 --- a/desktop/flipper-plugin/src/ui/DataList.tsx +++ b/desktop/flipper-plugin/src/ui/DataList.tsx @@ -7,14 +7,7 @@ * @format */ -import React, { - useCallback, - memo, - createRef, - useEffect, - useMemo, - useState, -} from 'react'; +import React, {memo, createRef, useMemo, useEffect, useCallback} from 'react'; import {DataFormatter} from './DataFormatter'; import {Layout} from './Layout'; import {Typography} from 'antd'; @@ -28,19 +21,13 @@ 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'; import {DataSource} from '../data-source'; +import {useMakeStableCallback} from '../utils/useMakeStableCallback'; const {Text} = Typography; -interface Item { - id: string; - title: string; - description?: string; -} - -interface DataListBaseProps { +type DataListBaseProps = { /** * Defines the styling of the component. By default shows a list, but alternatively the items can be displayed in a drop down */ @@ -50,14 +37,11 @@ interface DataListBaseProps { * By setting `scrollable={false}` the list will only take its natural size */ scrollable?: boolean; - /** - * The current selection - */ - selection: Atom; /** * Handler that is fired if selection is changed */ - onSelect?(id: string | undefined, value: T | undefined): void; + selection?: string | undefined; + onSelect?(id: string | undefined, value: Item | undefined): void; className?: string; style?: React.CSSProperties; /** @@ -67,103 +51,150 @@ interface DataListBaseProps { /** * Custom render function. By default the component will render the `title` in bold and description (if any) below it */ - onRenderItem?: ItemRenderer; + onRenderItem?: ItemRenderer; + /** + * The attributes that will be picked as id/title/description for the default rendering. + * Defaults to id/title/description, but can be customized + */ + + titleAttribute?: keyof Item & string; + descriptionAttribute?: keyof Item & string; /** * Show a right arrow by default */ enableArrow?: boolean; -} +} & (Item extends {id: string} + ? { + idAttribute?: keyof Item & string; // optional if id field is present + } + : { + idAttribute: keyof Item & string; + }); -export type DataListProps = DataListBaseProps & +export type DataListProps = DataListBaseProps & // Some table props are set by DataList instead, so override them - Omit, 'records' | 'dataSource' | 'columns' | 'onSelect'>; + Omit, 'records' | 'dataSource' | 'columns' | 'onSelect'>; -export const DataList: React.FC> = function DataList< - T extends Item, ->({ - 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>(); +export const DataList: ((props: DataListProps) => React.ReactElement) & { + Item: React.FC; +} = Object.assign( + function ({ + onSelect: baseOnSelect, + selection, + className, + style, + items, + onRenderItem, + enableArrow, + idAttribute, + titleAttribute, + descriptionAttribute, + ...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'); - useAssertStableRef(onRenderItem, 'onRenderItem'); - useAssertStableRef(enableArrow, 'enableArrow'); + useAssertStableRef(onRenderItem, 'onRenderItem'); + useAssertStableRef(enableArrow, 'enableArrow'); + const onSelect = useMakeStableCallback(baseOnSelect); - // create local selection atom if none provided - // eslint-disable-next-line - const selection = baseSelection ?? useState(() => createState())[0]; - - const handleSelect = useCallback( - (item: T | undefined) => { - selection.set(item?.id); - }, - [selection], - ); - - const dataListColumns: DataTableColumn[] = useMemo( - () => [ - { - key: 'id' as const, - wrap: true, - onRender(item: T, selected: boolean, index: number) { - return onRenderItem ? ( - onRenderItem(item, selected, index) - ) : ( - - ); - }, + const handleSelect = useCallback( + (item: T | undefined) => { + if (!item) { + onSelect?.(undefined, undefined); + } else { + const id = '' + item[idAttribute!]; + if (id == null) { + throw new Error(`No valid identifier for attribute ${idAttribute}`); + } + onSelect?.(id, item); + } }, - ], - [onRenderItem, enableArrow], - ); + [onSelect, idAttribute], + ); - 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], - ); + useEffect(() => { + if (selection) { + tableManagerRef.current?.selectItemById(selection); + } else { + tableManagerRef.current?.clearSelection(); + } + }, [selection, tableManagerRef]); - return ( - - - {...tableProps} - tableManagerRef={tableManagerRef} - records={Array.isArray(items) ? items : undefined} - dataSource={Array.isArray(items) ? undefined : (items as any)} - recordsKey="id" - columns={dataListColumns} - onSelect={handleSelect} - /> - - ); -}; + const dataListColumns: DataTableColumn[] = useMemo( + () => [ + { + key: idAttribute!, + wrap: true, + onRender(item: T, selected: boolean, index: number) { + return onRenderItem ? ( + onRenderItem(item, selected, index) + ) : ( + + ); + }, + }, + ], + [ + onRenderItem, + enableArrow, + titleAttribute, + descriptionAttribute, + idAttribute, + ], + ); -DataList.defaultProps = { + return ( + + + {...tableProps} + tableManagerRef={tableManagerRef} + records={Array.isArray(items) ? items : undefined!} + dataSource={Array.isArray(items) ? undefined! : (items as any)} + recordsKey={idAttribute} + columns={dataListColumns} + onSelect={handleSelect} + /> + + ); + }, + { + Item: memo(({title, description, enableArrow}: DataListItemProps) => { + return ( + + + {typeof title === 'string' ? ( + + {DataFormatter.format(title)} + + ) : ( + title + )} + {typeof description === 'string' ? ( + + {DataFormatter.format(description)} + + ) : ( + description + )} + + {enableArrow && ( + + + + )} + + ); + }), + }, +); + +(DataList as React.FC>).defaultProps = { type: 'default', scrollable: true, enableSearchbar: false, @@ -171,40 +202,20 @@ DataList.defaultProps = { enableArrow: true, enableContextMenu: false, enableMultiSelect: false, + idAttribute: 'id', + titleAttribute: 'title', + descriptionAttribute: 'description', +}; +(DataList.Item as React.FC).defaultProps = { + enableArrow: true, }; -const DataListItem = memo( - ({ - title, - description, - enableArrow, - }: { - // TODO: add icon support - title: string; - description?: string; - enableArrow?: boolean; - }) => { - return ( - - - - {DataFormatter.format(title)} - - {description != null && ( - - {DataFormatter.format(description)} - - )} - - {enableArrow && ( - - - - )} - - ); - }, -); +interface DataListItemProps { + // TODO: add icon support + title?: string | React.ReactElement; + description?: string | React.ReactElement; + enableArrow?: boolean; +} const ArrowWrapper = styled.div({ flex: 0, diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx index f801866da..58940b765 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx @@ -60,6 +60,7 @@ interface DataTableBaseProps { enableColumnHeaders?: boolean; enableMultiSelect?: boolean; enableContextMenu?: boolean; + enablePersistSettings?: boolean; // if set (the default) will grow and become scrollable. Otherwise will use natural size scrollable?: boolean; extraActions?: React.ReactElement; @@ -153,6 +154,7 @@ export function DataTable( scope, virtualizerRef, autoScroll: props.enableAutoScroll, + enablePersistSettings: props.enablePersistSettings, }), ); @@ -177,15 +179,20 @@ export function DataTable( const renderingConfig = useMemo>(() => { let startIndex = 0; + return { columns: visibleColumns, onMouseEnter(e, _item, index) { - if (dragging.current && e.buttons === 1) { + if (dragging.current && e.buttons === 1 && props.enableMultiSelect) { // by computing range we make sure no intermediate items are missed when scrolling fast tableManager.addRangeToSelection(startIndex, index); } }, onMouseDown(e, _item, index) { + if (!props.enableMultiSelect && e.buttons > 1) { + tableManager.selectItem(index, false, true); + return; + } if (!dragging.current) { if (e.buttons > 1) { // for right click we only want to add if needed, not deselect @@ -218,7 +225,13 @@ export function DataTable( } : undefined, }; - }, [visibleColumns, tableManager, onRowStyle, props.enableContextMenu]); + }, [ + visibleColumns, + tableManager, + onRowStyle, + props.enableContextMenu, + props.enableMultiSelect, + ]); const itemRenderer = useCallback( function itemRenderer( @@ -454,7 +467,7 @@ export function DataTable( searchValue={searchValue} useRegex={tableState.useRegex} dispatch={dispatch as any} - contextMenu={contexMenu} + contextMenu={props.enableContextMenu ? contexMenu : undefined} extraActions={props.extraActions} /> )} diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx index f619952f2..6641852f2 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx @@ -98,6 +98,7 @@ type DataManagerConfig = { onSelect: undefined | ((item: T | undefined, items: T[]) => void); virtualizerRef: MutableRefObject; autoScroll?: boolean; + enablePersistSettings?: boolean; }; type DataManagerState = { @@ -272,6 +273,7 @@ export type DataTableManager = { toggleColumnVisibility(column: keyof T): void; sortColumn(column: keyof T, direction?: SortDirection): void; setSearchValue(value: string): void; + dataSource: DataSource; }; export function createDataTableManager( @@ -315,6 +317,7 @@ export function createDataTableManager( setSearchValue(value) { dispatch({type: 'setSearchValue', value}); }, + dataSource, }; } @@ -324,7 +327,9 @@ export function createInitialState( const storageKey = `${config.scope}:DataTable:${config.defaultColumns .map((c) => c.key) .join(',')}`; - const prefs = loadStateFromStorage(storageKey); + const prefs = config.enablePersistSettings + ? loadStateFromStorage(storageKey) + : undefined; let initialColumns = computeInitialColumns(config.defaultColumns); if (prefs) { // merge prefs with the default column config @@ -411,7 +416,7 @@ export function savePreferences( state: DataManagerState, scrollOffset: number, ) { - if (!state.config.scope) { + if (!state.config.scope || !state.config.enablePersistSettings) { return; } const prefs: PersistedState = { diff --git a/desktop/flipper-plugin/src/utils/useLatestRef.tsx b/desktop/flipper-plugin/src/utils/useLatestRef.tsx new file mode 100644 index 000000000..7458b0706 --- /dev/null +++ b/desktop/flipper-plugin/src/utils/useLatestRef.tsx @@ -0,0 +1,24 @@ +/** + * 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 {useEffect, useRef} from 'react'; + +/** + * Creates a ref object that is always synced from the value passed in. + */ +export function useLatestRef(latest: T): {readonly current: T} { + const latestRef = useRef(latest); + + // TODO: sync eagerly (in render) or late? Introduce a `syncEarly` flag as second arg + useEffect(() => { + latestRef.current = latest; + }, [latest]); + + return latestRef; +} diff --git a/desktop/flipper-plugin/src/utils/useMakeStableCallback.tsx b/desktop/flipper-plugin/src/utils/useMakeStableCallback.tsx new file mode 100644 index 000000000..c84869952 --- /dev/null +++ b/desktop/flipper-plugin/src/utils/useMakeStableCallback.tsx @@ -0,0 +1,34 @@ +/** + * 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 {useCallback} from 'react'; +import {useLatestRef} from './useLatestRef'; + +/** + * This hook can be used to avoid forcing consumers of a component to wrap their callbacks + * in useCallback, by creating wrapper callback that redirects to the lastest prop passed in. + * + * Use this hook if you would like to avoid that passing a new callback to this component, + * will cause child components to rerender when the callback is passed further down. + * + * Use it like: `const onSelect = useMakeStableCallback(props.onSelect)`. + * @param fn + */ +export function useMakeStableCallback< + T extends undefined | ((...args: any[]) => any), +>(fn: T): T { + const latestFn = useLatestRef(fn); + + return useCallback( + (...args: any[]) => { + return latestFn.current?.apply(null, args); + }, + [latestFn], + ) as any; +} diff --git a/desktop/plugins/public/crash_reporter/Crashes.tsx b/desktop/plugins/public/crash_reporter/Crashes.tsx index e99a6b64d..49e01726e 100644 --- a/desktop/plugins/public/crash_reporter/Crashes.tsx +++ b/desktop/plugins/public/crash_reporter/Crashes.tsx @@ -37,7 +37,7 @@ export function Crashes() { title: crash.reason ?? crash.name, description: `${crash.date.toLocaleString()} - ${crash.name}`, }))} - selection={plugin.selectedCrash} + selection={selectedCrashId} onRenderEmpty={null} /> {selectedCrash ? ( diff --git a/desktop/plugins/public/network/request-mocking/ManageMockResponsePanel.tsx b/desktop/plugins/public/network/request-mocking/ManageMockResponsePanel.tsx index 746be05b8..5fc0c84ea 100644 --- a/desktop/plugins/public/network/request-mocking/ManageMockResponsePanel.tsx +++ b/desktop/plugins/public/network/request-mocking/ManageMockResponsePanel.tsx @@ -159,7 +159,7 @@ export function ManageMockResponsePanel(props: Props) {