From 22ec709bb6d233dd84c36ef45152871bcbf99df3 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 8 Oct 2024 11:31:23 -0700 Subject: [PATCH 01/14] merge griffin/eval-no-model-name --- .../pages/CallsPage/callsTableQuery.ts | 5 +- .../CompareEvaluationsPage.tsx | 6 +- .../compareEvaluationsContext.tsx | 18 +- .../ComparisonDefinitionSection.tsx | 217 ++++++++++++++++-- .../EvaluationDefinition.tsx | 57 ++++- 5 files changed, 279 insertions(+), 24 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/callsTableQuery.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/callsTableQuery.ts index 87a5ec9423a1..b80a9905f9d0 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/callsTableQuery.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallsPage/callsTableQuery.ts @@ -33,7 +33,8 @@ export const useCallsForQuery = ( gridFilter: GridFilterModel, gridSort: GridSortModel, gridPage: GridPaginationModel, - expandedColumns: Set + expandedColumns: Set, + columns?: string[] ): { result: CallSchema[]; loading: boolean; @@ -57,7 +58,7 @@ export const useCallsForQuery = ( offset, sortBy, filterBy, - undefined, + columns, expandedColumns, { refetchOnDelete: true, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx index a269463ff4e2..05da3ba49ed0 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx @@ -95,7 +95,9 @@ export const CompareEvaluationsPageContent: React.FC< ); React.useEffect(() => { - if (props.evaluationCallIds.length > 0) { + // Only update the baseline if we are switching evaluations, if there + // is more than 1, we are in the compare view and baseline is auto set + if (props.evaluationCallIds.length === 1) { setBaselineEvaluationCallId(props.evaluationCallIds[0]); } }, [props.evaluationCallIds]); @@ -108,7 +110,7 @@ export const CompareEvaluationsPageContent: React.FC< >; setSelectedInputDigest: React.Dispatch>; + addEvaluationCall: (newCallId: string) => void; + removeEvaluationCall: (callId: string) => void; } | null>(null); export const useCompareEvaluationsState = () => { @@ -29,7 +31,7 @@ export const useCompareEvaluationsState = () => { export const CompareEvaluationsProvider: React.FC<{ entity: string; project: string; - evaluationCallIds: string[]; + initialEvaluationCallIds: string[]; setBaselineEvaluationCallId: React.Dispatch< React.SetStateAction >; @@ -43,7 +45,7 @@ export const CompareEvaluationsProvider: React.FC<{ }> = ({ entity, project, - evaluationCallIds, + initialEvaluationCallIds, setBaselineEvaluationCallId, setComparisonDimensions, @@ -54,6 +56,9 @@ export const CompareEvaluationsProvider: React.FC<{ selectedInputDigest, children, }) => { + const [evaluationCallIds, setEvaluationCallIds] = useState( + initialEvaluationCallIds + ); const initialState = useEvaluationComparisonState( entity, project, @@ -72,10 +77,17 @@ export const CompareEvaluationsProvider: React.FC<{ setBaselineEvaluationCallId, setComparisonDimensions, setSelectedInputDigest, + addEvaluationCall: (newCallId: string) => { + setEvaluationCallIds(prev => [...prev, newCallId]); + }, + removeEvaluationCall: (callId: string) => { + setEvaluationCallIds(prev => prev.filter(id => id !== callId)); + }, }; }, [ initialState.loading, initialState.result, + setEvaluationCallIds, setBaselineEvaluationCallId, setComparisonDimensions, setSelectedInputDigest, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index a7fb35f1746d..3d461681a3ca 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -1,12 +1,28 @@ -import React, {useMemo} from 'react'; +import {Popover} from '@mui/material'; +import Input from '@wandb/weave/common/components/Input'; +import {Tailwind} from '@wandb/weave/components/Tailwind'; +import {parseRef, WeaveObjectRef} from '@wandb/weave/react'; +import React, {useEffect, useMemo, useRef, useState} from 'react'; import {Button} from '../../../../../../../Button'; +import { + DEFAULT_FILTER_CALLS, + DEFAULT_SORT_CALLS, +} from '../../../CallsPage/CallsTable'; +import {useCallsForQuery} from '../../../CallsPage/callsTableQuery'; +import {useEvaluationsFilter} from '../../../CallsPage/evaluationsFilter'; +import {Id} from '../../../common/Id'; +import {useWFHooks} from '../../../wfReactInterface/context'; +import { + CallSchema, + ObjectVersionKey, +} from '../../../wfReactInterface/wfDataModelHooksInterface'; import {useCompareEvaluationsState} from '../../compareEvaluationsContext'; import {STANDARD_PADDING} from '../../ecpConstants'; import {getOrderedCallIds} from '../../ecpState'; import {EvaluationComparisonState} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; -import {EvaluationDefinition} from './EvaluationDefinition'; +import {EvaluationDefinition, VerticalBar} from './EvaluationDefinition'; export const ComparisonDefinitionSection: React.FC<{ state: EvaluationComparisonState; @@ -28,25 +44,200 @@ export const ComparisonDefinitionSection: React.FC<{ {evalCallIds.map((key, ndx) => { return ( - {ndx !== 0 && } ); })} + ); }; -const SwapPositionsButton: React.FC<{callId: string}> = props => { - const {setBaselineEvaluationCallId} = useCompareEvaluationsState(); +const ModelRefLabel: React.FC<{modelRef: string}> = props => { + const {useObjectVersion} = useWFHooks(); + const objRef = useMemo( + () => parseRef(props.modelRef) as WeaveObjectRef, + [props.modelRef] + ); + const objVersionKey = useMemo(() => { + return { + scheme: 'weave', + entity: objRef.entityName, + project: objRef.projectName, + weaveKind: objRef.weaveKind, + objectId: objRef.artifactName, + versionHash: objRef.artifactVersion, + path: '', + refExtra: objRef.artifactRefExtra, + } as ObjectVersionKey; + }, [ + objRef.artifactName, + objRef.artifactRefExtra, + objRef.artifactVersion, + objRef.entityName, + objRef.projectName, + objRef.weaveKind, + ]); + const objectVersion = useObjectVersion(objVersionKey); + return ( + + {objectVersion.result?.objectId}:{objectVersion.result?.versionIndex} + + ); +}; + +const AddEvaluationButton: React.FC<{ + state: EvaluationComparisonState; +}> = props => { + const {addEvaluationCall} = useCompareEvaluationsState(); + + // Calls query for just evaluations + const evaluationsFilter = useEvaluationsFilter( + props.state.data.entity, + props.state.data.project + ); + const page = useMemo( + () => ({ + pageSize: 100, + page: 0, + }), + [] + ); + const expandedRefCols = useMemo(() => new Set(), []); + // Don't query for output here, re-queried in tsDataModelHooksEvaluationComparison.ts + const columns = useMemo(() => ['inputs'], []); + const calls = useCallsForQuery( + props.state.data.entity, + props.state.data.project, + evaluationsFilter, + DEFAULT_FILTER_CALLS, + DEFAULT_SORT_CALLS, + page, + expandedRefCols, + columns + ); + + const evalsNotComparing = useMemo(() => { + return calls.result.filter( + call => + !Object.keys(props.state.data.evaluationCalls).includes(call.callId) + ); + }, [calls.result, props.state.data.evaluationCalls]); + + const [menuOptions, setMenuOptions] = + useState(evalsNotComparing); + useEffect(() => { + setMenuOptions(evalsNotComparing); + }, [evalsNotComparing]); + + const onSearchChange = (e: React.ChangeEvent) => { + const search = e.target.value; + if (search === '') { + setMenuOptions(evalsNotComparing); + return; + } + + const filteredOptions = calls.result.filter(call => { + if ( + (call.displayName ?? call.spanName) + .toLowerCase() + .includes(search.toLowerCase()) + ) { + return true; + } + if (call.callId.slice(-4).includes(search)) { + return true; + } + const modelRef = parseRef(call.traceCall?.inputs.model) as WeaveObjectRef; + if (modelRef.artifactName.toLowerCase().includes(search.toLowerCase())) { + return true; + } + return false; + }); + + setMenuOptions(filteredOptions); + }; + + // Popover management + const refBar = useRef(null); + const refLabel = useRef(null); + const [anchorEl, setAnchorEl] = React.useState(null); + const onClick = (event: React.MouseEvent) => { + setAnchorEl(anchorEl ? null : refBar.current); + }; + const open = Boolean(anchorEl); + const id = open ? 'simple-popper' : undefined; + return ( - + + + setAnchorEl(null)}> + +
+ +
+ {menuOptions.length === 0 && ( +
No evaluations
+ )} + {menuOptions.map(call => ( +
+ +
+ ))} +
+
+ + + ); }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 576b17f58228..7fdb0c6c9482 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -1,5 +1,8 @@ import {Box} from '@material-ui/core'; import {Circle} from '@mui/icons-material'; +import {PopupDropdown} from '@wandb/weave/common/components/PopupDropdown'; +import {Button} from '@wandb/weave/components/Button'; +import {Pill} from '@wandb/weave/components/Tag'; import React, {useMemo} from 'react'; import { @@ -14,6 +17,7 @@ import {SmallRef} from '../../../../../Browse2/SmallRef'; import {CallLink, ObjectVersionLink} from '../../../common/Links'; import {useWFHooks} from '../../../wfReactInterface/context'; import {ObjectVersionKey} from '../../../wfReactInterface/wfDataModelHooksInterface'; +import {useCompareEvaluationsState} from '../../compareEvaluationsContext'; import { BOX_RADIUS, CIRCLE_SIZE, @@ -27,6 +31,36 @@ export const EvaluationDefinition: React.FC<{ state: EvaluationComparisonState; callId: string; }> = props => { + const {removeEvaluationCall, setBaselineEvaluationCallId} = + useCompareEvaluationsState(); + + const menuOptions = useMemo(() => { + return [ + { + key: 'add-to-baseline', + content: 'Set as baseline', + onClick: () => { + setBaselineEvaluationCallId(props.callId); + }, + disabled: props.callId === props.state.baselineEvaluationCallId, + }, + { + key: 'remove', + content: 'Remove', + onClick: () => { + removeEvaluationCall(props.callId); + }, + disabled: Object.keys(props.state.data.evaluationCalls).length === 1, + }, + ]; + }, [ + props.callId, + props.state.baselineEvaluationCallId, + props.state.data.evaluationCalls, + removeEvaluationCall, + setBaselineEvaluationCallId, + ]); + return ( - - + {props.callId === props.state.baselineEvaluationCallId && ( + + )} + + } + /> ); }; + export const EvaluationCallLink: React.FC<{ callId: string; state: EvaluationComparisonState; @@ -147,11 +195,12 @@ const ModelIcon: React.FC = () => { ); }; -const VerticalBar: React.FC = () => { + +export const VerticalBar: React.FC = () => { return (
Date: Tue, 8 Oct 2024 16:03:37 -0700 Subject: [PATCH 02/14] working ish, drop indicators not working --- .../CompareEvaluationsPage.tsx | 17 +- .../compareEvaluationsContext.tsx | 25 +- .../pages/CompareEvaluationsPage/ecpState.ts | 26 +- .../ComparisonDefinitionSection.tsx | 103 ++++-- .../EvaluationDefinition.tsx | 55 +++- .../ComparisonDefinitionSection/dragUtils.tsx | 293 ++++++++++++++++++ .../ExampleCompareSection.tsx | 3 +- .../ExampleFilterSection.tsx | 4 +- .../ScorecardSection/ScorecardSection.tsx | 8 +- 9 files changed, 456 insertions(+), 78 deletions(-) create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx index 05da3ba49ed0..3e29302d9e69 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx @@ -67,8 +67,9 @@ export const CompareEvaluationsPage: React.FC< export const CompareEvaluationsPageContent: React.FC< CompareEvaluationsPageProps > = props => { - const [baselineEvaluationCallId, setBaselineEvaluationCallId] = - React.useState(null); + const [selectedCallIdsOrdered, setSelectedCallIdsOrdered] = React.useState< + string[] + >([]); const [comparisonDimensions, setComparisonDimensions] = React.useState(null); @@ -95,12 +96,8 @@ export const CompareEvaluationsPageContent: React.FC< ); React.useEffect(() => { - // Only update the baseline if we are switching evaluations, if there - // is more than 1, we are in the compare view and baseline is auto set - if (props.evaluationCallIds.length === 1) { - setBaselineEvaluationCallId(props.evaluationCallIds[0]); - } - }, [props.evaluationCallIds]); + setSelectedCallIdsOrdered(props.evaluationCallIds); + }, [props.evaluationCallIds, setSelectedCallIdsOrdered]); if (props.evaluationCallIds.length === 0) { return
No evaluations to compare
; @@ -111,9 +108,9 @@ export const CompareEvaluationsPageContent: React.FC< entity={props.entity} project={props.project} initialEvaluationCallIds={props.evaluationCallIds} - baselineEvaluationCallId={baselineEvaluationCallId ?? undefined} + selectedCallIdsOrdered={selectedCallIdsOrdered} + setSelectedCallIdsOrdered={setSelectedCallIdsOrdered} comparisonDimensions={comparisonDimensions ?? undefined} - setBaselineEvaluationCallId={setBaselineEvaluationCallId} setComparisonDimensions={setComparisonDimensionsAndClearInputDigest} selectedInputDigest={selectedInputDigest ?? undefined} setSelectedInputDigest={setSelectedInputDigest}> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx index aa26f7ca5160..ec94b6b4d7fe 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx @@ -9,9 +9,7 @@ import {ComparisonDimensionsType} from './ecpState'; const CompareEvaluationsContext = React.createContext<{ state: EvaluationComparisonState; - setBaselineEvaluationCallId: React.Dispatch< - React.SetStateAction - >; + setSelectedCallIdsOrdered: React.Dispatch>; setComparisonDimensions: React.Dispatch< React.SetStateAction >; @@ -32,26 +30,23 @@ export const CompareEvaluationsProvider: React.FC<{ entity: string; project: string; initialEvaluationCallIds: string[]; - setBaselineEvaluationCallId: React.Dispatch< - React.SetStateAction - >; + selectedCallIdsOrdered: string[]; + setSelectedCallIdsOrdered: React.Dispatch>; setComparisonDimensions: React.Dispatch< React.SetStateAction >; setSelectedInputDigest: React.Dispatch>; - baselineEvaluationCallId?: string; comparisonDimensions?: ComparisonDimensionsType; selectedInputDigest?: string; }> = ({ entity, project, initialEvaluationCallIds, - setBaselineEvaluationCallId, + setSelectedCallIdsOrdered, + selectedCallIdsOrdered, setComparisonDimensions, - setSelectedInputDigest, - baselineEvaluationCallId, comparisonDimensions, selectedInputDigest, children, @@ -63,7 +58,7 @@ export const CompareEvaluationsProvider: React.FC<{ entity, project, evaluationCallIds, - baselineEvaluationCallId, + selectedCallIdsOrdered, comparisonDimensions, selectedInputDigest ); @@ -74,21 +69,25 @@ export const CompareEvaluationsProvider: React.FC<{ } return { state: initialState.result, - setBaselineEvaluationCallId, + setSelectedCallIdsOrdered, setComparisonDimensions, setSelectedInputDigest, addEvaluationCall: (newCallId: string) => { setEvaluationCallIds(prev => [...prev, newCallId]); + setSelectedCallIdsOrdered(prev => [...(prev ?? []), newCallId]); }, removeEvaluationCall: (callId: string) => { setEvaluationCallIds(prev => prev.filter(id => id !== callId)); + setSelectedCallIdsOrdered( + prev => prev?.filter(id => id !== callId) ?? null + ); }, }; }, [ initialState.loading, initialState.result, setEvaluationCallIds, - setBaselineEvaluationCallId, + setSelectedCallIdsOrdered, setComparisonDimensions, setSelectedInputDigest, ]); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts index ea28cc417e89..785f647ed3e0 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts @@ -18,12 +18,12 @@ import {getMetricIds} from './ecpUtil'; export type EvaluationComparisonState = { // The normalized data for the evaluations data: EvaluationComparisonData; - // The evaluation call id of the baseline model - baselineEvaluationCallId: string; // The dimensions to compare & filter results comparisonDimensions?: ComparisonDimensionsType; // The current digest which is in view selectedInputDigest?: string; + // The order of the evaluation calls, the first call is always the baseline + selectedCallIdsOrdered: string[]; }; export type ComparisonDimensionsType = Array<{ @@ -41,7 +41,7 @@ export const useEvaluationComparisonState = ( entity: string, project: string, evaluationCallIds: string[], - baselineEvaluationCallId?: string, + selectedCallIdsOrdered: string[], comparisonDimensions?: ComparisonDimensionsType, selectedInputDigest?: string ): Loadable => { @@ -89,17 +89,15 @@ export const useEvaluationComparisonState = ( loading: false, result: { data: data.result, - baselineEvaluationCallId: - baselineEvaluationCallId ?? evaluationCallIds[0], comparisonDimensions: newComparisonDimensions, selectedInputDigest, + selectedCallIdsOrdered, }, }; }, [ data.result, data.loading, - baselineEvaluationCallId, - evaluationCallIds, + selectedCallIdsOrdered, comparisonDimensions, selectedInputDigest, ]); @@ -112,17 +110,21 @@ export const useEvaluationComparisonState = ( * evaluation call is first. */ export const getOrderedCallIds = (state: EvaluationComparisonState) => { - const initial = Object.keys(state.data.evaluationCalls); - moveItemToFront(initial, state.baselineEvaluationCallId); - return initial; + return state.selectedCallIdsOrdered.filter( + callId => state.data.evaluationCalls[callId] != null + ); +}; + +export const getBaselineCallId = (state: EvaluationComparisonState) => { + return state.selectedCallIdsOrdered[0]; }; /** * Should use this over keys of `state.data.models` because it ensures the baseline model is first. */ export const getOrderedModelRefs = (state: EvaluationComparisonState) => { - const baselineRef = - state.data.evaluationCalls[state.baselineEvaluationCallId].modelRef; + const baselineCallId = getBaselineCallId(state); + const baselineRef = state.data.evaluationCalls[baselineCallId].modelRef; const refs = Object.keys(state.data.models); // Make sure the baseline model is first moveItemToFront(refs, baselineRef); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 3d461681a3ca..bbec4ac3eaf0 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -1,8 +1,13 @@ import {Popover} from '@mui/material'; import Input from '@wandb/weave/common/components/Input'; +import { + DragDropProvider, + DragSource, + DropTarget, +} from '@wandb/weave/common/containers/DragDropContainer'; import {Tailwind} from '@wandb/weave/components/Tailwind'; import {parseRef, WeaveObjectRef} from '@wandb/weave/react'; -import React, {useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {Button} from '../../../../../../../Button'; import { @@ -19,37 +24,80 @@ import { } from '../../../wfReactInterface/wfDataModelHooksInterface'; import {useCompareEvaluationsState} from '../../compareEvaluationsContext'; import {STANDARD_PADDING} from '../../ecpConstants'; -import {getOrderedCallIds} from '../../ecpState'; -import {EvaluationComparisonState} from '../../ecpState'; +import {EvaluationComparisonState, getOrderedCallIds} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; +import {useDragDropReorder} from './dragUtils'; import {EvaluationDefinition, VerticalBar} from './EvaluationDefinition'; export const ComparisonDefinitionSection: React.FC<{ state: EvaluationComparisonState; }> = props => { - const evalCallIds = useMemo( - () => getOrderedCallIds(props.state), - [props.state] + const {setSelectedCallIdsOrdered} = useCompareEvaluationsState(); + + const reorderItems = useCallback( + (fromIndex: number, toIndex: number) => { + setSelectedCallIdsOrdered(prev => { + if (prev == null) { + return prev; + } + const newOrder = [...prev]; + const from = newOrder[fromIndex]; + newOrder[fromIndex] = newOrder[toIndex]; + newOrder[toIndex] = from; + return newOrder; + }); + }, + [setSelectedCallIdsOrdered] ); + const { + makeDragSourceCallbackRef, + renderDropIndicators, + onDragOver, + onDragEnd, + onDrop, + } = useDragDropReorder({ + reorder: reorderItems, + dropzonePadding: 8, + }); + + const callIds = getOrderedCallIds(props.state); + return ( - - {evalCallIds.map((key, ndx) => { - return ( - - - - ); - })} - - + + + + {callIds.map((key, ndx) => { + return ( +
+ {renderDropIndicators(ndx)} + + + +
+ ); + })} + + + + ); }; @@ -119,10 +167,9 @@ const AddEvaluationButton: React.FC<{ const evalsNotComparing = useMemo(() => { return calls.result.filter( - call => - !Object.keys(props.state.data.evaluationCalls).includes(call.callId) + call => !props.state.selectedCallIdsOrdered.includes(call.callId) ); - }, [calls.result, props.state.data.evaluationCalls]); + }, [calls.result, props.state.selectedCallIdsOrdered]); const [menuOptions, setMenuOptions] = useState(evalsNotComparing); @@ -222,9 +269,7 @@ const AddEvaluationButton: React.FC<{ variant="ghost" size="small" className="pb-8 pt-8 font-['Source_Sans_Pro'] text-base font-normal text-moon-800" - onClick={() => { - addEvaluationCall(call.callId); - }}> + onClick={() => addEvaluationCall(call.callId)}> <> {call.displayName ?? call.spanName} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 7fdb0c6c9482..3de0beb01ddb 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -1,9 +1,12 @@ import {Box} from '@material-ui/core'; import {Circle} from '@mui/icons-material'; +import {WBIcon} from '@wandb/ui'; import {PopupDropdown} from '@wandb/weave/common/components/PopupDropdown'; +import {DragHandle} from '@wandb/weave/common/containers/DragDropContainer/DragHandle'; import {Button} from '@wandb/weave/components/Button'; import {Pill} from '@wandb/weave/components/Tag'; import React, {useMemo} from 'react'; +import styled from 'styled-components'; import { MOON_300, @@ -24,14 +27,31 @@ import { EVAL_DEF_HEIGHT, STANDARD_BORDER, } from '../../ecpConstants'; -import {EvaluationComparisonState} from '../../ecpState'; +import {EvaluationComparisonState, getBaselineCallId} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; +const DragHandleIcon = styled(WBIcon).attrs({name: 'vertical-handle'})` + font-size: calc(30 * 1.75); + border-radius: 50%; + color: gray300; + user-select: none; + cursor: grab; + &&&:hover { + background: gray100; + color: black; + } + &:active { + cursor: grabbing; + } +`; +DragHandle.displayName = 'S.DragHandle'; + export const EvaluationDefinition: React.FC<{ state: EvaluationComparisonState; callId: string; + partRef: {id: string}; }> = props => { - const {removeEvaluationCall, setBaselineEvaluationCallId} = + const {removeEvaluationCall, setSelectedCallIdsOrdered} = useCompareEvaluationsState(); const menuOptions = useMemo(() => { @@ -40,9 +60,21 @@ export const EvaluationDefinition: React.FC<{ key: 'add-to-baseline', content: 'Set as baseline', onClick: () => { - setBaselineEvaluationCallId(props.callId); + setSelectedCallIdsOrdered(prev => { + if (prev == null) { + return prev; + } + const index = prev.indexOf(props.callId); + if (index === 0) { + return prev; + } + const newOrder = [...prev]; + newOrder.splice(index, 1); + newOrder.unshift(props.callId); + return newOrder; + }); }, - disabled: props.callId === props.state.baselineEvaluationCallId, + disabled: props.callId === getBaselineCallId(props.state), }, { key: 'remove', @@ -55,10 +87,9 @@ export const EvaluationDefinition: React.FC<{ ]; }, [ props.callId, - props.state.baselineEvaluationCallId, - props.state.data.evaluationCalls, + props.state, removeEvaluationCall, - setBaselineEvaluationCallId, + setSelectedCallIdsOrdered, ]); return ( @@ -71,8 +102,11 @@ export const EvaluationDefinition: React.FC<{ alignItems: 'center', justifyContent: 'space-between', }}> + + + - {props.callId === props.state.baselineEvaluationCallId && ( + {props.callId === getBaselineCallId(props.state) && ( )} = props => { - const evaluationCall = props.state.data.evaluationCalls[props.callId]; + const evaluationCall = props.state.data.evaluationCalls?.[props.callId]; + if (!evaluationCall) { + return null; + } const {entity, project} = props.state.data; return ( diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx new file mode 100644 index 000000000000..df2f9355a8ab --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx @@ -0,0 +1,293 @@ +/***** COPIED FROM app/src/util/dragDrop.tsx *****/ + +import { + DragDropState, + DragSourceProps, + DropTargetProps, +} from '@wandb/weave/common/containers/DragDropContainer'; +import * as globals from '@wandb/weave/common/css/globals.styles'; +import _ from 'lodash'; +import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import styled from 'styled-components'; + +export type ReorderFn = (fromIndex: number, toIndex: number) => void; + +type DropIndicatorPosition = `top` | `bottom` | `left` | `right`; + +type DropIndicatorMode = `vertical` | `horizontal`; + +type DropzonePadding = {[P in DropIndicatorPosition]: number}; + +const ZERO_DROPZONE_PADDING: DropzonePadding = { + top: 0, + bottom: 0, + left: 0, + right: 0, +}; + +const DROPZONE_PADDING_BUFFER_PX = 2; + +type DragDropReorderParams = { + reorder: ReorderFn; + dropzonePadding?: Partial | number; + dropIndicatorMode?: DropIndicatorMode; +}; + +type DragDropReorder = { + makeDragSourceCallbackRef: ( + itemIndex: number + ) => (el: HTMLElement | null) => void; + renderDropIndicators: (itemIndex: number) => React.ReactNode; +} & Pick & + Pick; + +export function useDragDropReorder({ + reorder, + dropzonePadding: dropzonePaddingOption = ZERO_DROPZONE_PADDING, + dropIndicatorMode = `vertical`, +}: DragDropReorderParams): DragDropReorder { + // Dropzone padding + + const dropzonePaddingNormalized: DropzonePadding = + typeof dropzonePaddingOption === `number` + ? { + top: dropzonePaddingOption, + bottom: dropzonePaddingOption, + left: dropzonePaddingOption, + right: dropzonePaddingOption, + } + : {...ZERO_DROPZONE_PADDING, ...dropzonePaddingOption}; + + const dropzonePadding = useMemo( + () => dropzonePaddingNormalized, + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + dropzonePaddingNormalized.top, + dropzonePaddingNormalized.bottom, + dropzonePaddingNormalized.left, + dropzonePaddingNormalized.right, + ] + ); + + // Drag source refs + + const dragSourceByItemIndexRef = useRef>( + new Map() + ); + + const makeDragSourceCallbackRef = useCallback((itemIndex: number) => { + return (el: HTMLElement | null) => { + dragSourceByItemIndexRef.current.set(itemIndex, el); + }; + }, []); + + // Drop indicator lines + + const [dropIndicatorItemIndex, setDropIndicatorItemIndex] = useState< + number | null + >(null); + + const [dropIndicatorPosition, setDropIndicatorPosition] = + useState(null); + + const setDropIndicatorPositionForMode = useCallback( + (reorderTargetIndex: number, draggedItemIndex: number) => { + switch (dropIndicatorMode) { + case `vertical`: + setDropIndicatorPosition( + reorderTargetIndex < draggedItemIndex ? `left` : `right` + ); + return; + case `horizontal`: + setDropIndicatorPosition( + reorderTargetIndex < draggedItemIndex ? `top` : `bottom` + ); + return; + } + }, + [dropIndicatorMode] + ); + + const clearDropIndicator = useCallback(() => { + setDropIndicatorItemIndex(null); + setDropIndicatorPosition(null); + }, []); + + const renderDropIndicators = useCallback( + (itemIndex: number): React.ReactNode => { + if (dropIndicatorItemIndex !== itemIndex) { + return null; + } + return ( + <> + + {dropIndicatorPosition != null && ( + + )} + + ); + }, + [dropIndicatorItemIndex, dropIndicatorPosition, dropzonePadding] + ); + + // Reorder helper functions + + const findReorderTargetIndex: ( + ctx: DragDropState, + e: React.DragEvent + ) => number | null = useCallback( + (ctx, e) => { + const draggedItemIndex = getDraggedItemIndex(ctx); + if (draggedItemIndex == null) { + return null; + } + + const [x, y] = [e.clientX, e.clientY]; + + const entries = _.sortBy( + [...dragSourceByItemIndexRef.current.entries()], + ([itemIndex]) => itemIndex + ); + + const reorderTargetEntry = entries.find(([itemIndex, el]) => { + if (draggedItemIndex === itemIndex || el == null) { + return false; + } + + const {top, bottom, left, right} = el.getBoundingClientRect(); + return ( + x - DROPZONE_PADDING_BUFFER_PX >= left - dropzonePadding[`left`] && + x + DROPZONE_PADDING_BUFFER_PX <= right + dropzonePadding[`right`] && + y - DROPZONE_PADDING_BUFFER_PX >= top - dropzonePadding[`top`] && + y + DROPZONE_PADDING_BUFFER_PX <= bottom + dropzonePadding[`bottom`] + ); + }); + + return reorderTargetEntry?.[0] ?? null; + }, + [dropzonePadding] + ); + + // Event handlers + + const updateIndicator: DropTargetProps[`onDragOver`] = useCallback( + (ctx: DragDropState, e: React.DragEvent) => { + const draggedItemIndex = getDraggedItemIndex(ctx); + if (draggedItemIndex == null) { + return; + } + + const reorderTargetIndex = findReorderTargetIndex(ctx, e); + if (reorderTargetIndex == null) { + clearDropIndicator(); + return; + } + + setDropIndicatorItemIndex(reorderTargetIndex); + setDropIndicatorPositionForMode(reorderTargetIndex, draggedItemIndex); + }, + [ + findReorderTargetIndex, + clearDropIndicator, + setDropIndicatorPositionForMode, + ] + ); + + const triggerReorder: DropTargetProps[`onDrop`] = useCallback( + (ctx: DragDropState, e: React.DragEvent) => { + const draggedItemIndex = getDraggedItemIndex(ctx); + if (draggedItemIndex == null) { + return; + } + + clearDropIndicator(); + + const reorderTargetIndex = findReorderTargetIndex(ctx, e); + if (reorderTargetIndex == null) { + return; + } + + reorder(draggedItemIndex, reorderTargetIndex); + }, + [findReorderTargetIndex, clearDropIndicator, reorder] + ); + + useEffect(() => { + document.addEventListener(`dragover`, clearDropIndicator); + return () => { + document.removeEventListener(`dragover`, clearDropIndicator); + }; + }, [clearDropIndicator]); + + return { + makeDragSourceCallbackRef, + renderDropIndicators, + onDragOver: updateIndicator, + onDragEnd: clearDropIndicator, + onDrop: triggerReorder, + }; +} + +function getDraggedItemIndex(ctx: DragDropState): number | null { + const draggedItemIndexStr = ctx.dragRef?.id; + if (draggedItemIndexStr == null) { + return null; + } + return Number(draggedItemIndexStr); +} + +const DropIndicatorOverlay = styled.div` + position: absolute; + top: 0; + bottom: 0; + left: 0; + right: 0; + background-color: rgba(221, 237, 252, 0.5); + z-index: 1; +`; + +const DROP_INDICATOR_LINE_WIDTH_PX = 2; + +const DropIndicatorLine = styled.div<{ + paddingZone: number; + position: DropIndicatorPosition; +}>` + position: absolute; + background-color: ${globals.primary}; + + ${p => { + switch (p.position) { + case `top`: + return ` + left: 0; + right: 0; + top: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; + height: ${DROP_INDICATOR_LINE_WIDTH_PX}px; + `; + case `bottom`: + return ` + left: 0; + right: 0; + bottom: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; + height: ${DROP_INDICATOR_LINE_WIDTH_PX}px; + `; + case `left`: + return ` + top: 0; + bottom: 0; + left: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; + width: ${DROP_INDICATOR_LINE_WIDTH_PX}px; + `; + case `right`: + return ` + top: 0; + bottom: 0; + right: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; + width: ${DROP_INDICATOR_LINE_WIDTH_PX}px; + `; + } + }} +`; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx index 2573c7f4477e..987f4ce8d592 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx @@ -142,7 +142,7 @@ const stickySidebarHeaderMixin: React.CSSProperties = { /** * This component will occupy the entire space provided by the parent container. - * It is intended to be used in teh CompareEvaluations page, as it depends on + * It is intended to be used in the CompareEvaluations page, as it depends on * the EvaluationComparisonState. However, in principle, it is a general purpose * model-output comparison tool. It allows the user to view inputs, then compare * model outputs and evaluation metrics across multiple trials. @@ -194,6 +194,7 @@ const stickySidebarHeaderMixin: React.CSSProperties = { export const ExampleCompareSection: React.FC<{ state: EvaluationComparisonState; }> = props => { + console.log('ExampleCompareSection.props.state', props.state); const { filteredRows, outputColumnKeys, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleFilterSection/ExampleFilterSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleFilterSection/ExampleFilterSection.tsx index b2c8773a7d15..1146c8ea9604 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleFilterSection/ExampleFilterSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleFilterSection/ExampleFilterSection.tsx @@ -13,7 +13,7 @@ import { } from '../../compositeMetricsUtil'; import {PLOT_HEIGHT, STANDARD_PADDING} from '../../ecpConstants'; import {MAX_PLOT_DOT_SIZE, MIN_PLOT_DOT_SIZE} from '../../ecpConstants'; -import {EvaluationComparisonState} from '../../ecpState'; +import {EvaluationComparisonState, getBaselineCallId} from '../../ecpState'; import {metricDefinitionId} from '../../ecpUtil'; import { flattenedDimensionPath, @@ -103,7 +103,7 @@ const SingleDimensionFilter: React.FC<{ }, [props.state.data]); const {setComparisonDimensions} = useCompareEvaluationsState(); - const baselineCallId = props.state.baselineEvaluationCallId; + const baselineCallId = getBaselineCallId(props.state); const compareCallId = Object.keys(props.state.data.evaluationCalls).find( callId => callId !== baselineCallId )!; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx index 2422104f9648..154b824d6c8f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx @@ -30,7 +30,11 @@ import { STANDARD_PADDING, } from '../../ecpConstants'; import {SIGNIFICANT_DIGITS} from '../../ecpConstants'; -import {getOrderedCallIds, getOrderedModelRefs} from '../../ecpState'; +import { + getBaselineCallId, + getOrderedCallIds, + getOrderedModelRefs, +} from '../../ecpState'; import {EvaluationComparisonState} from '../../ecpState'; import {resolveSummaryMetricResultForEvaluateCall} from '../../ecpUtil'; import {usePeekCall} from '../../hooks'; @@ -407,7 +411,7 @@ export const ScorecardSection: React.FC<{ {evalCallIds.map((evalCallId, mNdx) => { const baseline = resolveSummaryMetricResult( - props.state.baselineEvaluationCallId, + getBaselineCallId(props.state), groupName, metricKey, compositeSummaryMetrics, From fb81f6eea16f6144bd0dcae37f55872a094f7f7f Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Wed, 9 Oct 2024 17:53:38 -0700 Subject: [PATCH 03/14] lessbroken --- .../ComparisonDefinitionSection.tsx | 31 +++----- .../EvaluationDefinition.tsx | 72 +++++++++++-------- .../ExampleCompareSection.tsx | 1 - 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index bbec4ac3eaf0..04fd25baf96d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -2,7 +2,6 @@ import {Popover} from '@mui/material'; import Input from '@wandb/weave/common/components/Input'; import { DragDropProvider, - DragSource, DropTarget, } from '@wandb/weave/common/containers/DragDropContainer'; import {Tailwind} from '@wandb/weave/components/Tailwind'; @@ -50,16 +49,11 @@ export const ComparisonDefinitionSection: React.FC<{ [setSelectedCallIdsOrdered] ); - const { - makeDragSourceCallbackRef, - renderDropIndicators, - onDragOver, - onDragEnd, - onDrop, - } = useDragDropReorder({ - reorder: reorderItems, - dropzonePadding: 8, - }); + const {makeDragSourceCallbackRef, onDragOver, onDrop, onDragEnd} = + useDragDropReorder({ + reorder: reorderItems, + dropzonePadding: 8, + }); const callIds = getOrderedCallIds(props.state); @@ -80,17 +74,12 @@ export const ComparisonDefinitionSection: React.FC<{ {callIds.map((key, ndx) => { return (
- {renderDropIndicators(ndx)} - - - + />
); })} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 3de0beb01ddb..5fb79a228926 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -2,6 +2,10 @@ import {Box} from '@material-ui/core'; import {Circle} from '@mui/icons-material'; import {WBIcon} from '@wandb/ui'; import {PopupDropdown} from '@wandb/weave/common/components/PopupDropdown'; +import { + DragDropState, + DragSource, +} from '@wandb/weave/common/containers/DragDropContainer'; import {DragHandle} from '@wandb/weave/common/containers/DragDropContainer/DragHandle'; import {Button} from '@wandb/weave/components/Button'; import {Pill} from '@wandb/weave/components/Tag'; @@ -49,7 +53,8 @@ DragHandle.displayName = 'S.DragHandle'; export const EvaluationDefinition: React.FC<{ state: EvaluationComparisonState; callId: string; - partRef: {id: string}; + ndx: number; + onDragEnd?: (ctx: DragDropState, e: React.DragEvent) => void; }> = props => { const {removeEvaluationCall, setSelectedCallIdsOrdered} = useCompareEvaluationsState(); @@ -92,36 +97,43 @@ export const EvaluationDefinition: React.FC<{ setSelectedCallIdsOrdered, ]); + const partRef = useMemo(() => ({id: `${props.ndx}`}), [props.ndx]); + return ( - - - - - - {props.callId === getBaselineCallId(props.state) && ( - - )} - - } - /> - + + + + + + + {props.callId === getBaselineCallId(props.state) && ( + + )} + + } + /> + + ); }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx index 987f4ce8d592..c9c34e1bb7d4 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ExampleCompareSection/ExampleCompareSection.tsx @@ -194,7 +194,6 @@ const stickySidebarHeaderMixin: React.CSSProperties = { export const ExampleCompareSection: React.FC<{ state: EvaluationComparisonState; }> = props => { - console.log('ExampleCompareSection.props.state', props.state); const { filteredRows, outputColumnKeys, From 2716b6ddc1728ae266f9962ebd8020dc0db67ddb Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 10 Oct 2024 15:13:16 -0700 Subject: [PATCH 04/14] better dropdown styling --- .../ComparisonDefinitionSection.tsx | 29 ++++++++-- .../EvaluationDefinition.tsx | 53 +++++++++++-------- 2 files changed, 54 insertions(+), 28 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 04fd25baf96d..427d376a1a50 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -60,6 +60,7 @@ export const ComparisonDefinitionSection: React.FC<{ return ( @@ -83,7 +84,9 @@ export const ComparisonDefinitionSection: React.FC<{
); })} - + + + @@ -118,7 +121,7 @@ const ModelRefLabel: React.FC<{modelRef: string}> = props => { const objectVersion = useObjectVersion(objVersionKey); return ( - {objectVersion.result?.objectId}:{objectVersion.result?.versionIndex} + {objectVersion.result?.objectId}:v{objectVersion.result?.versionIndex} ); }; @@ -142,7 +145,7 @@ const AddEvaluationButton: React.FC<{ ); const expandedRefCols = useMemo(() => new Set(), []); // Don't query for output here, re-queried in tsDataModelHooksEvaluationComparison.ts - const columns = useMemo(() => ['inputs'], []); + const columns = useMemo(() => ['inputs', 'display_name'], []); const calls = useCallsForQuery( props.state.data.entity, props.state.data.project, @@ -260,8 +263,24 @@ const AddEvaluationButton: React.FC<{ className="pb-8 pt-8 font-['Source_Sans_Pro'] text-base font-normal text-moon-800" onClick={() => addEvaluationCall(call.callId)}> <> - {call.displayName ?? call.spanName} - + + {call.displayName ?? call.spanName} + + + + diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 99812bb98fe2..f7318819de57 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -34,22 +34,6 @@ import { import {EvaluationComparisonState, getBaselineCallId} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; -const DragHandleIcon = styled(WBIcon).attrs({name: 'vertical-handle'})` - font-size: calc(30 * 1.75); - border-radius: 50%; - color: gray300; - user-select: none; - cursor: grab; - &&&:hover { - background: gray100; - color: black; - } - &:active { - cursor: grabbing; - } -`; -DragHandle.displayName = 'S.DragHandle'; - export const EvaluationDefinition: React.FC<{ state: EvaluationComparisonState; callId: string; @@ -109,27 +93,34 @@ export const EvaluationDefinition: React.FC<{ height: EVAL_DEF_HEIGHT, borderRadius: BOX_RADIUS, border: STANDARD_BORDER, - padding: '12px', + paddingTop: '12px', + paddingBottom: '12px', alignItems: 'center', justifyContent: 'space-between', }}> - + - +
+ +
{props.callId === getBaselineCallId(props.state) && ( - +
+ +
)} -
+
} /> @@ -258,3 +249,19 @@ export const VerticalBar: React.FC = () => { /> ); }; + +const DragHandleIcon = styled(WBIcon).attrs({name: 'vertical-handle'})` + font-size: 26px; + border-radius: 50%; + color: gray300; + user-select: none; + cursor: grab; + &&&:hover { + background: gray100; + color: black; + } + &:active { + cursor: grabbing; + } +`; +DragHandle.displayName = 'S.DragHandle'; From 8f26dba39320be0c5e2372be79bdcd80e1b6fa26 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 10 Oct 2024 15:18:56 -0700 Subject: [PATCH 05/14] use-opNiceName --- .../ComparisonDefinitionSection.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 427d376a1a50..141d45161036 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -16,6 +16,7 @@ import { import {useCallsForQuery} from '../../../CallsPage/callsTableQuery'; import {useEvaluationsFilter} from '../../../CallsPage/evaluationsFilter'; import {Id} from '../../../common/Id'; +import {opNiceName} from '../../../common/Links'; import {useWFHooks} from '../../../wfReactInterface/context'; import { CallSchema, @@ -272,7 +273,7 @@ const AddEvaluationButton: React.FC<{ flexShrink: 1, maxWidth: '250px', }}> - {call.displayName ?? call.spanName} + {call.displayName ?? opNiceName(call.spanName)} Date: Thu, 10 Oct 2024 15:25:24 -0700 Subject: [PATCH 06/14] move --- .../EvaluationDefinition.tsx | 20 +------------------ .../ComparisonDefinitionSection/dragUtils.tsx | 17 ++++++++++++++++ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index f7318819de57..57ba15b3b57a 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -1,6 +1,5 @@ import {Box} from '@material-ui/core'; import {Circle} from '@mui/icons-material'; -import {WBIcon} from '@wandb/ui'; import {PopupDropdown} from '@wandb/weave/common/components/PopupDropdown'; import { DragDropState, @@ -10,7 +9,6 @@ import {DragHandle} from '@wandb/weave/common/containers/DragDropContainer/DragH import {Button} from '@wandb/weave/components/Button'; import {Pill} from '@wandb/weave/components/Tag'; import React, {useMemo} from 'react'; -import styled from 'styled-components'; import { MOON_300, @@ -33,6 +31,7 @@ import { } from '../../ecpConstants'; import {EvaluationComparisonState, getBaselineCallId} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; +import {DragHandleIcon} from './dragUtils'; export const EvaluationDefinition: React.FC<{ state: EvaluationComparisonState; @@ -120,7 +119,6 @@ export const EvaluationDefinition: React.FC<{ icon="overflow-horizontal" size="small" variant="ghost" - // style={{marginLeft: '4px'}} /> } /> @@ -249,19 +247,3 @@ export const VerticalBar: React.FC = () => { /> ); }; - -const DragHandleIcon = styled(WBIcon).attrs({name: 'vertical-handle'})` - font-size: 26px; - border-radius: 50%; - color: gray300; - user-select: none; - cursor: grab; - &&&:hover { - background: gray100; - color: black; - } - &:active { - cursor: grabbing; - } -`; -DragHandle.displayName = 'S.DragHandle'; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx index df2f9355a8ab..524bdbb640bd 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx @@ -1,5 +1,6 @@ /***** COPIED FROM app/src/util/dragDrop.tsx *****/ +import {WBIcon} from '@wandb/ui'; import { DragDropState, DragSourceProps, @@ -291,3 +292,19 @@ const DropIndicatorLine = styled.div<{ } }} `; + +export const DragHandleIcon = styled(WBIcon).attrs({name: 'vertical-handle'})` + font-size: 26px; + border-radius: 50%; + color: gray300; + user-select: none; + cursor: grab; + &&&:hover { + background: gray100; + color: black; + } + &:active { + cursor: grabbing; + } +`; +DragHandleIcon.displayName = 'S.DragHandle'; From 71a11824203e1369d67c74e72d9430b10d6a4dd3 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 3 Dec 2024 15:17:11 -0800 Subject: [PATCH 07/14] review comments --- .../CompareEvaluationsPage.tsx | 9 --- .../compareEvaluationsContext.tsx | 15 ++--- .../pages/CompareEvaluationsPage/ecpState.ts | 57 ++++++++++++++----- .../pages/CompareEvaluationsPage/ecpTypes.ts | 1 + .../ComparisonDefinitionSection.tsx | 31 +++++----- .../EvaluationDefinition.tsx | 35 +++++------- .../ScorecardSection/ScorecardSection.tsx | 1 + 7 files changed, 79 insertions(+), 70 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx index ac1e0a830234..478c48875463 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/CompareEvaluationsPage.tsx @@ -77,9 +77,6 @@ export const CompareEvaluationsPage: React.FC< export const CompareEvaluationsPageContent: React.FC< CompareEvaluationsPageProps > = props => { - const [selectedCallIdsOrdered, setSelectedCallIdsOrdered] = React.useState< - string[] - >([]); const [comparisonDimensions, setComparisonDimensions] = React.useState(null); @@ -105,10 +102,6 @@ export const CompareEvaluationsPageContent: React.FC< [comparisonDimensions] ); - React.useEffect(() => { - setSelectedCallIdsOrdered(props.evaluationCallIds); - }, [props.evaluationCallIds, setSelectedCallIdsOrdered]); - if (props.evaluationCallIds.length === 0) { return
No evaluations to compare
; } @@ -120,8 +113,6 @@ export const CompareEvaluationsPageContent: React.FC< initialEvaluationCallIds={props.evaluationCallIds} selectedMetrics={props.selectedMetrics} setSelectedMetrics={props.setSelectedMetrics} - selectedCallIdsOrdered={selectedCallIdsOrdered} - setSelectedCallIdsOrdered={setSelectedCallIdsOrdered} comparisonDimensions={comparisonDimensions ?? undefined} onEvaluationCallIdsUpdate={props.onEvaluationCallIdsUpdate} setComparisonDimensions={setComparisonDimensionsAndClearInputDigest} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx index d743ebf479ba..638565e8ad6f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/compareEvaluationsContext.tsx @@ -10,7 +10,6 @@ import {ComparisonDimensionsType} from './ecpState'; const CompareEvaluationsContext = React.createContext<{ state: EvaluationComparisonState; - setSelectedCallIdsOrdered: React.Dispatch>; setComparisonDimensions: React.Dispatch< React.SetStateAction >; @@ -18,6 +17,7 @@ const CompareEvaluationsContext = React.createContext<{ setSelectedMetrics: (newModel: Record) => void; addEvaluationCall: (newCallId: string) => void; removeEvaluationCall: (callId: string) => void; + setEvaluationCallOrder: (newCallIdOrder: string[]) => void; } | null>(null); export const useCompareEvaluationsState = () => { @@ -32,8 +32,6 @@ export const CompareEvaluationsProvider: React.FC<{ entity: string; project: string; initialEvaluationCallIds: string[]; - selectedCallIdsOrdered: string[]; - setSelectedCallIdsOrdered: React.Dispatch>; selectedMetrics: Record | null; setSelectedMetrics: (newModel: Record) => void; @@ -48,8 +46,6 @@ export const CompareEvaluationsProvider: React.FC<{ entity, project, initialEvaluationCallIds, - setSelectedCallIdsOrdered, - selectedCallIdsOrdered, selectedMetrics, setSelectedMetrics, onEvaluationCallIdsUpdate, @@ -71,7 +67,6 @@ export const CompareEvaluationsProvider: React.FC<{ entity, project, evaluationCallIds, - selectedCallIdsOrdered, comparisonDimensions, selectedInputDigest, selectedMetrics ?? undefined @@ -83,14 +78,12 @@ export const CompareEvaluationsProvider: React.FC<{ } return { state: initialState.result, - setSelectedCallIdsOrdered, setComparisonDimensions, setSelectedInputDigest, setSelectedMetrics, addEvaluationCall: (newCallId: string) => { const newEvaluationCallIds = [...evaluationCallIds, newCallId]; setEvaluationCallIds(newEvaluationCallIds); - setSelectedCallIdsOrdered(newEvaluationCallIds); onEvaluationCallIdsUpdate(newEvaluationCallIds); }, removeEvaluationCall: (callId: string) => { @@ -98,15 +91,17 @@ export const CompareEvaluationsProvider: React.FC<{ id => id !== callId ); setEvaluationCallIds(newEvaluationCallIds); - setSelectedCallIdsOrdered(newEvaluationCallIds); onEvaluationCallIdsUpdate(newEvaluationCallIds); }, + setEvaluationCallOrder: (newCallIdOrder: string[]) => { + setEvaluationCallIds(newCallIdOrder); + onEvaluationCallIdsUpdate(newCallIdOrder); + }, }; }, [ initialState.loading, initialState.result, setEvaluationCallIds, - setSelectedCallIdsOrdered, evaluationCallIds, onEvaluationCallIdsUpdate, setComparisonDimensions, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts index 6760af3b05fa..e8509c426ed3 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts @@ -24,8 +24,8 @@ export type EvaluationComparisonState = { selectedInputDigest?: string; // The selected metrics to display selectedMetrics?: Record; - // The order of the evaluation calls, the first call is always the baseline - selectedCallIdsOrdered: string[]; + // Ordered call Ids + evaluationCallIdsOrdered: string[]; }; export type ComparisonDimensionsType = Array<{ @@ -43,12 +43,14 @@ export const useEvaluationComparisonState = ( entity: string, project: string, evaluationCallIds: string[], - selectedCallIdsOrdered: string[], comparisonDimensions?: ComparisonDimensionsType, selectedInputDigest?: string, selectedMetrics?: Record ): Loadable => { - const data = useEvaluationComparisonData(entity, project, evaluationCallIds); + const orderedCallIds = useMemo(() => { + return getCallIdsOrderedForQuery(evaluationCallIds); + }, [evaluationCallIds]); + const data = useEvaluationComparisonData(entity, project, orderedCallIds); const value = useMemo(() => { if (data.result == null || data.loading) { @@ -94,34 +96,35 @@ export const useEvaluationComparisonState = ( data: data.result, comparisonDimensions: newComparisonDimensions, selectedInputDigest, - selectedCallIdsOrdered, selectedMetrics, + evaluationCallIdsOrdered: evaluationCallIds, }, }; }, [ data.result, data.loading, - selectedCallIdsOrdered, comparisonDimensions, selectedInputDigest, selectedMetrics, + evaluationCallIds, ]); return value; }; -/** - * Should use this over keys of `state.data.evaluationCalls` because it ensures the baseline - * evaluation call is first. - */ export const getOrderedCallIds = (state: EvaluationComparisonState) => { - return state.selectedCallIdsOrdered.filter( - callId => state.data.evaluationCalls[callId] != null - ); + return state.evaluationCallIdsOrdered; }; export const getBaselineCallId = (state: EvaluationComparisonState) => { - return state.selectedCallIdsOrdered[0]; + return getOrderedCallIds(state)[0]; +}; + +/** + * Sort call IDs to ensure consistent order for memoized query params + */ +const getCallIdsOrderedForQuery = (callIds: string[]) => { + return Array.from(callIds).sort(); }; /** @@ -147,3 +150,29 @@ const moveItemToFront = (arr: T[], item: T): T[] => { } return arr; }; + +export const getOrderedEvalsWithNewBaseline = ( + evaluationCallIds: string[], + newBaselineCallId: string +) => { + return moveItemToFront(evaluationCallIds, newBaselineCallId); +}; + +export const swapEvaluationCalls = ( + evaluationCallIds: string[], + ndx1: number, + ndx2: number +): string[] => { + return swapArrayItems(evaluationCallIds, ndx1, ndx2); +}; + +const swapArrayItems = (arr: T[], ndx1: number, ndx2: number): T[] => { + if (ndx1 < 0 || ndx2 < 0 || ndx1 >= arr.length || ndx2 >= arr.length) { + throw new Error('Index out of bounds'); + } + const newArr = [...arr]; + const from = newArr[ndx1]; + newArr[ndx1] = newArr[ndx2]; + newArr[ndx2] = from; + return newArr; +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts index 7454e0707b48..b4642fae240d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts @@ -18,6 +18,7 @@ export type EvaluationComparisonData = { }; // EvaluationCalls are the specific calls of an evaluation. + // The visual order of the evaluation calls is determined by the order of the keys. evaluationCalls: { [callId: string]: EvaluationCall; }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 69d64d7466e4..c9c25c4ccb10 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -24,7 +24,11 @@ import { } from '../../../wfReactInterface/wfDataModelHooksInterface'; import {useCompareEvaluationsState} from '../../compareEvaluationsContext'; import {STANDARD_PADDING} from '../../ecpConstants'; -import {EvaluationComparisonState, getOrderedCallIds} from '../../ecpState'; +import { + EvaluationComparisonState, + getOrderedCallIds, + swapEvaluationCalls, +} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; import {useDragDropReorder} from './dragUtils'; import {EvaluationDefinition, VerticalBar} from './EvaluationDefinition'; @@ -32,22 +36,15 @@ import {EvaluationDefinition, VerticalBar} from './EvaluationDefinition'; export const ComparisonDefinitionSection: React.FC<{ state: EvaluationComparisonState; }> = props => { - const {setSelectedCallIdsOrdered} = useCompareEvaluationsState(); + const {setEvaluationCallOrder} = useCompareEvaluationsState(); const reorderItems = useCallback( (fromIndex: number, toIndex: number) => { - setSelectedCallIdsOrdered(prev => { - if (prev == null) { - return prev; - } - const newOrder = [...prev]; - const from = newOrder[fromIndex]; - newOrder[fromIndex] = newOrder[toIndex]; - newOrder[toIndex] = from; - return newOrder; - }); + const currentOrder = getOrderedCallIds(props.state); + const newOrder = swapEvaluationCalls(currentOrder, fromIndex, toIndex); + setEvaluationCallOrder(newOrder); }, - [setSelectedCallIdsOrdered] + [setEvaluationCallOrder, props.state] ); const {makeDragSourceCallbackRef, onDragOver, onDrop, onDragEnd} = @@ -56,7 +53,9 @@ export const ComparisonDefinitionSection: React.FC<{ dropzonePadding: 8, }); - const callIds = getOrderedCallIds(props.state); + const callIds = useMemo(() => { + return getOrderedCallIds(props.state); + }, [props.state]); return ( @@ -160,9 +159,9 @@ const AddEvaluationButton: React.FC<{ const evalsNotComparing = useMemo(() => { return calls.result.filter( - call => !props.state.selectedCallIdsOrdered.includes(call.callId) + call => !getOrderedCallIds(props.state).includes(call.callId) ); - }, [calls.result, props.state.selectedCallIdsOrdered]); + }, [calls.result, props.state]); const [menuOptions, setMenuOptions] = useState(evalsNotComparing); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 57ba15b3b57a..eab621cef3d9 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -29,7 +29,12 @@ import { EVAL_DEF_HEIGHT, STANDARD_BORDER, } from '../../ecpConstants'; -import {EvaluationComparisonState, getBaselineCallId} from '../../ecpState'; +import { + EvaluationComparisonState, + getBaselineCallId, + getOrderedCallIds, + getOrderedEvalsWithNewBaseline, +} from '../../ecpState'; import {HorizontalBox} from '../../Layout'; import {DragHandleIcon} from './dragUtils'; @@ -39,7 +44,7 @@ export const EvaluationDefinition: React.FC<{ ndx: number; onDragEnd?: (ctx: DragDropState, e: React.DragEvent) => void; }> = props => { - const {removeEvaluationCall, setSelectedCallIdsOrdered} = + const {removeEvaluationCall, setEvaluationCallOrder} = useCompareEvaluationsState(); const menuOptions = useMemo(() => { @@ -48,19 +53,12 @@ export const EvaluationDefinition: React.FC<{ key: 'add-to-baseline', content: 'Set as baseline', onClick: () => { - setSelectedCallIdsOrdered(prev => { - if (prev == null) { - return prev; - } - const index = prev.indexOf(props.callId); - if (index === 0) { - return prev; - } - const newOrder = [...prev]; - newOrder.splice(index, 1); - newOrder.unshift(props.callId); - return newOrder; - }); + const currentOrder = getOrderedCallIds(props.state); + const newOrder = getOrderedEvalsWithNewBaseline( + currentOrder, + props.callId + ); + setEvaluationCallOrder(newOrder); }, disabled: props.callId === getBaselineCallId(props.state), }, @@ -73,12 +71,7 @@ export const EvaluationDefinition: React.FC<{ disabled: Object.keys(props.state.data.evaluationCalls).length === 1, }, ]; - }, [ - props.callId, - props.state, - removeEvaluationCall, - setSelectedCallIdsOrdered, - ]); + }, [props.callId, props.state, removeEvaluationCall, setEvaluationCallOrder]); const partRef = useMemo(() => ({id: `${props.ndx}`}), [props.ndx]); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx index c587be8f0cdc..303f640f47e2 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ScorecardSection/ScorecardSection.tsx @@ -32,6 +32,7 @@ import { } from '../../ecpConstants'; import { EvaluationComparisonState, + getBaselineCallId, getOrderedCallIds, getOrderedModelRefs, } from '../../ecpState'; From 8a9feb1e05a4cef7c22872b63cdbfd78a69d253e Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 9 Dec 2024 17:36:56 -0800 Subject: [PATCH 08/14] use shopping cart --- .../compare/ComparePageObjectsLoaded.tsx | 2 +- .../pages/CompareEvaluationsPage/ecpState.ts | 2 +- .../ComparisonDefinitionSection.tsx | 110 +++---- .../EvaluationDefinition.tsx | 108 +----- .../ComparisonDefinitionSection/dragUtils.tsx | 310 ------------------ .../common/shoppingCart}/ShoppingCart.tsx | 6 +- .../common/shoppingCart}/ShoppingCartItem.tsx | 12 +- 7 files changed, 68 insertions(+), 482 deletions(-) delete mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx rename weave-js/src/components/PagePanelComponents/Home/Browse3/{compare => pages/common/shoppingCart}/ShoppingCart.tsx (95%) rename weave-js/src/components/PagePanelComponents/Home/Browse3/{compare => pages/common/shoppingCart}/ShoppingCartItem.tsx (92%) 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 3b3548f09969..64c79a731693 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx @@ -14,6 +14,7 @@ import {Icon} from '../../../../Icon'; import {TailwindContents} from '../../../../Tailwind'; import {isWeaveRef} from '../filters/common'; import {mapObject, TraverseContext} from '../pages/CallPage/traverse'; +import {ShoppingCart} from '../pages/common/shoppingCart/ShoppingCart'; import {SimplePageLayout} from '../pages/common/SimplePageLayout'; import {useWFHooks} from '../pages/wfReactInterface/context'; import { @@ -26,7 +27,6 @@ import {computeDiff, mergeObjects} from './compare'; import {CompareGrid, MAX_OBJECT_COLS} from './CompareGrid'; import {isSequentialVersions, parseSpecifier} from './hooks'; import {getExpandableRefs, RefValues, RESOLVED_REF_KEY} from './refUtil'; -import {ShoppingCart} from './ShoppingCart'; import {ComparableObject, Mode} from './types'; type ComparePageObjectsLoadedProps = { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts index e8509c426ed3..e5c1b03d60a4 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpState.ts @@ -113,7 +113,7 @@ export const useEvaluationComparisonState = ( }; export const getOrderedCallIds = (state: EvaluationComparisonState) => { - return state.evaluationCallIdsOrdered; + return Array.from(state.evaluationCallIdsOrdered); }; export const getBaselineCallId = (state: EvaluationComparisonState) => { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 18cc1a8579ce..00ce0826d780 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -1,14 +1,11 @@ import {Popover} from '@mui/material'; import Input from '@wandb/weave/common/components/Input'; -import { - DragDropProvider, - DropTarget, -} from '@wandb/weave/common/containers/DragDropContainer'; import {Tailwind} from '@wandb/weave/components/Tailwind'; import {parseRef, WeaveObjectRef} from '@wandb/weave/react'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useEffect, useMemo, useRef, useState} from 'react'; import {Button} from '../../../../../../../Button'; +import {ShoppingCartItemDefs} from '../../../../compare/types'; import { DEFAULT_FILTER_CALLS, DEFAULT_SORT_CALLS, @@ -17,79 +14,82 @@ import {useCallsForQuery} from '../../../CallsPage/callsTableQuery'; import {useEvaluationsFilter} from '../../../CallsPage/evaluationsFilter'; import {Id} from '../../../common/Id'; import {opNiceName} from '../../../common/Links'; +import {SortableItems} from '../../../common/shoppingCart/ShoppingCart'; import {useWFHooks} from '../../../wfReactInterface/context'; import { CallSchema, ObjectVersionKey, } from '../../../wfReactInterface/wfDataModelHooksInterface'; import {useCompareEvaluationsState} from '../../compareEvaluationsContext'; -import {STANDARD_PADDING} from '../../ecpConstants'; import { EvaluationComparisonState, getOrderedCallIds, + getOrderedEvalsWithNewBaseline, swapEvaluationCalls, } from '../../ecpState'; import {HorizontalBox} from '../../Layout'; -import {useDragDropReorder} from './dragUtils'; -import {EvaluationDefinition, VerticalBar} from './EvaluationDefinition'; +import {VerticalBar} from './EvaluationDefinition'; export const ComparisonDefinitionSection: React.FC<{ state: EvaluationComparisonState; }> = props => { - const {setEvaluationCallOrder} = useCompareEvaluationsState(); - - const reorderItems = useCallback( - (fromIndex: number, toIndex: number) => { - const currentOrder = getOrderedCallIds(props.state); - const newOrder = swapEvaluationCalls(currentOrder, fromIndex, toIndex); - setEvaluationCallOrder(newOrder); - }, - [setEvaluationCallOrder, props.state] - ); - - const {makeDragSourceCallbackRef, onDragOver, onDrop, onDragEnd} = - useDragDropReorder({ - reorder: reorderItems, - dropzonePadding: 8, - }); + const {setEvaluationCallOrder, removeEvaluationCall} = + useCompareEvaluationsState(); const callIds = useMemo(() => { return getOrderedCallIds(props.state); }, [props.state]); + const shoppingCartItems: ShoppingCartItemDefs = useMemo(() => { + return callIds.map(callId => ({ + key: 'evaluations', + value: callId, + label: props.state.data.evaluationCalls[callId]?.name ?? callId, + })); + }, [callIds, props.state.data.evaluationCalls]); + + const onSetBaseline = (value: string | null) => { + if (!value) { + return; + } + const newSortOrder = getOrderedEvalsWithNewBaseline(callIds, value); + setEvaluationCallOrder(newSortOrder); + }; + + const onRemoveShoppingCartItem = (value: string) => { + removeEvaluationCall(value); + }; + + const onSortEnd = ({ + oldIndex, + newIndex, + }: { + oldIndex: number; + newIndex: number; + }) => { + const newSortOrder = swapEvaluationCalls(callIds, oldIndex, newIndex); + setEvaluationCallOrder(newSortOrder); + }; + return ( - - - - {callIds.map((key, ndx) => { - return ( -
- -
- ); - })} - - - + +
+ {}} + onSetBaseline={onSetBaseline} + onRemoveShoppingCartItem={onRemoveShoppingCartItem} + onSortEnd={onSortEnd} + /> + + - - +
+ ); }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index eab621cef3d9..5dcf835e378a 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -1,13 +1,5 @@ import {Box} from '@material-ui/core'; import {Circle} from '@mui/icons-material'; -import {PopupDropdown} from '@wandb/weave/common/components/PopupDropdown'; -import { - DragDropState, - DragSource, -} from '@wandb/weave/common/containers/DragDropContainer'; -import {DragHandle} from '@wandb/weave/common/containers/DragDropContainer/DragHandle'; -import {Button} from '@wandb/weave/components/Button'; -import {Pill} from '@wandb/weave/components/Tag'; import React, {useMemo} from 'react'; import { @@ -22,104 +14,8 @@ import {SmallRef} from '../../../../../Browse2/SmallRef'; import {CallLink, ObjectVersionLink} from '../../../common/Links'; import {useWFHooks} from '../../../wfReactInterface/context'; import {ObjectVersionKey} from '../../../wfReactInterface/wfDataModelHooksInterface'; -import {useCompareEvaluationsState} from '../../compareEvaluationsContext'; -import { - BOX_RADIUS, - CIRCLE_SIZE, - EVAL_DEF_HEIGHT, - STANDARD_BORDER, -} from '../../ecpConstants'; -import { - EvaluationComparisonState, - getBaselineCallId, - getOrderedCallIds, - getOrderedEvalsWithNewBaseline, -} from '../../ecpState'; -import {HorizontalBox} from '../../Layout'; -import {DragHandleIcon} from './dragUtils'; - -export const EvaluationDefinition: React.FC<{ - state: EvaluationComparisonState; - callId: string; - ndx: number; - onDragEnd?: (ctx: DragDropState, e: React.DragEvent) => void; -}> = props => { - const {removeEvaluationCall, setEvaluationCallOrder} = - useCompareEvaluationsState(); - - const menuOptions = useMemo(() => { - return [ - { - key: 'add-to-baseline', - content: 'Set as baseline', - onClick: () => { - const currentOrder = getOrderedCallIds(props.state); - const newOrder = getOrderedEvalsWithNewBaseline( - currentOrder, - props.callId - ); - setEvaluationCallOrder(newOrder); - }, - disabled: props.callId === getBaselineCallId(props.state), - }, - { - key: 'remove', - content: 'Remove', - onClick: () => { - removeEvaluationCall(props.callId); - }, - disabled: Object.keys(props.state.data.evaluationCalls).length === 1, - }, - ]; - }, [props.callId, props.state, removeEvaluationCall, setEvaluationCallOrder]); - - const partRef = useMemo(() => ({id: `${props.ndx}`}), [props.ndx]); - - return ( - - - - - -
- -
- {props.callId === getBaselineCallId(props.state) && ( -
- -
- )} -
- - } - /> -
- - - ); -}; +import {CIRCLE_SIZE} from '../../ecpConstants'; +import {EvaluationComparisonState} from '../../ecpState'; export const EvaluationCallLink: React.FC<{ callId: string; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx deleted file mode 100644 index 524bdbb640bd..000000000000 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/dragUtils.tsx +++ /dev/null @@ -1,310 +0,0 @@ -/***** COPIED FROM app/src/util/dragDrop.tsx *****/ - -import {WBIcon} from '@wandb/ui'; -import { - DragDropState, - DragSourceProps, - DropTargetProps, -} from '@wandb/weave/common/containers/DragDropContainer'; -import * as globals from '@wandb/weave/common/css/globals.styles'; -import _ from 'lodash'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; -import styled from 'styled-components'; - -export type ReorderFn = (fromIndex: number, toIndex: number) => void; - -type DropIndicatorPosition = `top` | `bottom` | `left` | `right`; - -type DropIndicatorMode = `vertical` | `horizontal`; - -type DropzonePadding = {[P in DropIndicatorPosition]: number}; - -const ZERO_DROPZONE_PADDING: DropzonePadding = { - top: 0, - bottom: 0, - left: 0, - right: 0, -}; - -const DROPZONE_PADDING_BUFFER_PX = 2; - -type DragDropReorderParams = { - reorder: ReorderFn; - dropzonePadding?: Partial | number; - dropIndicatorMode?: DropIndicatorMode; -}; - -type DragDropReorder = { - makeDragSourceCallbackRef: ( - itemIndex: number - ) => (el: HTMLElement | null) => void; - renderDropIndicators: (itemIndex: number) => React.ReactNode; -} & Pick & - Pick; - -export function useDragDropReorder({ - reorder, - dropzonePadding: dropzonePaddingOption = ZERO_DROPZONE_PADDING, - dropIndicatorMode = `vertical`, -}: DragDropReorderParams): DragDropReorder { - // Dropzone padding - - const dropzonePaddingNormalized: DropzonePadding = - typeof dropzonePaddingOption === `number` - ? { - top: dropzonePaddingOption, - bottom: dropzonePaddingOption, - left: dropzonePaddingOption, - right: dropzonePaddingOption, - } - : {...ZERO_DROPZONE_PADDING, ...dropzonePaddingOption}; - - const dropzonePadding = useMemo( - () => dropzonePaddingNormalized, - // eslint-disable-next-line react-hooks/exhaustive-deps - [ - dropzonePaddingNormalized.top, - dropzonePaddingNormalized.bottom, - dropzonePaddingNormalized.left, - dropzonePaddingNormalized.right, - ] - ); - - // Drag source refs - - const dragSourceByItemIndexRef = useRef>( - new Map() - ); - - const makeDragSourceCallbackRef = useCallback((itemIndex: number) => { - return (el: HTMLElement | null) => { - dragSourceByItemIndexRef.current.set(itemIndex, el); - }; - }, []); - - // Drop indicator lines - - const [dropIndicatorItemIndex, setDropIndicatorItemIndex] = useState< - number | null - >(null); - - const [dropIndicatorPosition, setDropIndicatorPosition] = - useState(null); - - const setDropIndicatorPositionForMode = useCallback( - (reorderTargetIndex: number, draggedItemIndex: number) => { - switch (dropIndicatorMode) { - case `vertical`: - setDropIndicatorPosition( - reorderTargetIndex < draggedItemIndex ? `left` : `right` - ); - return; - case `horizontal`: - setDropIndicatorPosition( - reorderTargetIndex < draggedItemIndex ? `top` : `bottom` - ); - return; - } - }, - [dropIndicatorMode] - ); - - const clearDropIndicator = useCallback(() => { - setDropIndicatorItemIndex(null); - setDropIndicatorPosition(null); - }, []); - - const renderDropIndicators = useCallback( - (itemIndex: number): React.ReactNode => { - if (dropIndicatorItemIndex !== itemIndex) { - return null; - } - return ( - <> - - {dropIndicatorPosition != null && ( - - )} - - ); - }, - [dropIndicatorItemIndex, dropIndicatorPosition, dropzonePadding] - ); - - // Reorder helper functions - - const findReorderTargetIndex: ( - ctx: DragDropState, - e: React.DragEvent - ) => number | null = useCallback( - (ctx, e) => { - const draggedItemIndex = getDraggedItemIndex(ctx); - if (draggedItemIndex == null) { - return null; - } - - const [x, y] = [e.clientX, e.clientY]; - - const entries = _.sortBy( - [...dragSourceByItemIndexRef.current.entries()], - ([itemIndex]) => itemIndex - ); - - const reorderTargetEntry = entries.find(([itemIndex, el]) => { - if (draggedItemIndex === itemIndex || el == null) { - return false; - } - - const {top, bottom, left, right} = el.getBoundingClientRect(); - return ( - x - DROPZONE_PADDING_BUFFER_PX >= left - dropzonePadding[`left`] && - x + DROPZONE_PADDING_BUFFER_PX <= right + dropzonePadding[`right`] && - y - DROPZONE_PADDING_BUFFER_PX >= top - dropzonePadding[`top`] && - y + DROPZONE_PADDING_BUFFER_PX <= bottom + dropzonePadding[`bottom`] - ); - }); - - return reorderTargetEntry?.[0] ?? null; - }, - [dropzonePadding] - ); - - // Event handlers - - const updateIndicator: DropTargetProps[`onDragOver`] = useCallback( - (ctx: DragDropState, e: React.DragEvent) => { - const draggedItemIndex = getDraggedItemIndex(ctx); - if (draggedItemIndex == null) { - return; - } - - const reorderTargetIndex = findReorderTargetIndex(ctx, e); - if (reorderTargetIndex == null) { - clearDropIndicator(); - return; - } - - setDropIndicatorItemIndex(reorderTargetIndex); - setDropIndicatorPositionForMode(reorderTargetIndex, draggedItemIndex); - }, - [ - findReorderTargetIndex, - clearDropIndicator, - setDropIndicatorPositionForMode, - ] - ); - - const triggerReorder: DropTargetProps[`onDrop`] = useCallback( - (ctx: DragDropState, e: React.DragEvent) => { - const draggedItemIndex = getDraggedItemIndex(ctx); - if (draggedItemIndex == null) { - return; - } - - clearDropIndicator(); - - const reorderTargetIndex = findReorderTargetIndex(ctx, e); - if (reorderTargetIndex == null) { - return; - } - - reorder(draggedItemIndex, reorderTargetIndex); - }, - [findReorderTargetIndex, clearDropIndicator, reorder] - ); - - useEffect(() => { - document.addEventListener(`dragover`, clearDropIndicator); - return () => { - document.removeEventListener(`dragover`, clearDropIndicator); - }; - }, [clearDropIndicator]); - - return { - makeDragSourceCallbackRef, - renderDropIndicators, - onDragOver: updateIndicator, - onDragEnd: clearDropIndicator, - onDrop: triggerReorder, - }; -} - -function getDraggedItemIndex(ctx: DragDropState): number | null { - const draggedItemIndexStr = ctx.dragRef?.id; - if (draggedItemIndexStr == null) { - return null; - } - return Number(draggedItemIndexStr); -} - -const DropIndicatorOverlay = styled.div` - position: absolute; - top: 0; - bottom: 0; - left: 0; - right: 0; - background-color: rgba(221, 237, 252, 0.5); - z-index: 1; -`; - -const DROP_INDICATOR_LINE_WIDTH_PX = 2; - -const DropIndicatorLine = styled.div<{ - paddingZone: number; - position: DropIndicatorPosition; -}>` - position: absolute; - background-color: ${globals.primary}; - - ${p => { - switch (p.position) { - case `top`: - return ` - left: 0; - right: 0; - top: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; - height: ${DROP_INDICATOR_LINE_WIDTH_PX}px; - `; - case `bottom`: - return ` - left: 0; - right: 0; - bottom: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; - height: ${DROP_INDICATOR_LINE_WIDTH_PX}px; - `; - case `left`: - return ` - top: 0; - bottom: 0; - left: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; - width: ${DROP_INDICATOR_LINE_WIDTH_PX}px; - `; - case `right`: - return ` - top: 0; - bottom: 0; - right: -${p.paddingZone + DROP_INDICATOR_LINE_WIDTH_PX / 2}px; - width: ${DROP_INDICATOR_LINE_WIDTH_PX}px; - `; - } - }} -`; - -export const DragHandleIcon = styled(WBIcon).attrs({name: 'vertical-handle'})` - font-size: 26px; - border-radius: 50%; - color: gray300; - user-select: none; - cursor: grab; - &&&:hover { - background: gray100; - color: black; - } - &:active { - cursor: grabbing; - } -`; -DragHandleIcon.displayName = 'S.DragHandle'; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx similarity index 95% rename from weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx rename to weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx index 8bb0f05785b7..3592c1d2e6a5 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCart.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx @@ -7,9 +7,9 @@ import React from 'react'; import {useHistory} from 'react-router-dom'; import {SortableContainer} from 'react-sortable-hoc'; -import {queryToggleString, searchParamsSetArray} from '../urlQueryUtil'; +import {ShoppingCartItemDefs} from '../../../compare/types'; +import {queryToggleString, searchParamsSetArray} from '../../../urlQueryUtil'; import {ShoppingCartItem} from './ShoppingCartItem'; -import {ShoppingCartItemDefs} from './types'; type ShoppingCartItemsProps = { items: ShoppingCartItemDefs; @@ -23,7 +23,7 @@ type SortableItemsProps = ShoppingCartItemsProps & { onRemoveShoppingCartItem: (value: string) => void; }; -const SortableItems = SortableContainer( +export const SortableItems = SortableContainer( ({ items, baselineEnabled, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx similarity index 92% rename from weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx rename to weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx index 125b9ebd2711..a6dea143bf84 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ShoppingCartItem.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx @@ -7,12 +7,12 @@ 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'; +import {Button} from '../../../../../../Button'; +import {Icon} from '../../../../../../Icon'; +import {Pill} from '../../../../../../Tag'; +import {Tailwind} from '../../../../../../Tailwind'; +import {Tooltip} from '../../../../../../Tooltip'; +import {ShoppingCartItemDef} from '../../../compare/types'; // TODO: Change cursor to grabbing when dragging const DragHandle = SortableHandle(() => ( From e652973a025b9179c4d9bbec148065bbbcdb9381 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Dec 2024 09:14:06 -0800 Subject: [PATCH 09/14] the easy review comments, todo the color circle --- .../PagePanelComponents/Home/Browse3/compare/types.ts | 8 -------- .../ComparisonDefinitionSection.tsx | 2 +- .../Browse3/pages/common/shoppingCart/ShoppingCart.tsx | 2 +- .../pages/common/shoppingCart/ShoppingCartItem.tsx | 2 +- .../Home/Browse3/pages/common/shoppingCart/types.ts | 7 +++++++ 5 files changed, 10 insertions(+), 11 deletions(-) create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts index 26e5e01ab993..e6111427b3ef 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts @@ -1,11 +1,3 @@ -export type ShoppingCartItemDef = { - key: string; - value: string; - label?: string; -}; - -export type ShoppingCartItemDefs = ShoppingCartItemDef[]; - export type ComparableObject = Record; export type ComparableObjects = ComparableObject[]; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 00ce0826d780..fdd04b429b84 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -5,7 +5,6 @@ import {parseRef, WeaveObjectRef} from '@wandb/weave/react'; import React, {useEffect, useMemo, useRef, useState} from 'react'; import {Button} from '../../../../../../../Button'; -import {ShoppingCartItemDefs} from '../../../../compare/types'; import { DEFAULT_FILTER_CALLS, DEFAULT_SORT_CALLS, @@ -15,6 +14,7 @@ import {useEvaluationsFilter} from '../../../CallsPage/evaluationsFilter'; import {Id} from '../../../common/Id'; import {opNiceName} from '../../../common/Links'; import {SortableItems} from '../../../common/shoppingCart/ShoppingCart'; +import {ShoppingCartItemDefs} from '../../../common/shoppingCart/types'; import {useWFHooks} from '../../../wfReactInterface/context'; import { CallSchema, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx index 3592c1d2e6a5..91df551b31c3 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx @@ -7,9 +7,9 @@ import React from 'react'; import {useHistory} from 'react-router-dom'; import {SortableContainer} from 'react-sortable-hoc'; -import {ShoppingCartItemDefs} from '../../../compare/types'; import {queryToggleString, searchParamsSetArray} from '../../../urlQueryUtil'; import {ShoppingCartItem} from './ShoppingCartItem'; +import {ShoppingCartItemDefs} from './types'; type ShoppingCartItemsProps = { items: ShoppingCartItemDefs; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx index a6dea143bf84..e13d5205e561 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx @@ -12,7 +12,7 @@ import {Icon} from '../../../../../../Icon'; import {Pill} from '../../../../../../Tag'; import {Tailwind} from '../../../../../../Tailwind'; import {Tooltip} from '../../../../../../Tooltip'; -import {ShoppingCartItemDef} from '../../../compare/types'; +import {ShoppingCartItemDef} from './types'; // TODO: Change cursor to grabbing when dragging const DragHandle = SortableHandle(() => ( diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts new file mode 100644 index 000000000000..08f474863a48 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts @@ -0,0 +1,7 @@ +export type ShoppingCartItemDef = { + key: string; + value: string; + label?: string; +}; + +export type ShoppingCartItemDefs = ShoppingCartItemDef[]; From 280d00fad4ca57b0ae0a4a71710805f24830eb28 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Dec 2024 09:44:50 -0800 Subject: [PATCH 10/14] undo --- .../compare/ComparePageObjectsLoaded.tsx | 2 +- .../Home/Browse3/compare/ShoppingCart.tsx | 150 ++++++++++++++++++ .../Home/Browse3/compare/ShoppingCartItem.tsx | 137 ++++++++++++++++ .../Home/Browse3/compare/types.ts | 8 + 4 files changed, 296 insertions(+), 1 deletion(-) 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 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 64c79a731693..3b3548f09969 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx @@ -14,7 +14,6 @@ import {Icon} from '../../../../Icon'; import {TailwindContents} from '../../../../Tailwind'; import {isWeaveRef} from '../filters/common'; import {mapObject, TraverseContext} from '../pages/CallPage/traverse'; -import {ShoppingCart} from '../pages/common/shoppingCart/ShoppingCart'; import {SimplePageLayout} from '../pages/common/SimplePageLayout'; import {useWFHooks} from '../pages/wfReactInterface/context'; import { @@ -27,6 +26,7 @@ import {computeDiff, mergeObjects} from './compare'; import {CompareGrid, MAX_OBJECT_COLS} from './CompareGrid'; import {isSequentialVersions, parseSpecifier} from './hooks'; import {getExpandableRefs, RefValues, RESOLVED_REF_KEY} from './refUtil'; +import {ShoppingCart} from './ShoppingCart'; import {ComparableObject, Mode} from './types'; type ComparePageObjectsLoadedProps = { 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/types.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts index e6111427b3ef..26e5e01ab993 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/compare/types.ts @@ -1,3 +1,11 @@ +export type ShoppingCartItemDef = { + key: string; + value: string; + label?: string; +}; + +export type ShoppingCartItemDefs = ShoppingCartItemDef[]; + export type ComparableObject = Record; export type ComparableObjects = ComparableObject[]; From deed09b5a407a0a5adaced3e9f28ff46c817d91f Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Dec 2024 10:10:40 -0800 Subject: [PATCH 11/14] comically better --- .../ComparisonDefinitionSection.tsx | 22 +-- .../DraggableSection/DraggableItem.tsx | 111 +++++++++++++ .../DraggableSection/DraggableSection.tsx | 34 ++++ .../common/shoppingCart/ShoppingCart.tsx | 150 ------------------ .../common/shoppingCart/ShoppingCartItem.tsx | 137 ---------------- .../pages/common/shoppingCart/types.ts | 7 - 6 files changed, 153 insertions(+), 308 deletions(-) create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx delete mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx delete mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx delete mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index fdd04b429b84..d854754ff3cc 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -13,8 +13,6 @@ import {useCallsForQuery} from '../../../CallsPage/callsTableQuery'; import {useEvaluationsFilter} from '../../../CallsPage/evaluationsFilter'; import {Id} from '../../../common/Id'; import {opNiceName} from '../../../common/Links'; -import {SortableItems} from '../../../common/shoppingCart/ShoppingCart'; -import {ShoppingCartItemDefs} from '../../../common/shoppingCart/types'; import {useWFHooks} from '../../../wfReactInterface/context'; import { CallSchema, @@ -28,6 +26,8 @@ import { swapEvaluationCalls, } from '../../ecpState'; import {HorizontalBox} from '../../Layout'; +import {ItemDef} from '../DraggableSection/DraggableItem'; +import {DraggableSection} from '../DraggableSection/DraggableSection'; import {VerticalBar} from './EvaluationDefinition'; export const ComparisonDefinitionSection: React.FC<{ @@ -40,7 +40,7 @@ export const ComparisonDefinitionSection: React.FC<{ return getOrderedCallIds(props.state); }, [props.state]); - const shoppingCartItems: ShoppingCartItemDefs = useMemo(() => { + const items: ItemDef[] = useMemo(() => { return callIds.map(callId => ({ key: 'evaluations', value: callId, @@ -55,11 +55,7 @@ export const ComparisonDefinitionSection: React.FC<{ const newSortOrder = getOrderedEvalsWithNewBaseline(callIds, value); setEvaluationCallOrder(newSortOrder); }; - - const onRemoveShoppingCartItem = (value: string) => { - removeEvaluationCall(value); - }; - + const onRemoveItem = (value: string) => removeEvaluationCall(value); const onSortEnd = ({ oldIndex, newIndex, @@ -74,15 +70,13 @@ export const ComparisonDefinitionSection: React.FC<{ return (
- {}} + state={props.state} + items={items} onSetBaseline={onSetBaseline} - onRemoveShoppingCartItem={onRemoveShoppingCartItem} + onRemoveItem={onRemoveItem} onSortEnd={onSortEnd} /> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx new file mode 100644 index 000000000000..cb8fbf7c2840 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx @@ -0,0 +1,111 @@ +import {Button} from '@wandb/weave/components/Button'; +import * as DropdownMenu from '@wandb/weave/components/DropdownMenu'; +import {Icon} from '@wandb/weave/components/Icon'; +import {Pill} from '@wandb/weave/components/Tag/Pill'; +import {Tailwind} from '@wandb/weave/components/Tailwind'; +import classNames from 'classnames'; +import React, {useState} from 'react'; +import {SortableElement, SortableHandle} from 'react-sortable-hoc'; + +import {EvaluationComparisonState} from '../../ecpState'; +import {EvaluationCallLink} from '../ComparisonDefinitionSection/EvaluationDefinition'; + +export type ItemDef = { + key: string; + value: string; + label?: string; +}; + +type DraggableItemProps = { + state: EvaluationComparisonState; + item: ItemDef; + numItems: number; + idx: number; + onRemoveItem: (value: string) => void; + onSetBaseline: (value: string | null) => void; +}; + +export const DraggableItem = SortableElement( + ({ + state, + item, + numItems, + idx, + onRemoveItem, + onSetBaseline, + }: DraggableItemProps) => { + const isDeletable = numItems > 2; + const isBaseline = idx === 0; + const [isOpen, setIsOpen] = useState(false); + + const onMakeBaselinePropagated = (e: React.MouseEvent) => { + e.stopPropagation(); + onSetBaseline(item.value); + }; + + const onRemoveItemPropagated = (e: React.MouseEvent) => { + e.stopPropagation(); + onRemoveItem(item.value); + }; + + return ( + +
+ +
+ + {isBaseline && ( + + )} +
+ + +
+ + ); + } +); + +const DragHandle = SortableHandle(() => ( +
+ +
+)); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx new file mode 100644 index 000000000000..0cd09c92dd1c --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx @@ -0,0 +1,34 @@ +import React from 'react'; +import {SortableContainer} from 'react-sortable-hoc'; + +import {EvaluationComparisonState} from '../../ecpState'; +import {DraggableItem} from './DraggableItem'; +import {ItemDef} from './DraggableItem'; + +type DraggableSectionProps = { + state: EvaluationComparisonState; + items: ItemDef[]; + onSetBaseline: (value: string | null) => void; + onRemoveItem: (value: string) => void; +}; + +export const DraggableSection = SortableContainer( + ({state, items, onSetBaseline, onRemoveItem}: DraggableSectionProps) => { + return ( +
+ {items.map((item, index) => ( + + ))} +
+ ); + } +); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx deleted file mode 100644 index 91df551b31c3..000000000000 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCart.tsx +++ /dev/null @@ -1,150 +0,0 @@ -/** - * 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; -}; - -export 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/pages/common/shoppingCart/ShoppingCartItem.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx deleted file mode 100644 index e13d5205e561..000000000000 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/ShoppingCartItem.tsx +++ /dev/null @@ -1,137 +0,0 @@ -/** - * 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/pages/common/shoppingCart/types.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts deleted file mode 100644 index 08f474863a48..000000000000 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/shoppingCart/types.ts +++ /dev/null @@ -1,7 +0,0 @@ -export type ShoppingCartItemDef = { - key: string; - value: string; - label?: string; -}; - -export type ShoppingCartItemDefs = ShoppingCartItemDef[]; From 2ec511d782d9cfa7e1f90a67184327e58b8c799f Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Dec 2024 10:15:37 -0800 Subject: [PATCH 12/14] style= -> className= --- .../ComparisonDefinitionSection.tsx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index d854754ff3cc..60529c6d1417 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -257,18 +257,10 @@ const AddEvaluationButton: React.FC<{ className="pb-8 pt-8 font-['Source_Sans_Pro'] text-base font-normal text-moon-800" onClick={() => addEvaluationCall(call.callId)}> <> - + {call.displayName ?? opNiceName(call.spanName)} - + Date: Tue, 10 Dec 2024 14:44:57 -0800 Subject: [PATCH 13/14] fix remove baseline and isDeletable (allow comparison of 1 item) --- .../DraggableSection/DraggableItem.tsx | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx index cb8fbf7c2840..1510c502b990 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableItem.tsx @@ -34,7 +34,7 @@ export const DraggableItem = SortableElement( onRemoveItem, onSetBaseline, }: DraggableItemProps) => { - const isDeletable = numItems > 2; + const isDeletable = numItems > 1; const isBaseline = idx === 0; const [isOpen, setIsOpen] = useState(false); @@ -72,20 +72,12 @@ export const DraggableItem = SortableElement( - {isBaseline ? ( - { - onSetBaseline(null); - }}> - - Remove baseline - - ) : ( - - - Make baseline - - )} + + + Make baseline + {isDeletable && ( <> From c5f3db4513055fec148ea05f9b43f8ab990935fe Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 10 Dec 2024 15:05:30 -0800 Subject: [PATCH 14/14] increase gap from 8 -> 16 to match prod, allow overflow with horizontal scrollbar --- .../ComparisonDefinitionSection.tsx | 23 +++++++++++-------- .../DraggableSection/DraggableSection.tsx | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx index 60529c6d1417..2704a66cbea6 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/ComparisonDefinitionSection.tsx @@ -69,16 +69,19 @@ export const ComparisonDefinitionSection: React.FC<{ return ( -
- +
+ + + diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx index 0cd09c92dd1c..23a03ceb5b3d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/DraggableSection/DraggableSection.tsx @@ -15,7 +15,7 @@ type DraggableSectionProps = { export const DraggableSection = SortableContainer( ({state, items, onSetBaseline, onRemoveItem}: DraggableSectionProps) => { return ( -
+
{items.map((item, index) => (