From e07d5c5bfe3b33f996e3ab855a79c8d6668c3b32 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Mon, 30 May 2022 04:37:25 -0700 Subject: [PATCH] Added support for dotted key paths in Data table column Summary: This adds support for the key of DataTableColumn to be a dotted path into a nested object, e.g foo.bar. Currently the typescript types only allow a top level key to be set, making this feature currently unusuable from plugin code. While this could be addressed in a future commit the intention of this is to allow the user to add custom fields into their table columns at run time Note there is a side effect to free text search from this commit. Previously it would search all top keys in the object. Now it will only search in columns that are in the table. changelog: Searching data table will now only search columns in the table, rather than all top level attributes of the object Reviewed By: mweststrate Differential Revision: D36663929 fbshipit-source-id: 3688e9f26aa7e1828f8e9ee69f8e6f86268c8a54 --- .../src/ui/data-table/DataTable.tsx | 1 + .../src/ui/data-table/DataTableManager.tsx | 39 +++- .../src/ui/data-table/TableContextMenu.tsx | 5 +- .../src/ui/data-table/TableRow.tsx | 3 +- .../data-table/__tests__/DataTable.node.tsx | 167 ++++++++++++------ 5 files changed, 152 insertions(+), 63 deletions(-) diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx index 95bed6280..8c56ca7dc 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx @@ -94,6 +94,7 @@ type DataTableInput = }; export type DataTableColumn = { + //this can be a dotted path into a nest objects. e.g foo.bar key: keyof T & string; // possible future extension: getValue(row) (and free-form key) to support computed columns onRender?: (row: T, selected: boolean, index: number) => React.ReactNode; diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx index 33a43c27b..77a47b078 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx @@ -250,7 +250,7 @@ export const dataTableManagerReducer = produce< addColumnFilter( draft.columns, action.column, - (item as any)[action.column], + getValueAtPath(item, String(action.column)), index === 0, // remove existing filters before adding the first ); }); @@ -541,6 +541,25 @@ function computeInitialColumns( })); } +/** + * A somewhat primitive and unsafe way to access nested fields an object. + * @param obj keys should only be strings + * @param keyPath dotted string path, e.g foo.bar + * @returns value at the key path + */ +export function getValueAtPath(obj: any, keyPath: string): any { + let res = obj; + for (const key of keyPath.split('.')) { + if (res == null) { + return null; + } else { + res = res[key]; + } + } + + return res; +} + export function computeDataTableFilter( searchValue: string, useRegex: boolean, @@ -563,7 +582,10 @@ export function computeDataTableFilter( for (const column of filteringColumns) { const rowMatchesFilter = column.filters!.some( (f) => - f.enabled && String(item[column.key]).toLowerCase().includes(f.value), + f.enabled && + String(getValueAtPath(item, column.key)) + .toLowerCase() + .includes(f.value), ); if (column.inversed && rowMatchesFilter) { return false; @@ -572,11 +594,14 @@ export function computeDataTableFilter( return false; } } - return Object.values(item).some((v) => - searchRegex - ? searchRegex.test(String(v)) - : String(v).toLowerCase().includes(searchString), - ); + + return columns + .map((c) => getValueAtPath(item, c.key)) + .some((v) => + searchRegex + ? searchRegex.test(String(v)) + : String(v).toLowerCase().includes(searchString), + ); }; } diff --git a/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx b/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx index 4e898a5a9..af1caccbe 100644 --- a/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx @@ -13,6 +13,7 @@ import { DataTableDispatch, getSelectedItem, getSelectedItems, + getValueAtPath, Selection, } from './DataTableManager'; import React from 'react'; @@ -136,7 +137,9 @@ export function tableContextMenuFactory( const items = getSelectedItems(datasource, selection); if (items.length) { lib.writeTextToClipboard( - items.map((item) => '' + item[column.key]).join('\n'), + items + .map((item) => '' + getValueAtPath(item, column.key)) + .join('\n'), ); } }}> diff --git a/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx b/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx index ab0de2e79..f6e4c3e2d 100644 --- a/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx @@ -15,6 +15,7 @@ import {Width} from '../../utils/widthUtils'; import {DataFormatter} from '../DataFormatter'; import {Dropdown} from 'antd'; import {contextMenuTrigger} from '../data-inspector/DataInspectorNode'; +import {getValueAtPath} from './DataTableManager'; // heuristic for row estimation, should match any future styling updates export const DEFAULT_ROW_HEIGHT = 24; @@ -159,5 +160,5 @@ export function renderColumnValue( ) { return col.onRender ? col.onRender(record, highlighted, itemIndex) - : DataFormatter.format((record as any)[col.key], col.formatters); + : DataFormatter.format(getValueAtPath(record, col.key), col.formatters); } diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx index eb9c5d98a..3e2d15218 100644 --- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx @@ -50,23 +50,23 @@ test('update and append', async () => { const elem = await rendering.findAllByText('test DataTable'); expect(elem.length).toBe(1); expect(elem[0].parentElement).toMatchInlineSnapshot(` -
-
- test DataTable -
-
- true -
-
- `); +
+
+ test DataTable +
+
+ true +
+
+ `); } act(() => { @@ -104,23 +104,23 @@ test('column visibility', async () => { const elem = await rendering.findAllByText('test DataTable'); expect(elem.length).toBe(1); expect(elem[0].parentElement).toMatchInlineSnapshot(` -
-
- test DataTable -
-
- true -
-
- `); +
+
+ test DataTable +
+
+ true +
+
+ `); } // hide done @@ -131,17 +131,17 @@ test('column visibility', async () => { const elem = await rendering.findAllByText('test DataTable'); expect(elem.length).toBe(1); expect(elem[0].parentElement).toMatchInlineSnapshot(` -
-
- test DataTable -
-
- `); +
+
+ test DataTable +
+
+ `); } // reset @@ -285,15 +285,26 @@ test('search', async () => { }); test('compute filters', () => { + const levelCol = {key: 'level'}; + const titleCol = {key: 'title'}; + const doneCol = {key: 'done'}; + const baseColumns = [levelCol, titleCol, doneCol]; + const coffee = { level: 'info', title: 'Drink coffee', done: true, + extras: { + comment: 'tasty', + }, }; const espresso = { level: 'info', title: 'Make espresso', done: false, + extras: { + comment: 'dull', + }, }; const meet = { level: 'error', @@ -320,52 +331,64 @@ test('compute filters', () => { ).toBeUndefined(); { - const filter = computeDataTableFilter('tEsT', false, [])!; + const filter = computeDataTableFilter('tEsT', false, baseColumns)!; expect(data.filter(filter)).toEqual([]); } { - const filter = computeDataTableFilter('EE', false, [])!; + const filter = computeDataTableFilter('EE', false, baseColumns)!; expect(data.filter(filter)).toEqual([coffee, meet]); } { - const filter = computeDataTableFilter('D', false, [])!; + const filter = computeDataTableFilter('D', false, baseColumns)!; + expect(data.filter(filter)).toEqual([coffee]); + } + + const commentCol = {key: 'extras.comment'}; + { + // free search on value tasty in nested column + const filter = computeDataTableFilter('tasty', false, [ + ...baseColumns, + commentCol, + ])!; expect(data.filter(filter)).toEqual([coffee]); } { // regex, positive (mind the double escaping of \\b) - const filter = computeDataTableFilter('..t', true, [])!; + const filter = computeDataTableFilter('..t', true, baseColumns)!; expect(data.filter(filter)).toEqual([meet]); } { // regex, words with 6 chars - const filter = computeDataTableFilter('\\w{6}', true, [])!; + const filter = computeDataTableFilter('\\w{6}', true, baseColumns)!; expect(data.filter(filter)).toEqual([coffee, espresso]); } { // no match - const filter = computeDataTableFilter('\\w{18}', true, [])!; + const filter = computeDataTableFilter('\\w{18}', true, baseColumns)!; expect(data.filter(filter)).toEqual([]); } { // invalid regex - const filter = computeDataTableFilter('bla/[', true, [])!; + const filter = computeDataTableFilter('bla/[', true, baseColumns)!; expect(data.filter(filter)).toEqual([]); } { - const filter = computeDataTableFilter('true', false, [])!; + const filter = computeDataTableFilter('true', false, baseColumns)!; expect(data.filter(filter)).toEqual([coffee]); } { - const filter = computeDataTableFilter('false', false, [])!; + const filter = computeDataTableFilter('false', false, baseColumns)!; expect(data.filter(filter)).toEqual([espresso, meet]); } { const filter = computeDataTableFilter('EE', false, [ + levelCol, + titleCol, { key: 'level', filters: [ @@ -381,6 +404,8 @@ test('compute filters', () => { } { const filter = computeDataTableFilter('EE', false, [ + doneCol, + titleCol, { key: 'level', filters: [ @@ -401,6 +426,8 @@ test('compute filters', () => { } { const filter = computeDataTableFilter('', false, [ + doneCol, + titleCol, { key: 'level', filters: [ @@ -421,6 +448,8 @@ test('compute filters', () => { } { const filter = computeDataTableFilter('', false, [ + levelCol, + titleCol, { key: 'done', filters: [ @@ -437,6 +466,8 @@ test('compute filters', () => { { // nothing selected anything will not filter anything out for that column const filter = computeDataTableFilter('', false, [ + doneCol, + titleCol, { key: 'level', filters: [ @@ -455,8 +486,30 @@ test('compute filters', () => { ])!; expect(filter).toBeUndefined(); } + { + //nested filter on comment const filter = computeDataTableFilter('', false, [ + ...baseColumns, + { + key: 'extras.comment', + filters: [ + { + enabled: true, + value: 'dull', + label: 'dull', + }, + ], + }, + ])!; + expect(data.filter(filter)).toEqual([espresso]); + } + + { + //filter 'level' on values info and error which will match all records + const filter = computeDataTableFilter('', false, [ + doneCol, + titleCol, { key: 'level', filters: [ @@ -477,6 +530,7 @@ test('compute filters', () => { } { const filter = computeDataTableFilter('', false, [ + titleCol, { key: 'level', filters: [ @@ -503,6 +557,8 @@ test('compute filters', () => { { // inverse filter const filter = computeDataTableFilter('', false, [ + doneCol, + titleCol, { key: 'level', filters: [ @@ -520,6 +576,8 @@ test('compute filters', () => { { // inverse filter with search const filter = computeDataTableFilter('coffee', false, [ + doneCol, + titleCol, { key: 'level', filters: [ @@ -536,6 +594,7 @@ test('compute filters', () => { } { const filter = computeDataTableFilter('nonsense', false, [ + titleCol, { key: 'level', filters: [