Skip to content

Commit

Permalink
CR feedback - Tim
Browse files Browse the repository at this point in the history
  • Loading branch information
@jamie-rasmussen
jamie-rasmussen committed Nov 20, 2024
1 parent f4ec251 commit 7a249f3
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 87 deletions.
100 changes: 51 additions & 49 deletions weave-js/src/components/PagePanelComponents/Home/Browse3/compare/CompareGrid.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ type CompareGridProps = {
mode: Mode;
baselineEnabled: boolean;
onlyChanged: boolean;
isExpanded?: boolean;

expandedIds: GridRowId[];
setExpandedIds: React.Dispatch<React.SetStateAction<GridRowId[]>>;
addExpandedRefs: (path: string, refs: string[]) => void;
};

export const MAX_OBJECT_COLS = 6;

const objectVersionSchemaToRef = (
objVersion: ObjectVersionSchema
): WeaveObjectRef => {
Expand All @@ -60,7 +61,6 @@ export const CompareGrid = ({
mode,
baselineEnabled,
onlyChanged,
isExpanded,
expandedIds,
setExpandedIds,
addExpandedRefs,
Expand Down Expand Up @@ -112,53 +112,55 @@ export const CompareGrid = ({
},
});
} else {
const versionCols: GridColDef[] = objectIds.map(objId => ({
field: objId,
headerName: objId,
flex: 1,
display: 'flex',
width: 500,
sortable: false,
valueGetter: (unused: any, row: any) => {
return row.values[objId];
},
renderHeader: (params: any) => {
if (objectType === 'call') {
// TODO: Make this a peek drawer link
return objId;
}
const idx = objectIds.indexOf(objId);
const objVersion = objects[idx];
const objRef = objectVersionSchemaToRef(
objVersion as ObjectVersionSchema
);
return <SmallRef objRef={objRef} />;
},
renderCell: (cellParams: any) => {
const compareIdx = baselineEnabled
? 0
: Math.max(0, objectIds.indexOf(objId) - 1);
const compareId = objectIds[compareIdx];
const compareValue = cellParams.row.values[compareId];
const compareValueType = cellParams.row.types[compareId];
const value = cellParams.row.values[objId];
const valueType = cellParams.row.types[objId];
const rowChangeType = cellParams.row.changeType;
return (
<div className="w-full p-8">
<CompareGridCell
path={cellParams.row.path}
displayType="diff"
value={value}
valueType={valueType}
compareValue={compareValue}
compareValueType={compareValueType}
rowChangeType={rowChangeType}
/>
</div>
);
},
}));
const versionCols: GridColDef[] = objectIds
.slice(0, MAX_OBJECT_COLS)
.map(objId => ({
field: objId,
headerName: objId,
flex: 1,
display: 'flex',
width: 500,
sortable: false,
valueGetter: (unused: any, row: any) => {
return row.values[objId];
},
renderHeader: (params: any) => {
if (objectType === 'call') {
// TODO: Make this a peek drawer link
return objId;
}
const idx = objectIds.indexOf(objId);
const objVersion = objects[idx];
const objRef = objectVersionSchemaToRef(
objVersion as ObjectVersionSchema
);
return <SmallRef objRef={objRef} />;
},
renderCell: (cellParams: any) => {
const compareIdx = baselineEnabled
? 0
: Math.max(0, objectIds.indexOf(objId) - 1);
const compareId = objectIds[compareIdx];
const compareValue = cellParams.row.values[compareId];
const compareValueType = cellParams.row.types[compareId];
const value = cellParams.row.values[objId];
const valueType = cellParams.row.types[objId];
const rowChangeType = cellParams.row.changeType;
return (
<div className="w-full p-8">
<CompareGridCell
path={cellParams.row.path}
displayType="diff"
value={value}
valueType={valueType}
compareValue={compareValue}
compareValueType={compareValueType}
rowChangeType={rowChangeType}
/>
</div>
);
},
}));
columns.push(...versionCols);
}

Expand Down
83 changes: 45 additions & 38 deletions ...e-js/src/components/PagePanelComponents/Home/Browse3/compare/ComparePageObjectsLoaded.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
queryToggleBoolean,
} from '../urlQueryUtil';
import {computeDiff, mergeObjects} from './compare';
import {CompareGrid} from './CompareGrid';
import {CompareGrid, MAX_OBJECT_COLS} from './CompareGrid';
import {isSequentialVersions, parseSpecifier} from './hooks';
import {getExpandableRefs, RefValues, RESOLVED_REF_KEY} from './refUtil';
import {ShoppingCart} from './ShoppingCart';
Expand Down Expand Up @@ -262,6 +262,8 @@ export const ComparePageObjectsLoaded = ({
});
};

const tooManyCols = objectIds.length > MAX_OBJECT_COLS && !selected;

return (
<SimplePageLayout
title={title}
Expand All @@ -279,7 +281,7 @@ export const ComparePageObjectsLoaded = ({
icon="back"
variant="ghost"
onClick={onPrev}
tooltip="View previous versions"
tooltip="Decrement the selected versions of this object to view earlier changes"
/>
)}
<ShoppingCart
Expand All @@ -297,7 +299,7 @@ export const ComparePageObjectsLoaded = ({
icon="forward-next"
variant="ghost"
onClick={onNext}
tooltip="View subsequent versions"
tooltip="Increment the selected versions of this object to view later changes"
/>
)}
</div>
Expand Down Expand Up @@ -338,43 +340,48 @@ export const ComparePageObjectsLoaded = ({
</Button>
</div>
)}
{tooManyCols && (
<div>Viewing first {MAX_OBJECT_COLS} columns only</div>
)}
<div className="flex-grow" />
<DropdownMenu.Root open={isOpen} onOpenChange={setIsOpen}>
<DropdownMenu.Trigger>
<div className="cursor-pointer">
<div className="flex items-center gap-10">
{baselineEnabled
? 'Compare with baseline'
: 'Compare with previous'}{' '}
<Icon name="chevron-down" />
{objectIds.length > 2 && (
<DropdownMenu.Root open={isOpen} onOpenChange={setIsOpen}>
<DropdownMenu.Trigger>
<div className="cursor-pointer">
<div className="flex items-center gap-10">
{baselineEnabled
? 'Compare with baseline'
: 'Compare with previous'}{' '}
<Icon name="chevron-down" />
</div>
</div>
</div>
</DropdownMenu.Trigger>
<DropdownMenu.Portal>
<DropdownMenu.Content align="start">
<DropdownMenu.Item
className="select-none"
onClick={() => onSetBaseline(false)}>
{!baselineEnabled ? (
<Icon name="checkmark" />
) : (
<IconPlaceholder />
)}{' '}
Compare with previous
</DropdownMenu.Item>
<DropdownMenu.Item
className="select-none"
onClick={() => onSetBaseline(true)}>
{baselineEnabled ? (
<Icon name="checkmark" />
) : (
<IconPlaceholder />
)}{' '}
Compare with baseline
</DropdownMenu.Item>
</DropdownMenu.Content>
</DropdownMenu.Portal>
</DropdownMenu.Root>
</DropdownMenu.Trigger>
<DropdownMenu.Portal>
<DropdownMenu.Content align="start">
<DropdownMenu.Item
className="select-none"
onClick={() => onSetBaseline(false)}>
{!baselineEnabled ? (
<Icon name="checkmark" />
) : (
<IconPlaceholder />
)}{' '}
Compare with previous
</DropdownMenu.Item>
<DropdownMenu.Item
className="select-none"
onClick={() => onSetBaseline(true)}>
{baselineEnabled ? (
<Icon name="checkmark" />
) : (
<IconPlaceholder />
)}{' '}
Compare with baseline
</DropdownMenu.Item>
</DropdownMenu.Content>
</DropdownMenu.Portal>
</DropdownMenu.Root>
)}
</div>
<div className="overflow-auto px-16">
<CompareGrid
Expand Down

0 comments on commit 7a249f3

Please sign in to comment.