From b4772a823a2c1d1a7419ff8006acf02001aed15b Mon Sep 17 00:00:00 2001 From: Jamie Rasmussen Date: Thu, 7 Nov 2024 17:30:58 -0600 Subject: [PATCH 1/2] feat(ui): object comparison --- .../PagePanelComponents/Home/Browse3.tsx | 10 + .../Home/Browse3/compare/CodeDiff.tsx | 94 +++++ .../Home/Browse3/compare/CompareGrid.tsx | 266 ++++++++++++ .../Home/Browse3/compare/CompareGridCell.tsx | 95 +++++ .../Browse3/compare/CompareGridCellValue.tsx | 70 +++ .../compare/CompareGridCellValueCode.tsx | 20 + .../compare/CompareGridCellValueTimestamp.tsx | 39 ++ .../compare/CompareGridGroupingCell.tsx | 130 ++++++ .../Home/Browse3/compare/CompareGridPill.tsx | 43 ++ .../Browse3/compare/CompareGridPillNumber.tsx | 69 +++ .../Home/Browse3/compare/ComparePage.tsx | 70 +++ .../Home/Browse3/compare/ComparePageCalls.tsx | 52 +++ .../Browse3/compare/ComparePageObjects.tsx | 39 ++ .../compare/ComparePageObjectsLoaded.tsx | 399 ++++++++++++++++++ .../Home/Browse3/compare/ShoppingCart.tsx | 150 +++++++ .../Home/Browse3/compare/ShoppingCartItem.tsx | 137 ++++++ .../Home/Browse3/compare/compare.ts | 140 ++++++ .../Home/Browse3/compare/hooks.ts | 121 ++++++ .../Home/Browse3/compare/refUtil.ts | 28 ++ .../Home/Browse3/compare/types.ts | 13 + .../Home/Browse3/context.tsx | 54 +++ .../Browse3/pages/CallPage/ObjectViewer.tsx | 8 +- .../CallPage/ValueViewNumberTimestamp.tsx | 19 +- .../Home/Browse3/pages/CallPage/traverse.ts | 2 +- .../Browse3/pages/CallsPage/CallsTable.tsx | 12 +- .../pages/CallsPage/CallsTableButtons.tsx | 26 +- .../Home/Browse3/pages/ObjectVersionsPage.tsx | 93 +++- .../Home/Browse3/urlQueryUtil.ts | 73 ++++ 28 files changed, 2254 insertions(+), 18 deletions(-) create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CodeDiff.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCell.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValue.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueCode.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueTimestamp.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridGroupingCell.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridPill.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridPillNumber.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePage.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageCalls.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjects.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/compare.ts create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/hooks.ts create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/refUtil.ts create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3.tsx index 405e26ccd99d..976e232716a4 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3.tsx @@ -47,6 +47,7 @@ import {Button} from '../../Button'; import {ErrorBoundary} from '../../ErrorBoundary'; import {Browse2EntityPage} from './Browse2/Browse2EntityPage'; import {Browse2HomePage} from './Browse2/Browse2HomePage'; +import {ComparePage} from './Browse3/compare/ComparePage'; import { baseContext, browse2Context, @@ -536,6 +537,9 @@ const Browse3ProjectRoot: FC<{ ]}> + + + ); @@ -1039,6 +1043,12 @@ const TablesPageBinding = () => { return ; }; +const ComparePageBinding = () => { + const params = useParamsDecoded(); + + return ; +}; + const AppBarLink = (props: ComponentProps) => ( { + if (uri.endsWith('.py')) { + return 'python'; + } + if (uri.endsWith('.js') || uri.endsWith('.ts')) { + return 'javascript'; + } + if (code.includes('def ') || code.includes('import ')) { + return 'python'; + } + if (code.includes('function ') || code.includes('const ')) { + return 'javascript'; + } + return 'plaintext'; +}; + +type CodeDiffProps = { + oldValueRef: string; + newValueRef: string; +}; + +export const CodeDiff = ({oldValueRef, newValueRef}: CodeDiffProps) => { + const { + derived: {useCodeForOpRef}, + } = useWFHooks(); + const opContentsQueryOld = useCodeForOpRef(oldValueRef); + const opContentsQueryNew = useCodeForOpRef(newValueRef); + const textOld = opContentsQueryOld.result ?? ''; + const textNew = opContentsQueryNew.result ?? ''; + const loading = opContentsQueryOld.loading || opContentsQueryNew.loading; + + if (loading) { + return ; + } + + const sanitizedOld = sanitizeString(textOld); + const sanitizedNew = sanitizeString(textNew); + const languageOld = detectLanguage(oldValueRef, sanitizedOld); + const languageNew = detectLanguage(newValueRef, sanitizedNew); + + const inner = + sanitizedOld !== sanitizedNew ? ( + + ) : ( + + ); + + const maxRowsInView = 20; + const totalLines = sanitizedNew.split('\n').length ?? 0; + const showLines = Math.min(totalLines, maxRowsInView); + const lineHeight = 18; + const padding = 20; + const height = showLines * lineHeight + padding + 'px'; + return {inner}; +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx new file mode 100644 index 000000000000..2e8b56d5e47d --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx @@ -0,0 +1,266 @@ +/** + * This is similar to the ObjectViewer but allows for multiple objects + * to be displayed side-by-side. + */ + +import { + DataGridProProps, + GRID_TREE_DATA_GROUPING_FIELD, + GridColDef, + GridPinnedColumnFields, + GridRowHeightParams, + GridRowId, + GridValidRowModel, + useGridApiRef, +} from '@mui/x-data-grid-pro'; +import React, {useCallback, useEffect, useMemo} from 'react'; + +import {WeaveObjectRef} from '../../../../../react'; +import {SmallRef} from '../../Browse2/SmallRef'; +import {ObjectVersionSchema} from '../pages/wfReactInterface/wfDataModelHooksInterface'; +import {StyledDataGrid} from '../StyledDataGrid'; +import {RowDataWithDiff, UNCHANGED} from './compare'; +import {CompareGridCell} from './CompareGridCell'; +import {CompareGridGroupingCell} from './CompareGridGroupingCell'; +import {ComparableObject, Mode} from './types'; + +type CompareGridProps = { + objectType: 'object' | 'call'; + objectIds: string[]; + objects: ComparableObject[]; + rows: RowDataWithDiff[]; + mode: Mode; + baselineEnabled: boolean; + onlyChanged: boolean; + isExpanded?: boolean; + + expandedIds: GridRowId[]; + setExpandedIds: React.Dispatch>; + addExpandedRefs: (path: string, refs: string[]) => void; +}; + +const objectVersionSchemaToRef = ( + objVersion: ObjectVersionSchema +): WeaveObjectRef => { + return { + scheme: 'weave', + entityName: objVersion.entity, + projectName: objVersion.project, + weaveKind: 'object', + artifactName: objVersion.objectId, + artifactVersion: objVersion.versionHash, + }; +}; + +export const CompareGrid = ({ + objectType, + objectIds, + objects, + rows, + mode, + baselineEnabled, + onlyChanged, + isExpanded, + expandedIds, + setExpandedIds, + addExpandedRefs, +}: CompareGridProps) => { + const apiRef = useGridApiRef(); + + const filteredRows = onlyChanged + ? rows.filter(row => row.changeType !== UNCHANGED) + : rows; + + const pinnedColumns: GridPinnedColumnFields = { + left: [ + GRID_TREE_DATA_GROUPING_FIELD, + ...(baselineEnabled ? [objectIds[0]] : []), + ], + }; + const columns: GridColDef[] = []; + if (mode === 'unified' && objectIds.length === 2) { + columns.push({ + field: 'value', + headerName: 'Value', + flex: 1, + display: 'flex', + sortable: false, + renderCell: cellParams => { + const objId = objectIds[1]; + const compareIdx = baselineEnabled + ? 0 + : Math.max(0, objectIds.indexOf(objId) - 1); + const compareId = objectIds[compareIdx]; + const compareValue = cellParams.row.values[compareId]; + const compareValueType = cellParams.row.types[compareId]; + const value = cellParams.row.values[objId]; + const valueType = cellParams.row.types[objId]; + const rowChangeType = cellParams.row.changeType; + return ( +
+ +
+ ); + }, + }); + } else { + const versionCols: GridColDef[] = objectIds.map(objId => ({ + field: objId, + headerName: objId, + flex: 1, + display: 'flex', + width: 500, + sortable: false, + valueGetter: (unused: any, row: any) => { + return row.values[objId]; + }, + renderHeader: (params: any) => { + if (objectType === 'call') { + // TODO: Make this a peek drawer link + return objId; + } + const idx = objectIds.indexOf(objId); + const objVersion = objects[idx]; + const objRef = objectVersionSchemaToRef( + objVersion as ObjectVersionSchema + ); + return ; + }, + renderCell: (cellParams: any) => { + const compareIdx = baselineEnabled + ? 0 + : Math.max(0, objectIds.indexOf(objId) - 1); + const compareId = objectIds[compareIdx]; + const compareValue = cellParams.row.values[compareId]; + const compareValueType = cellParams.row.types[compareId]; + const value = cellParams.row.values[objId]; + const valueType = cellParams.row.types[objId]; + const rowChangeType = cellParams.row.changeType; + return ( +
+ +
+ ); + }, + })); + columns.push(...versionCols); + } + + const groupingColDef: DataGridProProps['groupingColDef'] = useMemo( + () => ({ + field: '__group__', + hideDescendantCount: true, + width: 300, + renderHeader: () => { + // Keep padding in sync with + // INSET_SPACING (32) + left change indication border (3) - header padding (10) + return
Path
; + }, + renderCell: params => { + return ( + { + setExpandedIds(eIds => { + if (eIds.includes(params.row.id)) { + return eIds.filter(id => id !== params.row.id); + } + return [...eIds, params.row.id]; + }); + addExpandedRefs(params.row.id, params.row.expandableRefs); + }} + /> + ); + }, + }), + [addExpandedRefs, setExpandedIds] + ); + + const getRowId = (row: GridValidRowModel) => { + return row.path.toString(); + }; + + // Next we define a function that updates the row expansion state. This + // function is responsible for setting the expansion state of rows that have + // been expanded by the user. It is bound to the `rowsSet` event so that it is + // called whenever the rows are updated. The MUI data grid will rerender and + // close all expanded rows when the rows are updated. This function is + // responsible for re-expanding the rows that were previously expanded. + const updateRowExpand = useCallback(() => { + expandedIds.forEach(id => { + if (apiRef.current.getRow(id)) { + const children = apiRef.current.getRowGroupChildren({groupId: id}); + if (children.length !== 0) { + apiRef.current.setRowChildrenExpansion(id, true); + } + } + }); + }, [apiRef, expandedIds]); + useEffect(() => { + updateRowExpand(); + return apiRef.current.subscribeEvent('rowsSet', () => { + updateRowExpand(); + }); + }, [apiRef, expandedIds, updateRowExpand]); + + const getGroupIds = useCallback(() => { + const rowIds = apiRef.current.getAllRowIds(); + return rowIds.filter(rowId => { + const rowNode = apiRef.current.getRowNode(rowId); + return rowNode && rowNode.type === 'group'; + }); + }, [apiRef]); + + // On first render expand groups + useEffect(() => { + setExpandedIds(getGroupIds()); + }, [setExpandedIds, getGroupIds]); + + return ( + row.path.toStringArray()} + columns={columns} + rows={filteredRows} + isGroupExpandedByDefault={node => { + return expandedIds.includes(node.id); + }} + columnHeaderHeight={38} + disableColumnReorder={true} + disableColumnMenu={true} + getRowHeight={(params: GridRowHeightParams) => { + return 'auto'; + }} + rowSelection={false} + hideFooter + pinnedColumns={pinnedColumns} + keepBorders + sx={{ + '& .MuiDataGrid-cell': { + alignItems: 'flex-start', + padding: 0, + }, + }} + /> + ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCell.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCell.tsx new file mode 100644 index 000000000000..1a53882437fb --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCell.tsx @@ -0,0 +1,95 @@ +/** + * This handles the logic around the unified diff mode and + * showing either A -> B (delta) or some specific diff tool. + */ + +import _ from 'lodash'; +import React from 'react'; +import ReactDiffViewer, {DiffMethod} from 'react-diff-viewer'; + +import {ObjectPath} from '../pages/CallPage/traverse'; +import {CodeDiff} from './CodeDiff'; +import {CompareGridCellValue} from './CompareGridCellValue'; +import {CompareGridPill} from './CompareGridPill'; +import {RESOLVED_REF_KEY} from './refUtil'; + +const ARROW = '→'; + +type CompareGridCellProps = { + path: ObjectPath; + displayType: 'both' | 'diff'; + value: any; + valueType: any; + compareValue: any; + compareValueType: any; + rowChangeType: any; +}; + +export const CompareGridCell = ({ + path, + displayType, + value, + valueType, + compareValue, + compareValueType, + rowChangeType, +}: CompareGridCellProps) => { + if (valueType === 'array') { + return null; + } + + // If all of the row values are the same we can just display the value + if (rowChangeType === 'UNCHANGED' && _.isEqual(value, compareValue)) { + return ( + + ); + } + + if (valueType === 'object' && compareValueType === 'object') { + if (!(RESOLVED_REF_KEY in value) || !(RESOLVED_REF_KEY in compareValue)) { + return null; + } + } + + if (valueType === 'code' && compareValueType === 'code') { + return ; + } + + if (valueType === 'string' && compareValueType === 'string') { + // TODO: Need to figure out how to override font to be 'Source Sans Pro' + return ( +
+ +
+ ); + } + + return ( +
+ {displayType === 'both' && ( + <> + + {ARROW} + + )} + + +
+ ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValue.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValue.tsx new file mode 100644 index 000000000000..473f60cd8a13 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValue.tsx @@ -0,0 +1,70 @@ +/** + * This is similar to ValueView or CellValue - it dispatches + * to the appropriate component for rendering based on the value type. + */ +import React from 'react'; + +import {parseRef} from '../../../../../react'; +import {UserLink} from '../../../../UserLink'; +import {SmallRef} from '../../Browse2/SmallRef'; +import {ObjectPath} from '../pages/CallPage/traverse'; +import {ValueViewNumber} from '../pages/CallPage/ValueViewNumber'; +import { + isProbablyTimestampMs, + isProbablyTimestampSec, +} from '../pages/CallPage/ValueViewNumberTimestamp'; +import {ValueViewPrimitive} from '../pages/CallPage/ValueViewPrimitive'; +import {MISSING} from './compare'; +import {CompareGridCellValueCode} from './CompareGridCellValueCode'; +import {CompareGridCellValueTimestamp} from './CompareGridCellValueTimestamp'; +import {RESOLVED_REF_KEY} from './refUtil'; + +type CompareGridCellValueProps = { + path: ObjectPath; + value: any; + valueType: any; +}; + +export const CompareGridCellValue = ({ + path, + value, + valueType, +}: CompareGridCellValueProps) => { + if (value === MISSING) { + return missing; + } + if (value === undefined) { + return undefined; + } + if (value === null) { + return null; + } + if (path.toString() === 'wb_user_id') { + return ; + } + if (valueType === 'code') { + return ; + } + if (valueType === 'object') { + if (RESOLVED_REF_KEY in value) { + return ; + } + // We don't need to show anything for this row because user can expand it to compare child keys + return null; + } + if (valueType === 'array') { + return array; + } + + if (valueType === 'number') { + if (isProbablyTimestampSec(value)) { + return ; + } + if (isProbablyTimestampMs(value)) { + return ; + } + return ; + } + + return
{value}
; +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueCode.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueCode.tsx new file mode 100644 index 000000000000..b6cfea71f97a --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueCode.tsx @@ -0,0 +1,20 @@ +import React from 'react'; + +import {Browse2OpDefCode} from '../../Browse2/Browse2OpDefCode'; + +type CompareGridCellValueCodeProps = { + value: string; +}; + +export const CompareGridCellValueCode = ({ + value, +}: CompareGridCellValueCodeProps) => { + // Negative margin used to invert (mostly) the padding that we add to other cell types + // TODO: For better code layout/dependency management might be better to duplicate + // Browse2OpDefCode instead of referencing something in Browse2. + return ( +
+ +
+ ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueTimestamp.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueTimestamp.tsx new file mode 100644 index 000000000000..953af39c0957 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridCellValueTimestamp.tsx @@ -0,0 +1,39 @@ +/** + * When we have an integer that appears to be a timestamp, we display it as a + * nice date, but allow clicking to toggle between that and the raw value. + */ +import React, {useState} from 'react'; + +import {Timestamp} from '../../../../Timestamp'; +import {Tooltip} from '../../../../Tooltip'; + +type CompareGridCellValueTimestampProps = { + value: number; + unit: 'ms' | 's'; +}; + +export const CompareGridCellValueTimestamp = ({ + value, + unit, +}: CompareGridCellValueTimestampProps) => { + const [showRaw, setShowRaw] = useState(false); + + let body = null; + if (showRaw) { + body = ( + {value}} + content="Click to format as date" + /> + ); + } else { + const tsValue = unit === 'ms' ? value / 1000 : value; + body = ; + } + + return ( +
setShowRaw(!showRaw)}> + {body} +
+ ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridGroupingCell.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridGroupingCell.tsx new file mode 100644 index 000000000000..80e1bcc6f0a3 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGridGroupingCell.tsx @@ -0,0 +1,130 @@ +/** + * This is the component used to render the left-most "tree" part of the grid, + * handling indentation and expansion. + */ + +import {Box, BoxProps} from '@mui/material'; +import {GridRenderCellParams, useGridApiContext} from '@mui/x-data-grid-pro'; +import _ from 'lodash'; +import React, {FC, MouseEvent} from 'react'; + +import {GOLD_400, MOON_600} from '../../../../../common/css/color.styles'; +import {Button} from '../../../../Button'; +import {Tooltip} from '../../../../Tooltip'; +import {CursorBox} from '../pages/CallPage/CursorBox'; +import {UNCHANGED} from './compare'; + +const INSET_SPACING = 32; + +export const CompareGridGroupingCell: FC< + GridRenderCellParams & {onClick?: (event: MouseEvent) => void} +> = props => { + const {id, field, rowNode, row} = props; + const isGroup = rowNode.type === 'group'; + const hasExpandableRefs = row.expandableRefs.length > 0; + const isExpandable = (isGroup || hasExpandableRefs) && !row.isCode; + const apiRef = useGridApiContext(); + const onClick: BoxProps['onClick'] = event => { + if (isGroup) { + apiRef.current.setRowChildrenExpansion(id, !rowNode.childrenExpanded); + apiRef.current.setCellFocus(id, field); + } + + if (props.onClick) { + props.onClick(event); + } + + event.stopPropagation(); + }; + + const tooltipContent = row.path ? row.path.toString() : undefined; + const box = ( + + {_.range(rowNode.depth).map(i => { + return ( + + ); + })} + + {isExpandable ? ( + + + + )} +
+ + +
+
+ {baselineEnabled + ? 'Compare with baseline' + : 'Compare with previous'}{' '} + +
+
+ + + + onSetBaseline(false)}> + {!baselineEnabled ? ( + + ) : ( + + )}{' '} + Compare with previous + + onSetBaseline(true)}> + {baselineEnabled ? ( + + ) : ( + + )}{' '} + Compare with baseline + + + + +
+
+ +
+ + ), + }, + ]} + /> + ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx new file mode 100644 index 000000000000..8bb0f05785b7 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx @@ -0,0 +1,150 @@ +/** + * This name is an analogy - the collection of items that you have + * "put in your basket" for comparison. + */ + +import React from 'react'; +import {useHistory} from 'react-router-dom'; +import {SortableContainer} from 'react-sortable-hoc'; + +import {queryToggleString, searchParamsSetArray} from '../urlQueryUtil'; +import {ShoppingCartItem} from './ShoppingCartItem'; +import {ShoppingCartItemDefs} from './types'; + +type ShoppingCartItemsProps = { + items: ShoppingCartItemDefs; + baselineEnabled: boolean; + selected: string | null; +}; + +type SortableItemsProps = ShoppingCartItemsProps & { + onClickShoppingCartItem: (value: string) => void; + onSetBaseline: (value: string | null) => void; + onRemoveShoppingCartItem: (value: string) => void; +}; + +const SortableItems = SortableContainer( + ({ + items, + baselineEnabled, + selected, + onClickShoppingCartItem, + onSetBaseline, + onRemoveShoppingCartItem, + }: SortableItemsProps) => { + return ( +
+ {items.map((item, index) => ( + + ))} +
+ ); + } +); + +// Create a copy of the specified array, moving an item from one index to another. +function arrayMove(array: readonly T[], from: number, to: number) { + const slicedArray = array.slice(); + slicedArray.splice( + to < 0 ? array.length + to : to, + 0, + slicedArray.splice(from, 1)[0] + ); + return slicedArray; +} + +export const ShoppingCart = ({ + items, + baselineEnabled, + selected, +}: ShoppingCartItemsProps) => { + const history = useHistory(); + + const onSortEnd = ({ + oldIndex, + newIndex, + }: { + oldIndex: number; + newIndex: number; + }) => { + if (oldIndex === newIndex) { + return; + } + const {search} = history.location; + const params = new URLSearchParams(search); + const newShoppingCartItems = arrayMove(items, oldIndex, newIndex); + const values = newShoppingCartItems.map(i => i.value); + searchParamsSetArray(params, items[0].key, values); + if (newIndex === 0 && selected === newShoppingCartItems[0].value) { + params.delete('sel'); + } + history.replace({ + search: params.toString(), + }); + }; + + const onClickShoppingCartItem = (value: string) => { + queryToggleString(history, 'sel', value); + }; + + const onSetBaseline = (value: string | null) => { + const {search} = history.location; + const params = new URLSearchParams(search); + if (value === null) { + params.delete('baseline'); + } else { + let values = items.map(b => b.value); + values = arrayMove(values, values.indexOf(value), 0); + searchParamsSetArray(params, items[0].key, values); + params.set('baseline', '1'); + if (selected === value) { + params.delete('sel'); + } + } + history.replace({ + search: params.toString(), + }); + }; + + const onRemoveShoppingCartItem = (value: string) => { + const newShoppingCartItems = items.filter(b => b.value !== value); + const {search} = history.location; + const params = new URLSearchParams(search); + searchParamsSetArray( + params, + newShoppingCartItems[0].key, + newShoppingCartItems.map(b => b.value) + ); + if (selected === value || selected === newShoppingCartItems[0].value) { + params.delete('sel'); + } + history.replace({ + search: params.toString(), + }); + }; + + return ( + + ); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx new file mode 100644 index 000000000000..125b9ebd2711 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx @@ -0,0 +1,137 @@ +/** + * Draggable item representing one item being compared. + */ + +import * as DropdownMenu from '@wandb/weave/components/DropdownMenu'; +import classNames from 'classnames'; +import React, {useState} from 'react'; +import {SortableElement, SortableHandle} from 'react-sortable-hoc'; + +import {Button} from '../../../../Button'; +import {Icon} from '../../../../Icon'; +import {Pill} from '../../../../Tag'; +import {Tailwind} from '../../../../Tailwind'; +import {Tooltip} from '../../../../Tooltip'; +import {ShoppingCartItemDef} from './types'; + +// TODO: Change cursor to grabbing when dragging +const DragHandle = SortableHandle(() => ( +
+ +
+)); + +type ShoppingCartItemProps = { + numItems: number; + idx: number; + item: ShoppingCartItemDef; + baselineEnabled: boolean; + isSelected: boolean; + onClickShoppingCartItem: (value: string) => void; + onSetBaseline: (value: string | null) => void; + onRemoveShoppingCartItem: (value: string) => void; +}; + +export const ShoppingCartItem = SortableElement( + ({ + numItems, + idx, + item, + baselineEnabled, + isSelected, + onClickShoppingCartItem, + onSetBaseline, + onRemoveShoppingCartItem, + }: ShoppingCartItemProps) => { + const isSelectable = numItems > 2 && idx !== 0; + const isDeletable = numItems > 2; + const isBaseline = baselineEnabled && idx === 0; + const [isOpen, setIsOpen] = useState(false); + + const onClickItem = isSelectable + ? () => { + onClickShoppingCartItem(item.value); + } + : undefined; + + const onMakeBaseline = (e: React.MouseEvent) => { + e.stopPropagation(); + onSetBaseline(item.value); + }; + + const onRemoveItem = (e: React.MouseEvent) => { + e.stopPropagation(); + onRemoveShoppingCartItem(item.value); + }; + + const inner = ( +
+ +
+ {item.label ?? item.value} + {isBaseline && ( + + )} +
+ + +
+ ); + + if (!isSelectable) { + return {inner}; + } + + return ( + + + + ); + } +); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/compare.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/compare.ts new file mode 100644 index 000000000000..efbb18a5c54c --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/compare.ts @@ -0,0 +1,140 @@ +import _ from 'lodash'; + +import {ObjectPath, traverse, ValueType} from '../pages/CallPage/traverse'; +import {isExpandableRef} from '../pages/wfReactInterface/tsDataModelHooksCallRefExpansion'; +import {RESOLVED_REF_KEY} from './refUtil'; +import {ComparableObject} from './types'; + +// Row change types +export const UNCHANGED = 'UNCHANGED'; +export const DELETED = 'DELETED'; +export const ADDED = 'ADDED'; +export const CHANGED = 'CHANGED'; +export type RowChangeType = + | typeof UNCHANGED + | typeof DELETED + | typeof ADDED + | typeof CHANGED; + +export const MISSING = '__WEAVE_MISSING__'; +export const CODE = 'code'; +export type ColumnType = ValueType | typeof MISSING | typeof CODE; +export type ColumnId = string; + +export type RowData = { + // Row id is the string representation of path + id: string; + path: ObjectPath; + values: Record; + types: Record; // ColumnType + isCode?: boolean; +}; + +export type RowDataWithDiff = RowData & { + // Overall row change type, used for filtering rows and coloring grouping column + changeType: RowChangeType; + changeTypes: Record; + expandableRefs: string[]; +}; + +type PathString = string; +type PathInfo = { + path: ObjectPath; + values: Record; + types: Record; + isCode?: boolean; +}; + +export const mergeObjects = ( + columnIds: string[], + objects: ComparableObject[] +): RowData[] => { + const values: Record = {}; + + for (let i = 0; i < columnIds.length; i++) { + const columnId = columnIds[i]; + const object = objects[i]; + traverse(object, context => { + // Ops should be migrated to the generic CustomWeaveType pattern, but for + // now they are custom handled. + const isOpPayload = context.value?.weave_type?.type === 'Op'; + + if (context.path.tail() === RESOLVED_REF_KEY) { + return 'skip'; + } + const key = context.path.toString(); + if (!(key in values)) { + values[key] = { + path: context.path, + values: {}, + types: {}, + }; + } + values[key].values[columnId] = context.value; + values[key].types[columnId] = context.valueType; + + if (isOpPayload) { + const codeKey = key + '.code'; + if (!(codeKey in values)) { + values[codeKey] = { + isCode: true, + path: context.path.plus('code'), + values: {}, + types: {}, + }; + } + values[codeKey].values[columnId] = context.value._ref; + values[codeKey].types[columnId] = 'code'; + } + return isOpPayload ? 'skip' : true; + }); + } + + const rows: RowData[] = []; + for (const d of Object.values(values)) { + rows.push({ + id: d.path.toString(), + isCode: d.isCode, + path: d.path, + values: d.values, + types: d.types, + }); + } + + return rows; +}; + +export const computeDiff = ( + columnIds: ColumnId[], + rows: RowData[], + againstBaseline: boolean +): RowDataWithDiff[] => { + const nCols = columnIds.length; + const diffRows: RowDataWithDiff[] = []; + for (const row of rows) { + let rowChangeType: RowChangeType = UNCHANGED; + const changeTypes: Record = {}; + changeTypes[columnIds[0]] = UNCHANGED; + for (let i = 1; i < nCols; i++) { + const leftIdx = againstBaseline ? 0 : i - 1; + const rightName = columnIds[i]; + const left = row.values[columnIds[leftIdx]]; + const right = row.values[rightName]; + let changeType: RowChangeType = UNCHANGED; + // TODO: Handle added/deleted and missing + if (!_.isEqual(left, right)) { + changeType = CHANGED; + rowChangeType = CHANGED; + } + changeTypes[rightName] = changeType; + } + const rowWithDiff: RowDataWithDiff = { + ...row, + changeType: rowChangeType, + changeTypes, + expandableRefs: _.uniq(Object.values(row.values).filter(isExpandableRef)), + }; + diffRows.push(rowWithDiff); + } + return diffRows; +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/hooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/hooks.ts new file mode 100644 index 000000000000..cd7302fcd9e5 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/hooks.ts @@ -0,0 +1,121 @@ +import _ from 'lodash'; + +import {useWFHooks} from '../pages/wfReactInterface/context'; +import {ObjectVersionSchema} from '../pages/wfReactInterface/wfDataModelHooksInterface'; + +// Given a list of specifiers like ['obj1:v9', 'obj1:v10', 'obj2:v2'] +// return a list of unique object names like ['obj1', 'obj2'] +const getUniqueNames = (specifiers: string[]): string[] => { + const names = specifiers.map(spec => spec.split(':')[0]); + return [...new Set(names)]; +}; + +// Given a string like v23 return the number 23 +// Return null if can't extract a version number +const parseVersionNumber = (versionStr: string): number | null => { + if (versionStr.startsWith('v')) { + const num = parseInt(versionStr.slice(1), 10); + if (!isNaN(num) && num >= 0) { + return num; + } + } + return null; +}; + +export const parseSpecifier = ( + specifier: string +): {name: string; version: number | null; versionStr: string} => { + const parts = specifier.split(':', 2); + const name = parts[0]; + const versionStr = parts[1] ?? 'latest'; + const version = parseVersionNumber(versionStr); + return {name, version, versionStr}; +}; + +// Check if we have sequentially increasing version numbers of the same object +export const isSequentialVersions = (specifiers: string[]): boolean => { + if (specifiers.length < 2) { + return false; + } + const first = parseSpecifier(specifiers[0]); + if (first.version === null) { + return false; + } + for (let i = 1; i < specifiers.length; i++) { + const next = parseSpecifier(specifiers[i]); + if (next.name !== first.name || next.version !== first.version + i) { + return false; + } + } + return true; +}; + +// Find object version in array by specifier. +// specifiers can be in the forms: +// name, name:latest, name:v#, name:digest +const findObjectVersion = ( + objectVersions: ObjectVersionSchema[], + specifier: string +): ObjectVersionSchema | undefined => { + const {name, versionStr} = parseSpecifier(specifier); + if (versionStr === 'latest') { + const correctName = _.filter(objectVersions, {objectId: name}); + return _.maxBy(correctName, 'versionIndex'); + } + const versionIndex = parseVersionNumber(versionStr); + if (versionIndex !== null) { + return objectVersions.find( + v => v.objectId === name && v.versionIndex === versionIndex + ); + } + return objectVersions.find( + v => v.objectId === name && v.versionHash === versionStr + ); +}; + +type ObjectVersionsResult = { + loading: boolean; + objectVersions: ObjectVersionSchema[]; + lastVersionIndices: Record; // Name to number +}; + +export const useObjectVersions = ( + entity: string, + project: string, + objectVersionSpecifiers: string[] +): ObjectVersionsResult => { + // TODO: Need to introduce a backend query (/objs/read?) to bulk get specific versions of objects + const {useRootObjectVersions} = useWFHooks(); + const objectIds = getUniqueNames(objectVersionSpecifiers); + const rootObjectVersions = useRootObjectVersions( + entity, + project, + { + objectIds, + }, + undefined, // limit + // TODO: This is super wasteful - getting all the data for all versions of every object mentioned + // but hooks on array would be a pain, proper solution is new read API + false // metadataOnly + ); + if (rootObjectVersions.loading) { + return {loading: true, objectVersions: [], lastVersionIndices: {}}; + } + + const result = rootObjectVersions.result ?? []; + // TODO: Allow ommitting version specifier and handling other cases + // objname => latest version - 1, latest version + // obj1, obj2 => latest version for both + // For now, we require a version specifier for each + const objectVersions = objectVersionSpecifiers + .map(spec => findObjectVersion(result, spec)) + .filter((v): v is NonNullable => v !== undefined); + const lastVersionIndices: Record = {}; + for (const obj of result) { + const current = lastVersionIndices[obj.objectId] ?? -1; + if (obj.versionIndex > current) { + lastVersionIndices[obj.objectId] = obj.versionIndex; + } + } + return {loading: false, objectVersions, lastVersionIndices}; +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/refUtil.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/refUtil.ts new file mode 100644 index 000000000000..f5b712a6895d --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/refUtil.ts @@ -0,0 +1,28 @@ +import {isWeaveRef} from '../filters/common'; +import {traverse, TraverseContext} from '../pages/CallPage/traverse'; +import {isExpandableRef} from '../pages/wfReactInterface/tsDataModelHooksCallRefExpansion'; +import {ComparableObjects} from './types'; + +// When we replace a ref with its resolved data we insert this key +// with the ref URI to keep track of the original value. +export const RESOLVED_REF_KEY = '_ref'; + +export type RefValues = Record; // ref URI to value + +// Traverse the data and find all ref URIs. +export const getRefs = (objects: ComparableObjects): string[] => { + const refs = new Set(); + for (const obj of objects) { + traverse(obj, (context: TraverseContext) => { + if (isWeaveRef(context.value)) { + refs.add(context.value); + } + }); + } + return Array.from(refs); +}; + +// Get the refs in the objects that are expandable. +export const getExpandableRefs = (objects: ComparableObjects): string[] => { + return getRefs(objects).filter(isExpandableRef); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts new file mode 100644 index 000000000000..26e5e01ab993 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts @@ -0,0 +1,13 @@ +export type ShoppingCartItemDef = { + key: string; + value: string; + label?: string; +}; + +export type ShoppingCartItemDefs = ShoppingCartItemDef[]; + +export type ComparableObject = Record; +export type ComparableObjects = ComparableObject[]; + +// For two objects, whether to show side-by-side or unified +export type Mode = 'parallel' | 'unified'; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/context.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/context.tsx index 4aba7f882e57..fc522c0a9611 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/context.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/context.tsx @@ -205,6 +205,20 @@ export const browse2Context = { ) => { throw new Error('Not implemented'); }, + compareCallsUri: ( + entityName: string, + projectName: string, + callIds: string[] + ) => { + throw new Error('Not implemented'); + }, + compareObjectsUri: ( + entityName: string, + projectName: string, + objectSpecifiers: string[] + ) => { + throw new Error('Not implemented'); + }, }; export const browse3ContextGen = ( @@ -453,6 +467,26 @@ export const browse3ContextGen = ( leaderboardName ? `/${leaderboardName}` : '' }${edit ? '?edit=true' : ''}`; }, + compareCallsUri: ( + entityName: string, + projectName: string, + callIds: string[] + ) => { + const params = callIds + .map(id => 'call=' + encodeURIComponent(id)) + .join('&'); + return `${projectRoot(entityName, projectName)}/compare?${params}`; + }, + compareObjectsUri: ( + entityName: string, + projectName: string, + objectSpecifiers: string[] + ) => { + const params = objectSpecifiers + .map(id => 'obj=' + encodeURIComponent(id)) + .join('&'); + return `${projectRoot(entityName, projectName)}/compare?${params}`; + }, }; return browse3Context; }; @@ -547,6 +581,16 @@ type RouteType = { leaderboardName?: string, edit?: boolean ) => string; + compareCallsUri: ( + entityName: string, + projectName: string, + callIds: string[] + ) => string; + compareObjectsUri: ( + entityName: string, + projectName: string, + objectSpecifiers: string[] + ) => string; }; const useSetSearchParam = () => { @@ -669,6 +713,16 @@ const useMakePeekingRouter = (): RouteType => { ) => { return setSearchParam(PEEK_PARAM, baseContext.leaderboardsUIUrl(...args)); }, + compareCallsUri: ( + ...args: Parameters + ) => { + return setSearchParam(PEEK_PARAM, baseContext.compareCallsUri(...args)); + }, + compareObjectsUri: ( + ...args: Parameters + ) => { + return setSearchParam(PEEK_PARAM, baseContext.compareObjectsUri(...args)); + }, }; }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx index 0341688d4c9a..d741db647714 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx @@ -100,9 +100,11 @@ export const ObjectViewer = ({ const dataRefs = useMemo(() => getRefs(data).filter(isExpandableRef), [data]); // Expanded refs are the explicit set of refs that have been expanded by the user. Note that - // this might contain nested refs not in the `dataRefs` set. The keys are refs and the values - // are the paths at which the refs were expanded. - const [expandedRefs, setExpandedRefs] = useState<{[ref: string]: string}>({}); + // this might contain nested refs not in the `dataRefs` set. The keys are object paths at which the refs were expanded + // and the values are the corresponding ref string. + const [expandedRefs, setExpandedRefs] = useState<{[path: string]: string}>( + {} + ); // `addExpandedRef` is a function that can be used to add an expanded ref to the `expandedRefs` state. const addExpandedRef = useCallback((path: string, ref: string) => { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ValueViewNumberTimestamp.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ValueViewNumberTimestamp.tsx index ebacccda3917..baf5d8e8937f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ValueViewNumberTimestamp.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ValueViewNumberTimestamp.tsx @@ -8,16 +8,17 @@ const JAN_1_2000_MS = 1000 * JAN_1_2000_S; const JAN_1_2100_MS = 1000 * JAN_1_2100_S; // TODO: This is only looking at value, but we could also consider the value name, e.g. "created". + +export const isProbablyTimestampMs = (value: number) => { + return JAN_1_2000_MS <= value && value <= JAN_1_2100_MS; +}; + +export const isProbablyTimestampSec = (value: number) => { + return JAN_1_2000_S <= value && value <= JAN_1_2100_S; +}; + export const isProbablyTimestamp = (value: number) => { - const inRangeSec = JAN_1_2000_S <= value && value <= JAN_1_2100_S; - if (inRangeSec) { - return true; - } - const inRangeMs = JAN_1_2000_MS <= value && value <= JAN_1_2100_MS; - if (inRangeMs) { - return true; - } - return false; + return isProbablyTimestampMs(value) || isProbablyTimestampSec(value); }; type ValueViewNumberTimestampProps = { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/traverse.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/traverse.ts index 0f07c7b450b4..cdfdd6d7cdc3 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/traverse.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/traverse.ts @@ -155,7 +155,7 @@ export class ObjectPath { } } -type ValueType = +export type ValueType = | 'null' | 'undefined' | 'boolean' diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTable.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTable.tsx index 1b6a0ccbaa37..d7db3324ac98 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTable.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTable.tsx @@ -85,6 +85,7 @@ import {CallsCustomColumnMenu} from './CallsCustomColumnMenu'; import { BulkDeleteButton, CompareEvaluationsTableButton, + CompareTracesTableButton, ExportSelector, PaginationButtons, RefreshButton, @@ -860,7 +861,7 @@ export const CallsTable: FC<{ }} /> )} - {isEvaluateTable && ( + {isEvaluateTable ? ( { history.push( @@ -874,6 +875,15 @@ export const CallsTable: FC<{ }} disabled={selectedCalls.length === 0} /> + ) : ( + { + history.push( + router.compareCallsUri(entity, project, selectedCalls) + ); + }} + disabled={selectedCalls.length < 2} + /> )} {!isReadonly && selectedCalls.length !== 0 && ( <> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTableButtons.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTableButtons.tsx index 3718e696586b..fa8e9205dca9 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTableButtons.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/CallsTableButtons.tsx @@ -12,8 +12,7 @@ import { import {MOON_500} from '@wandb/weave/common/css/color.styles'; import {useOrgName} from '@wandb/weave/common/hooks/useOrganization'; import {useViewerUserInfo2} from '@wandb/weave/common/hooks/useViewerUserInfo'; -import {Radio} from '@wandb/weave/components'; -import {Switch} from '@wandb/weave/components'; +import {Radio, Switch} from '@wandb/weave/components'; import {Button} from '@wandb/weave/components/Button'; import {CodeEditor} from '@wandb/weave/components/CodeEditor'; import { @@ -424,6 +423,29 @@ export const CompareEvaluationsTableButton: FC<{ ); +export const CompareTracesTableButton: FC<{ + onClick: () => void; + disabled?: boolean; + tooltipText?: string; +}> = ({onClick, disabled, tooltipText}) => ( + + + +); + export const BulkDeleteButton: FC<{ disabled?: boolean; onClick: () => void; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx index 3c07c1ebd5b8..619cf7763fff 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx @@ -15,15 +15,21 @@ import { GridRowSelectionModel, GridRowsProp, } from '@mui/x-data-grid-pro'; +import {Checkbox} from '@wandb/weave/components/Checkbox'; import _ from 'lodash'; import React, {useEffect, useMemo, useState} from 'react'; +import {useHistory} from 'react-router-dom'; import {TEAL_600} from '../../../../../common/css/color.styles'; +import {Button} from '../../../../Button'; import {ErrorPanel} from '../../../../ErrorPanel'; import {Loading} from '../../../../Loading'; import {LoadingDots} from '../../../../LoadingDots'; import {Timestamp} from '../../../../Timestamp'; -import {useWeaveflowRouteContext} from '../context'; +import { + useWeaveflowCurrentRouteContext, + useWeaveflowRouteContext, +} from '../context'; import {StyledDataGrid} from '../StyledDataGrid'; import {basicField} from './common/DataTable'; import {Empty} from './common/Empty'; @@ -76,10 +82,17 @@ export const ObjectVersionsPage: React.FC<{ // is responsible for updating the filter. onFilterUpdate?: (filter: WFHighLevelObjectVersionFilter) => void; }> = props => { + const history = useHistory(); + const router = useWeaveflowCurrentRouteContext(); const [filter, setFilter] = useControllableState( props.initialFilter ?? {}, props.onFilterUpdate ); + const {entity, project} = props; + const [selectedVersions, setSelectedVersions] = useState([]); + const onCompare = () => { + history.push(router.compareObjectsUri(entity, project, selectedVersions)); + }; const title = useMemo(() => { if (filter.objectName) { @@ -90,10 +103,21 @@ export const ObjectVersionsPage: React.FC<{ return 'All Objects'; }, [filter.objectName, filter.baseObjectClass]); + const hasComparison = filter.objectName != null; + const headerExtra = hasComparison ? ( + + ) : undefined; + return ( ), }, @@ -126,6 +154,8 @@ export const FilterableObjectVersionsTable: React.FC<{ // Setting this will make the component a controlled component. The parent // is responsible for updating the filter. onFilterUpdate?: (filter: WFHighLevelObjectVersionFilter) => void; + selectedVersions?: string[]; + setSelectedVersions?: (selected: string[]) => void; }> = props => { const {useRootObjectVersions} = useWFHooks(); const {baseRouter} = useWeaveflowRouteContext(); @@ -199,6 +229,8 @@ export const FilterableObjectVersionsTable: React.FC<{ hidePeerVersionsColumn={!effectivelyLatestOnly} hideCategoryColumn={props.hideCategoryColumn} hideCreatedAtColumn={props.hideCreatedAtColumn} + selectedVersions={props.selectedVersions} + setSelectedVersions={props.setSelectedVersions} /> ); @@ -213,8 +245,11 @@ export const ObjectVersionsTable: React.FC<{ hideCreatedAtColumn?: boolean; hideVersionSuffix?: boolean; onRowClick?: (objectVersion: ObjectVersionSchema) => void; + selectedVersions?: string[]; + setSelectedVersions?: (selected: string[]) => void; }> = props => { // `showPropsAsColumns` probably needs to be a bit more robust + const {selectedVersions, setSelectedVersions} = props; const showPropsAsColumns = !props.hidePropsAsColumns; const rows: GridRowsProp = useMemo(() => { const vals = props.objectVersions.map(ov => ov.val); @@ -246,7 +281,61 @@ export const ObjectVersionsTable: React.FC<{ // extracted and shared. const {cols: columns, groups: columnGroupingModel} = useMemo(() => { let groups: GridColumnGroupingModel = []; + const checkboxColumnArr: GridColDef[] = + selectedVersions != null && setSelectedVersions + ? [ + { + minWidth: 30, + width: 34, + field: 'CustomCheckbox', + sortable: false, + disableColumnMenu: true, + resizable: false, + disableExport: true, + display: 'flex', + renderHeader: (params: any) => { + // TODO: Adding a select all checkbox here not that useful for compare + // but might for be for other bulk actions. + return null; + }, + renderCell: (params: any) => { + const {objectId, versionIndex} = params.row.obj; + const objSpecifier = `${objectId}:v${versionIndex}`; + const isSelected = selectedVersions.includes(objSpecifier); + return ( + { + if (isSelected) { + setSelectedVersions( + selectedVersions.filter(id => id !== objSpecifier) + ); + } else { + // Keep the objects in sorted order, regardless of the order checked. + setSelectedVersions( + [...selectedVersions, objSpecifier].sort((a, b) => { + const [aName, aVer] = a.split(':'); + const [bName, bVer] = b.split(':'); + if (aName !== bName) { + return aName.localeCompare(bName); + } + const aNum = parseInt(aVer.slice(1), 10); + const bNum = parseInt(bVer.slice(1), 10); + return aNum - bNum; + }) + ); + } + }} + /> + ); + }, + }, + ] + : []; const cols: GridColDef[] = [ + ...checkboxColumnArr, + // This field name chosen to reduce possibility of conflict // with the dynamic fields added below. basicField('weave__object_version_link', props.objectTitle ?? 'Object', { @@ -379,7 +468,7 @@ export const ObjectVersionsTable: React.FC<{ } return {cols, groups}; - }, [props, showPropsAsColumns, rows]); + }, [props, showPropsAsColumns, rows, selectedVersions, setSelectedVersions]); // Highlight table row if it matches peek drawer. const query = useURLSearchParamsDict(); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/urlQueryUtil.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/urlQueryUtil.ts index 949471f22339..4fe4b23a6d86 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/urlQueryUtil.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/urlQueryUtil.ts @@ -24,6 +24,24 @@ export function querySetString(history: History, key: string, value: string) { }); } +export const queryToggleString = ( + history: History, + key: string, + value: string +) => { + const currentValue = queryGetString(history, key); + const {search} = history.location; + const params = new URLSearchParams(search); + if (currentValue === value) { + params.delete(key); + } else { + params.set(key, value); + } + history.replace({ + search: params.toString(), + }); +}; + export function queryGetBoolean( history: History, key: string, @@ -68,3 +86,58 @@ export const queryToggleBoolean = ( search: params.toString(), }); }; + +export const querySetArray = (history: History, key: string, value: any[]) => { + const {search} = history.location; + const params = new URLSearchParams(search); + searchParamsSetArray(params, key, value); + history.replace({ + search: params.toString(), + }); +}; + +export const searchParamsSetArray = ( + params: URLSearchParams, + key: string, + value: any[] +) => { + params.delete(key); + value.forEach(item => { + params.append(key, String(item)); + }); +}; + +export const queryGetDict = (history: History): Record => { + const {search} = history.location; + const searchParams = new URLSearchParams(search); + const params: Record = {}; + searchParams.forEach((value, key) => { + if (params[key]) { + // If the key already exists, convert the existing value to an array (if it isn't already) + // and push the new value into the array + if (Array.isArray(params[key])) { + params[key].push(value); + } else { + params[key] = [params[key], value]; + } + } else { + // If the key doesn't exist, add it to the dictionary + params[key] = value; + } + }); + return params; +}; + +export const getParamArray = ( + d: Record, + key: string, + defaultValue: any[] = [] +): any[] => { + if (d[key] === undefined) { + return defaultValue; + } + if (Array.isArray(d[key])) { + return d[key]; + } + return [d[key]]; +}; From 33c27a273b3a62664e21409698370d8bd186a820 Mon Sep 17 00:00:00 2001 From: Jamie Rasmussen Date: Wed, 20 Nov 2024 14:26:40 -0600 Subject: [PATCH 2/2] CR feedback - Tim --- .../Home/Browse3/compare/CompareGrid.tsx | 100 +++++++++--------- .../compare/ComparePageObjectsLoaded.tsx | 83 ++++++++------- 2 files changed, 96 insertions(+), 87 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx index 2e8b56d5e47d..a9efcd2bf6d5 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx @@ -32,13 +32,14 @@ type CompareGridProps = { mode: Mode; baselineEnabled: boolean; onlyChanged: boolean; - isExpanded?: boolean; expandedIds: GridRowId[]; setExpandedIds: React.Dispatch>; addExpandedRefs: (path: string, refs: string[]) => void; }; +export const MAX_OBJECT_COLS = 6; + const objectVersionSchemaToRef = ( objVersion: ObjectVersionSchema ): WeaveObjectRef => { @@ -60,7 +61,6 @@ export const CompareGrid = ({ mode, baselineEnabled, onlyChanged, - isExpanded, expandedIds, setExpandedIds, addExpandedRefs, @@ -112,53 +112,55 @@ export const CompareGrid = ({ }, }); } else { - const versionCols: GridColDef[] = objectIds.map(objId => ({ - field: objId, - headerName: objId, - flex: 1, - display: 'flex', - width: 500, - sortable: false, - valueGetter: (unused: any, row: any) => { - return row.values[objId]; - }, - renderHeader: (params: any) => { - if (objectType === 'call') { - // TODO: Make this a peek drawer link - return objId; - } - const idx = objectIds.indexOf(objId); - const objVersion = objects[idx]; - const objRef = objectVersionSchemaToRef( - objVersion as ObjectVersionSchema - ); - return ; - }, - renderCell: (cellParams: any) => { - const compareIdx = baselineEnabled - ? 0 - : Math.max(0, objectIds.indexOf(objId) - 1); - const compareId = objectIds[compareIdx]; - const compareValue = cellParams.row.values[compareId]; - const compareValueType = cellParams.row.types[compareId]; - const value = cellParams.row.values[objId]; - const valueType = cellParams.row.types[objId]; - const rowChangeType = cellParams.row.changeType; - return ( -
- -
- ); - }, - })); + const versionCols: GridColDef[] = objectIds + .slice(0, MAX_OBJECT_COLS) + .map(objId => ({ + field: objId, + headerName: objId, + flex: 1, + display: 'flex', + width: 500, + sortable: false, + valueGetter: (unused: any, row: any) => { + return row.values[objId]; + }, + renderHeader: (params: any) => { + if (objectType === 'call') { + // TODO: Make this a peek drawer link + return objId; + } + const idx = objectIds.indexOf(objId); + const objVersion = objects[idx]; + const objRef = objectVersionSchemaToRef( + objVersion as ObjectVersionSchema + ); + return ; + }, + renderCell: (cellParams: any) => { + const compareIdx = baselineEnabled + ? 0 + : Math.max(0, objectIds.indexOf(objId) - 1); + const compareId = objectIds[compareIdx]; + const compareValue = cellParams.row.values[compareId]; + const compareValueType = cellParams.row.types[compareId]; + const value = cellParams.row.values[objId]; + const valueType = cellParams.row.types[objId]; + const rowChangeType = cellParams.row.changeType; + return ( +
+ +
+ ); + }, + })); columns.push(...versionCols); } diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx index 21d809df97ad..3b3548f09969 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx @@ -23,7 +23,7 @@ import { queryToggleBoolean, } from '../urlQueryUtil'; import {computeDiff, mergeObjects} from './compare'; -import {CompareGrid} from './CompareGrid'; +import {CompareGrid, MAX_OBJECT_COLS} from './CompareGrid'; import {isSequentialVersions, parseSpecifier} from './hooks'; import {getExpandableRefs, RefValues, RESOLVED_REF_KEY} from './refUtil'; import {ShoppingCart} from './ShoppingCart'; @@ -262,6 +262,8 @@ export const ComparePageObjectsLoaded = ({ }); }; + const tooManyCols = objectIds.length > MAX_OBJECT_COLS && !selected; + return ( )} )} @@ -338,43 +340,48 @@ export const ComparePageObjectsLoaded = ({ )} + {tooManyCols && ( +
Viewing first {MAX_OBJECT_COLS} columns only
+ )}
- - -
-
- {baselineEnabled - ? 'Compare with baseline' - : 'Compare with previous'}{' '} - + {objectIds.length > 2 && ( + + +
+
+ {baselineEnabled + ? 'Compare with baseline' + : 'Compare with previous'}{' '} + +
-
- - - - onSetBaseline(false)}> - {!baselineEnabled ? ( - - ) : ( - - )}{' '} - Compare with previous - - onSetBaseline(true)}> - {baselineEnabled ? ( - - ) : ( - - )}{' '} - Compare with baseline - - - - + + + + onSetBaseline(false)}> + {!baselineEnabled ? ( + + ) : ( + + )}{' '} + Compare with previous + + onSetBaseline(true)}> + {baselineEnabled ? ( + + ) : ( + + )}{' '} + Compare with baseline + + + + + )}