Minor improvements

Summary:
Some styling fixes and minor improvements in DataTable, used by network plugin:

- be able to customise the context menu
- be able to customise how entire rows are copied and presented on the clipboard to be able to deviate from the standard JSON
- deeplink handling was made async, this gives the plugin the opportunity to first handle initial setup and rendering before trying to jump somewhere which is a typical use case for deeplinking

Reviewed By: passy

Differential Revision: D27947186

fbshipit-source-id: a56f081d60520c4bc2ad3c547a8ca5b9357e71a1
This commit is contained in:
Michel Weststrate
2021-04-23 09:28:45 -07:00
committed by Facebook GitHub Bot
parent ae88f5d200
commit faf8588097
11 changed files with 92 additions and 24 deletions

View File

@@ -24,6 +24,7 @@ import {
import {selectPlugin} from '../reducers/connections';
import {updateSettings} from '../reducers/settings';
import {switchPlugin} from '../reducers/pluginManager';
import {sleep} from 'flipper-plugin/src/utils/sleep';
interface PersistedState {
count: 1;
@@ -528,6 +529,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
);
});
await sleep(10);
expect(linksSeen).toEqual(['universe!']);
expect(renderer.baseElement).toMatchInlineSnapshot(`
<body>
@@ -558,6 +560,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
}),
);
});
await sleep(10);
expect(linksSeen).toEqual(['universe!']);
// ...nor does a random other store update that does trigger a plugin container render
@@ -580,6 +583,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
}),
);
});
await sleep(10);
expect(linksSeen).toEqual(['universe!', 'london!']);
// and same link does trigger if something else was selected in the mean time
@@ -601,6 +605,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
}),
);
});
await sleep(10);
expect(linksSeen).toEqual(['universe!', 'london!', 'london!']);
});
@@ -802,6 +807,7 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
);
});
await sleep(10);
expect(linksSeen).toEqual([theUniverse]);
expect(renderer.baseElement).toMatchInlineSnapshot(`
<body>
@@ -832,6 +838,7 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
}),
);
});
await sleep(10);
expect(linksSeen).toEqual([theUniverse]);
// ...nor does a random other store update that does trigger a plugin container render
@@ -854,6 +861,7 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
}),
);
});
await sleep(10);
expect(linksSeen).toEqual([theUniverse, 'london!']);
// and same link does trigger if something else was selected in the mean time
@@ -875,6 +883,7 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
}),
);
});
await sleep(10);
expect(linksSeen).toEqual([theUniverse, 'london!', 'london!']);
});
@@ -977,6 +986,7 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => {
pluginInstance.selectPlugin(definition.id, 'data');
expect(store.getState().connections.selectedPlugin).toBe(definition.id);
expect(pluginInstance.activatedStub).toBeCalledTimes(2);
await sleep(10);
expect(renderer.baseElement.querySelector('h1')).toMatchInlineSnapshot(`
<h1>
Plugin1

View File

@@ -402,7 +402,7 @@ test('plugins can receive deeplinks', async () => {
});
expect(plugin.instance.field1.get()).toBe('');
plugin.triggerDeepLink('test');
await plugin.triggerDeepLink('test');
expect(plugin.instance.field1.get()).toBe('test');
});
@@ -424,7 +424,7 @@ test('device plugins can receive deeplinks', async () => {
});
expect(plugin.instance.field1.get()).toBe('');
plugin.triggerDeepLink('test');
await plugin.triggerDeepLink('test');
expect(plugin.instance.field1.get()).toBe('test');
});
@@ -455,7 +455,7 @@ test('plugins can register menu entries', async () => {
});
expect(plugin.instance.counter.get()).toBe(0);
plugin.triggerDeepLink('test');
await plugin.triggerDeepLink('test');
plugin.triggerMenuEntry('createPaste');
plugin.triggerMenuEntry('Custom Action');
expect(plugin.instance.counter.get()).toBe(4);

View File

@@ -325,7 +325,14 @@ export abstract class BasePluginInstance {
this.assertNotDestroyed();
if (deepLink !== this.lastDeeplink) {
this.lastDeeplink = deepLink;
if (typeof setImmediate !== 'undefined') {
// we only want to trigger deeplinks after the plugin had a chance to render
setImmediate(() => {
this.events.emit('deeplink', deepLink);
});
} else {
this.events.emit('deeplink', deepLink);
}
}
}

View File

@@ -95,7 +95,7 @@ interface BasePluginResult {
/**
* Emulate triggering a deeplink
*/
triggerDeepLink(deeplink: unknown): void;
triggerDeepLink(deeplink: unknown): Promise<void>;
/**
* Grab all the persistable state, but will ignore any onExport handler
@@ -386,8 +386,13 @@ function createBasePluginResult(
exportStateAsync: () =>
pluginInstance.exportState(createStubIdler(), () => {}),
exportState: () => pluginInstance.exportStateSync(),
triggerDeepLink: (deepLink: unknown) => {
triggerDeepLink: async (deepLink: unknown) => {
pluginInstance.triggerDeepLink(deepLink);
return new Promise((resolve) => {
// this ensures the test won't continue until the setImmediate used by
// the deeplink handling event is handled
setImmediate(resolve);
});
},
destroy: () => pluginInstance.destroy(),
triggerMenuEntry: (action: string) => {

View File

@@ -49,7 +49,7 @@ export const DataFormatter = {
return (
value.toTimeString().split(' ')[0] +
'.' +
pad('' + value.getMilliseconds(), 3)
pad('' + value.getMilliseconds(), 3, '0')
);
}
if (value instanceof Map) {

View File

@@ -57,6 +57,8 @@ interface DataTableProps<T = any> {
// multiselect?: true
tableManagerRef?: RefObject<DataTableManager<T> | 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...
_testHeight?: number; // exposed for unit testing only
onCopyRows?(records: T[]): string;
onContextMenu?: (selection: undefined | T) => React.ReactElement;
}
export type DataTableColumn<T = any> = {
@@ -94,11 +96,13 @@ export interface RenderContext<T = any> {
export function DataTable<T extends object>(
props: DataTableProps<T>,
): React.ReactElement {
const {dataSource, onRowStyle, onSelect} = props;
const {dataSource, onRowStyle, onSelect, onCopyRows, onContextMenu} = props;
useAssertStableRef(dataSource, 'dataSource');
useAssertStableRef(onRowStyle, 'onRowStyle');
useAssertStableRef(props.onSelect, 'onRowSelect');
useAssertStableRef(props.columns, 'columns');
useAssertStableRef(onCopyRows, 'onCopyRows');
useAssertStableRef(onContextMenu, 'onContextMenu');
useAssertStableRef(props._testHeight, '_testHeight');
// lint disabled for conditional inclusion of a hook (_testHeight is asserted to be stable)
@@ -357,8 +361,18 @@ export function DataTable<T extends object>(
selection,
tableState.columns,
visibleColumns,
onCopyRows,
onContextMenu,
),
[dataSource, dispatch, selection, tableState.columns, visibleColumns],
[
dataSource,
dispatch,
selection,
tableState.columns,
visibleColumns,
onCopyRows,
onContextMenu,
],
);
useEffect(function initialSetup() {

View File

@@ -62,6 +62,13 @@ type DataManagerActions<T> =
addToSelection?: boolean;
}
>
| Action<
'selectItemById',
{
id: string | number;
addToSelection?: boolean;
}
>
| Action<
'addRangeToSelection',
{
@@ -161,6 +168,17 @@ export const dataTableManagerReducer = produce<
);
break;
}
case 'selectItemById': {
const {id, addToSelection} = action;
// TODO: fix that this doesn't jumpt selection if items are shifted! sorting is swapped etc
const idx = config.dataSource.getIndexOfKey(id);
if (idx !== -1) {
draft.selection = castDraft(
computeSetSelection(draft.selection, idx, addToSelection),
);
}
break;
}
case 'addRangeToSelection': {
const {start, end, allowUnselect} = action;
draft.selection = castDraft(
@@ -241,6 +259,7 @@ export type DataTableManager<T> = {
end: number,
allowUnselect?: boolean,
): void;
selectItemById(id: string | number, addToSelection?: boolean): void;
clearSelection(): void;
getSelectedItem(): T | undefined;
getSelectedItems(): readonly T[];
@@ -261,6 +280,9 @@ export function createDataTableManager<T>(
selectItem(index: number, addToSelection = false) {
dispatch({type: 'selectItem', nextIndex: index, addToSelection});
},
selectItemById(id, addToSelection = false) {
dispatch({type: 'selectItemById', id, addToSelection});
},
addRangeToSelection(start, end, allowUnselect = false) {
dispatch({type: 'addRangeToSelection', start, end, allowUnselect});
},

View File

@@ -11,6 +11,7 @@ import {CopyOutlined, FilterOutlined} from '@ant-design/icons';
import {Checkbox, Menu} from 'antd';
import {
DataTableDispatch,
getSelectedItem,
getSelectedItems,
Selection,
} from './DataTableManager';
@@ -21,12 +22,18 @@ import {DataSource} from '../../state/DataSource';
const {Item, SubMenu} = Menu;
function defaultOnCopyRows<T>(items: T[]) {
return JSON.stringify(items.length > 1 ? items : items[0], null, 2);
}
export function tableContextMenuFactory<T>(
datasource: DataSource<T>,
dispatch: DataTableDispatch<T>,
selection: Selection,
columns: DataTableColumn<T>[],
visibleColumns: DataTableColumn<T>[],
onCopyRows: (rows: T[]) => string = defaultOnCopyRows,
onContextMenu?: (selection: undefined | T) => React.ReactElement,
) {
const lib = tryGetFlipperLibImplementation();
if (!lib) {
@@ -40,6 +47,9 @@ export function tableContextMenuFactory<T>(
return (
<Menu>
{onContextMenu
? onContextMenu(getSelectedItem(datasource, selection))
: null}
<SubMenu
title="Filter on same"
icon={<FilterOutlined />}
@@ -81,9 +91,7 @@ export function tableContextMenuFactory<T>(
onClick={() => {
const items = getSelectedItems(datasource, selection);
if (items.length) {
lib.writeTextToClipboard(
JSON.stringify(items.length > 1 ? items : items[0], null, 2),
);
lib.writeTextToClipboard(onCopyRows(items));
}
}}>
Copy row(s)
@@ -94,9 +102,7 @@ export function tableContextMenuFactory<T>(
onClick={() => {
const items = getSelectedItems(datasource, selection);
if (items.length) {
lib.createPaste(
JSON.stringify(items.length > 1 ? items : items[0], null, 2),
);
lib.createPaste(onCopyRows(items));
}
}}>
Create paste

View File

@@ -111,7 +111,11 @@ const TableHeadContainer = styled.div({
borderBottom: `1px solid ${theme.dividerColor}`,
backgroundColor: theme.backgroundWash,
userSelect: 'none',
whiteSpace: 'nowrap',
borderLeft: `4px solid ${theme.backgroundWash}`, // space for selection, see TableRow
// hardcoded value to correct for the scrollbar in the main container.
// ideally we should measure this instead.
paddingRight: 15,
});
TableHeadContainer.displayName = 'TableHead:TableHeadContainer';

View File

@@ -48,6 +48,7 @@ const TableBodyRowContainer = styled.div<TableBodyRowContainerProps>(
borderLeft: props.highlighted
? `4px solid ${theme.primaryColor}`
: `4px solid transparent`,
borderBottom: `1px solid ${theme.dividerColor}`,
paddingTop: 1,
minHeight: DEFAULT_ROW_HEIGHT,
lineHeight: `${DEFAULT_ROW_HEIGHT - 2}px`,
@@ -74,7 +75,6 @@ const TableBodyColumnContainer = styled.div<{
flexGrow: props.width === undefined ? 1 : 0,
overflow: 'hidden',
padding: `0 ${theme.space.small}px`,
borderBottom: `1px solid ${theme.dividerColor}`,
verticalAlign: 'top',
// pre-wrap preserves explicit newlines and whitespace, and wraps as well when needed
whiteSpace: props.multiline ? 'pre-wrap' : 'nowrap',

View File

@@ -55,15 +55,15 @@ test('update and append', async () => {
expect(elem.length).toBe(1);
expect(elem[0].parentElement).toMatchInlineSnapshot(`
<div
class="css-w3o588-TableBodyRowContainer e1luu51r1"
class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
>
<div
class="css-1xxqqu6-TableBodyColumnContainer e1luu51r0"
class="css-744e08-TableBodyColumnContainer e1luu51r0"
>
test DataTable
</div>
<div
class="css-1xxqqu6-TableBodyColumnContainer e1luu51r0"
class="css-744e08-TableBodyColumnContainer e1luu51r0"
>
true
</div>
@@ -112,15 +112,15 @@ test('column visibility', async () => {
expect(elem.length).toBe(1);
expect(elem[0].parentElement).toMatchInlineSnapshot(`
<div
class="css-w3o588-TableBodyRowContainer e1luu51r1"
class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
>
<div
class="css-1xxqqu6-TableBodyColumnContainer e1luu51r0"
class="css-744e08-TableBodyColumnContainer e1luu51r0"
>
test DataTable
</div>
<div
class="css-1xxqqu6-TableBodyColumnContainer e1luu51r0"
class="css-744e08-TableBodyColumnContainer e1luu51r0"
>
true
</div>
@@ -137,10 +137,10 @@ test('column visibility', async () => {
expect(elem.length).toBe(1);
expect(elem[0].parentElement).toMatchInlineSnapshot(`
<div
class="css-w3o588-TableBodyRowContainer e1luu51r1"
class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
>
<div
class="css-1xxqqu6-TableBodyColumnContainer e1luu51r0"
class="css-744e08-TableBodyColumnContainer e1luu51r0"
>
test DataTable
</div>