Skip to content

Commit

Permalink
chore: DataSet refactor3: eliminate id sharing between items and case…
Browse files Browse the repository at this point in the history
…s of the child-most collection (#1360)
  • Loading branch information
kswenson authored Jul 24, 2024
1 parent 15f57c1 commit d26ed76
Show file tree
Hide file tree
Showing 42 changed files with 442 additions and 379 deletions.
4 changes: 2 additions & 2 deletions v3/src/components/case-table/collection-table-spacer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ export const CollectionTableSpacer = observer(function CollectionTableSpacer({ o
// collapse the parent case
caseMetadata?.setIsCollapsed(parentCaseId, !caseMetadata?.isCollapsed(parentCaseId))
// scroll to the first expanded/collapsed child case (if necessary)
const parentPseudoCase = data?.caseGroupMap.get(parentCaseId)
const firstChildId = parentPseudoCase?.childCaseIds?.[0] || parentPseudoCase?.childItemIds?.[0]
const parentCase = data?.caseInfoMap.get(parentCaseId)
const firstChildId = parentCase?.childCaseIds?.[0] || parentCase?.childItemIds?.[0]
const rowIndex = (firstChildId ? childTableModel?.getRowIndexOfCase(firstChildId) : -1) ?? -1
;(rowIndex >= 0) && childTableModel?.scrollRowIntoView(rowIndex)
}
Expand Down
4 changes: 2 additions & 2 deletions v3/src/components/case-table/use-index-column.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ export const useIndexColumn = () => {
// renderer
const RenderIndexCell = useCallback(({ row }: TRenderCellProps) => {
const { __id__, [symIndex]: _index, [symParent]: parentId } = row
const index = _index != null ? _index : data?.caseIndexFromID(__id__)
const index = _index != null ? _index : data?.getItemIndex(__id__)
const collapsedCases = data && parentId && caseMetadata?.isCollapsed(parentId)
? data.caseGroupMap.get(parentId)?.childCaseIds ?? []
? data.caseInfoMap.get(parentId)?.childCaseIds ?? []
: []
const collapsedCaseCount = collapsedCases.length

Expand Down
12 changes: 6 additions & 6 deletions v3/src/components/case-table/use-rows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const useRows = () => {
const domRows = grid?.querySelectorAll(".rdg-row")
domRows?.forEach(row => {
const rowIndex = Number(row.getAttribute("aria-rowindex")) - 2
const caseId = data?.caseIDFromIndex(rowIndex)
const caseId = data?.itemIDFromIndex(rowIndex)
const cells = row.querySelectorAll(".rdg-cell")
cells.forEach(cell => {
const colIndex = Number(cell.getAttribute("aria-colindex")) - 2
Expand Down Expand Up @@ -122,7 +122,7 @@ export const useRows = () => {

// rebuild the entire cache after grouping changes
const reactionDisposer = reaction(
() => data?.isValidCaseGroups && data?.validationCount,
() => data?.isValidCases && data?.validationCount,
validation => {
if (typeof validation === "number") {
resetRowCacheAndSyncRows()
Expand All @@ -143,7 +143,7 @@ export const useRows = () => {
// have to determine the lowest index before the cases are actually removed
lowestIndex.current = Math.min(
...caseIds
.map(id => data.caseIndexFromID(id) ?? -1)
.map(id => data.getItemIndex(id) ?? -1)
.filter(index => index >= 0)
)
}
Expand All @@ -169,7 +169,7 @@ export const useRows = () => {
lowestIndex.current = index != null ? index : data.items.length
const casesToUpdate = []
for (let i=0; i<_cases.length; ++i) {
lowestIndex.current = Math.min(lowestIndex.current, data.caseIndexFromID(_cases[i].__id__) ?? Infinity)
lowestIndex.current = Math.min(lowestIndex.current, data.getItemIndex(_cases[i].__id__) ?? Infinity)
}
for (let j=lowestIndex.current; j < data.items.length; ++j) {
casesToUpdate.push(data.items[j])
Expand Down Expand Up @@ -213,7 +213,7 @@ export const useRows = () => {
const metadataDisposer = caseMetadata && onAnyAction(caseMetadata, action => {
if (isSetIsCollapsedAction(action)) {
const [caseId] = action.args
const caseGroup = data?.caseGroupMap.get(caseId)
const caseGroup = data?.caseInfoMap.get(caseId)
const childCaseIds = caseGroup?.childCaseIds ?? caseGroup?.childItemIds
const firstChildCaseId = childCaseIds?.[0]
if (firstChildCaseId) {
Expand Down Expand Up @@ -313,7 +313,7 @@ export const useRows = () => {
notifications: () => {
const notifications = []
if (updatedCaseIds.length > 0) {
const updatedCases = updatedCaseIds.map(caseId => data.caseGroupMap.get(caseId))
const updatedCases = updatedCaseIds.map(caseId => data.caseInfoMap.get(caseId))
.filter(caseGroup => !!caseGroup)
.map(caseGroup => caseGroup.groupedCase)
notifications.push(updateCasesNotification(data, updatedCases))
Expand Down
33 changes: 10 additions & 23 deletions v3/src/components/data-display/hooks/use-connecting-lines.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { curveLinear, line, select } from "d3"
import { tip as d3tip } from "d3-v6-tip"
import { useCallback } from "react"
import { setSelectedCases } from "../../../models/data/data-set-utils"
import { selectCases, setSelectedCases } from "../../../models/data/data-set-utils"
import { t } from "../../../utilities/translation/translate"
import { IConnectingLineDescription, transitionDuration } from "../data-display-types"
import { PixiBackgroundPassThroughEvent, PixiPoints } from "../pixi/pixi-points"
Expand Down Expand Up @@ -62,26 +62,12 @@ export const useConnectingLines = (props: IProps) => {

const linesPath = event.target && select(event.target as HTMLElement)
if (linesPath) {
const alreadySelectedCases = Array.from(dataset?.selection ?? [])
const isSelected = linesPath.classed("selected")
// If the line is already selected and `caseIDs` contains exactly the same as the set of IDs as
// `alreadySelectedCases`, we'll deselect the line. Otherwise, we'll select the line and deselect all others.
const caseIDsSet = new Set(caseIDs)
const lineCasesAlreadySelected = alreadySelectedCases.length === caseIDs.length &&
alreadySelectedCases.every(caseID => caseIDsSet.has(caseID))
const shouldDeselect = isSelected && lineCasesAlreadySelected
const selected = shouldDeselect ? false : !isSelected
const strokeWidth = shouldDeselect ? 2 : 4
linesPath.classed("selected", selected).attr("stroke-width", strokeWidth)
let newSelection: string[] = []
if (!shouldDeselect) {
if (event.shiftKey) {
newSelection = [...caseIDs, ...alreadySelectedCases]
} else {
newSelection = caseIDs
}
const areLineCasesSelected = caseIDs.every(caseID => dataset?.isCaseSelected(caseID))
if (areLineCasesSelected || event.shiftKey) {
selectCases(caseIDs, dataset, !areLineCasesSelected)
} else {
setSelectedCases(caseIDs, dataset)
}
setSelectedCases(newSelection, dataset)
}
}, [dataset, onConnectingLinesClick])

Expand Down Expand Up @@ -116,7 +102,7 @@ export const useConnectingLines = (props: IProps) => {
for (const [linesIndex, [primaryAttrValue, cases]] of Object.entries(lineGroups).entries()) {
const allLineCoords = cases.map((l: any) => l.lineCoords)
const lineCaseIds = allLineCaseIds[primaryAttrValue]
const allCasesSelected = lineCaseIds?.every((caseID: string) => dataConfig?.selection.includes(caseID))
const allCasesSelected = lineCaseIds?.every((caseID: string) => dataset?.isCaseSelected(caseID))
const legendID = dataConfig?.attributeID("legend")
const color = parentAttrID && legendID ? pointColorAtIndex(linesIndex) : pointColorAtIndex(0)

Expand All @@ -135,6 +121,7 @@ export const useConnectingLines = (props: IProps) => {
.attr("fill", "none")
.attr("stroke", color)
.attr("stroke-width", allCasesSelected ? 4 : 2)
.attr("stroke-linejoin", "round")
.style("cursor", "pointer")
.style("opacity", connectingLinesActivatedRef.current ? 1 : 0)
.transition()
Expand All @@ -145,8 +132,8 @@ export const useConnectingLines = (props: IProps) => {
!showConnectingLines && connectingLinesArea.selectAll("path").remove()
})
}
}, [clientType, connectingLinesActivatedRef, connectingLinesArea, dataConfig, dataTip, handleConnectingLinesClick,
handleConnectingLinesMouseOut, handleConnectingLinesMouseOver])
}, [clientType, connectingLinesActivatedRef, connectingLinesArea, dataConfig, dataTip, dataset,
handleConnectingLinesClick, handleConnectingLinesMouseOut, handleConnectingLinesMouseOver])

const prepareConnectingLines = useCallback((prepareLineProps: IPrepareLineProps) => {
const { connectingLines, parentAttrID, cellKey, parentAttrName, showConnectingLines } = prepareLineProps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {CaseData} from "../d3-types"
import {AttrRole, TipAttrRoles, graphPlaceToAttrRole} from "../data-display-types"
import {GraphPlace} from "../../axis-graph-shared"
import { numericSortComparator } from "../../../utilities/data-utils"
import { isFiniteNumber } from "../../../utilities/math-utils"

export const AttributeDescription = types
.model('AttributeDescription', {
Expand Down Expand Up @@ -188,7 +189,7 @@ export const DataConfigurationModel = types
if (["caption", "legend"].includes(role)) return true
switch (self.attributeType(role as AttrRole)) {
case "numeric":
return isFinite(data.getNumeric(caseID, attributeID) ?? NaN)
return isFiniteNumber(data.getNumeric(caseID, attributeID))
default:
// for now, all other types must just be non-empty
return !!data.getValue(caseID, attributeID)
Expand Down
107 changes: 57 additions & 50 deletions v3/src/components/graph/models/graph-data-configuration-model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ describe("DataConfigurationModel", () => {
]))
})

const caseIdFromItemId = (itemId: string) => tree.data.getItemChildCaseId(itemId)
const caseIdsFromItemIds = (itemIds: string[]) => itemIds.map(caseIdFromItemId)

it("behaves as expected when empty", () => {
const config = tree.config
expect(config.attributeID('caption')).toBe('')
Expand Down Expand Up @@ -62,9 +65,9 @@ describe("DataConfigurationModel", () => {
expect(config.tipAttributes).toEqual([{attributeID: "nId", role: "caption"}])
expect(config.uniqueTipAttributes).toEqual([{attributeID: "nId", role: "caption"}])
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c2"},
{plotNum: 0, caseID: "c3"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c2")},
{plotNum: 0, caseID: caseIdFromItemId("c3")}
])
})

Expand All @@ -87,8 +90,8 @@ describe("DataConfigurationModel", () => {
{attributeID: "nId", role: "caption"}])
expect(config.uniqueTipAttributes).toEqual([{attributeID: "nId", role: "caption"}])
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c3"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c3")}
])
})

Expand All @@ -111,8 +114,8 @@ describe("DataConfigurationModel", () => {
expect(config.uniqueTipAttributes).toEqual([{attributeID: "xId", role: "x"},
{attributeID: "nId", role: "caption"}])
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c2"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c2")}
])
})

Expand All @@ -138,7 +141,7 @@ describe("DataConfigurationModel", () => {
{attributeID: "yId", role: "y"}, {attributeID: "nId", role: "caption"}])
expect(config.uniqueTipAttributes).toEqual([{attributeID: "xId", role: "x"},
{attributeID: "yId", role: "y"}, {attributeID: "nId", role: "caption"}])
expect(config.caseDataArray).toEqual([{plotNum: 0, caseID: "c1"}])
expect(config.caseDataArray).toEqual([{plotNum: 0, caseID: caseIdFromItemId("c1")}])

// behaves as expected after adding "x" as an additional y attribute
config.addYAttribute({ attributeID: "xId" })
Expand Down Expand Up @@ -171,16 +174,16 @@ describe("DataConfigurationModel", () => {
expect(config.uniqueTipAttributes).toEqual([{attributeID: "yId", role: "y"},
{attributeID: "nId", role: "caption"}])
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c3"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c3")}
])

// updates cases when values change
tree.data.setCaseValues([{ __id__: "c2", "yId": 2 }])
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c2"},
{plotNum: 0, caseID: "c3"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c2")},
{plotNum: 0, caseID: caseIdFromItemId("c3")}
])

// triggers observers when values change
Expand All @@ -193,16 +196,16 @@ describe("DataConfigurationModel", () => {
tree.data.setCaseValues([{ __id__: "c2", "yId": "" }])
expect(trigger).toHaveBeenCalled()
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c3"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c3")}
])
trigger.mockClear()
tree.data.setCaseValues([{ __id__: "c2", "yId": "2" }])
expect(trigger).toHaveBeenCalled()
expect(config.caseDataArray).toEqual([
{plotNum: 0, caseID: "c1"},
{plotNum: 0, caseID: "c2"},
{plotNum: 0, caseID: "c3"}
{plotNum: 0, caseID: caseIdFromItemId("c1")},
{plotNum: 0, caseID: caseIdFromItemId("c2")},
{plotNum: 0, caseID: caseIdFromItemId("c3")}
])
})

Expand Down Expand Up @@ -238,22 +241,26 @@ describe("DataConfigurationModel", () => {
const handleAction = jest.fn()
config.onAction(handleAction)

tree.data.setCaseValues([{ __id__: "c1", xId: 1.1 }])
tree.data.setCaseValues([{ __id__: caseIdFromItemId("c1")!, xId: 1.1 }])
expect(handleAction).toHaveBeenCalled()
expect(handleAction.mock.lastCall[0].name).toBe("setCaseValues")
handleAction.mockClear()

tree.data.setCaseValues([{ __id__: "c3", xId: 3 }])
tree.data.setCaseValues([{ __id__: caseIdFromItemId("c3")!, xId: 3 }])
expect(handleAction).toHaveBeenCalled()
expect(handleAction.mock.lastCall[0].name).toBe("addCases")
handleAction.mockClear()

tree.data.setCaseValues([{ __id__: "c1", xId: "" }])
tree.data.setCaseValues([{ __id__: caseIdFromItemId("c1")!, xId: "" }])
expect(handleAction).toHaveBeenCalled()
expect(handleAction.mock.lastCall[0].name).toBe("removeCases")
handleAction.mockClear()

tree.data.setCaseValues([{ __id__: "c1", xId: 1 }, { __id__: "c2", xId: "" }, { __id__: "c3", xId: 3.3 }])
tree.data.setCaseValues([
{ __id__: caseIdFromItemId("c1")!, xId: 1 },
{ __id__: caseIdFromItemId("c2")!, xId: "" },
{ __id__: caseIdFromItemId("c3")!, xId: 3.3 }
])
expect(handleAction).toHaveBeenCalled()
})

Expand Down Expand Up @@ -293,46 +300,46 @@ describe("DataConfigurationModel", () => {
it("returns an array of cases in a plot", () => {
const config = tree.config
config.setDataset(tree.data, tree.metadata)
expect(config.allPlottedCases()).toEqual(["c1", "c2", "c3"])
expect(config.subPlotCases({})).toEqual(["c1", "c2", "c3"])
expect(config.cellCases({})).toEqual(["c1", "c2", "c3"])
expect(config.rowCases({})).toEqual(["c1", "c2", "c3"])
expect(config.columnCases({})).toEqual(["c1", "c2", "c3"])
expect(config.allPlottedCases()).toEqual(caseIdsFromItemIds(["c1", "c2", "c3"]))
expect(config.subPlotCases({})).toEqual(caseIdsFromItemIds(["c1", "c2", "c3"]))
expect(config.cellCases({})).toEqual(caseIdsFromItemIds(["c1", "c2", "c3"]))
expect(config.rowCases({})).toEqual(caseIdsFromItemIds(["c1", "c2", "c3"]))
expect(config.columnCases({})).toEqual(caseIdsFromItemIds(["c1", "c2", "c3"]))

config.setAttribute("x", { attributeID: "xId" })
expect(config.allPlottedCases()).toEqual(["c1", "c2"])
expect(config.subPlotCases({})).toEqual(["c1", "c2"])
expect(config.cellCases({})).toEqual(["c1", "c2"])
expect(config.rowCases({})).toEqual(["c1", "c2"])
expect(config.columnCases({})).toEqual(["c1", "c2"])
expect(config.allPlottedCases()).toEqual(caseIdsFromItemIds(["c1", "c2"]))
expect(config.subPlotCases({})).toEqual(caseIdsFromItemIds(["c1", "c2"]))
expect(config.cellCases({})).toEqual(caseIdsFromItemIds(["c1", "c2"]))
expect(config.rowCases({})).toEqual(caseIdsFromItemIds(["c1", "c2"]))
expect(config.columnCases({})).toEqual(caseIdsFromItemIds(["c1", "c2"]))

config.setAttribute("y", { attributeID: "yId" })
expect(config.allPlottedCases()).toEqual(["c1"])
expect(config.subPlotCases({})).toEqual(["c1"])
expect(config.cellCases({})).toEqual(["c1"])
expect(config.rowCases({})).toEqual(["c1"])
expect(config.columnCases({})).toEqual(["c1"])
expect(config.allPlottedCases()).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.subPlotCases({})).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.cellCases({})).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.rowCases({})).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.columnCases({})).toEqual(caseIdsFromItemIds(["c1"]))

config.setAttribute("topSplit", { attributeID: "nId" })
expect(config.allPlottedCases()).toEqual(["c1"])
expect(config.subPlotCases({})).toEqual(["c1"])
expect(config.cellCases({ nId: "n1" })).toEqual(["c1"])
expect(config.allPlottedCases()).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.subPlotCases({})).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.cellCases({ nId: "n1" })).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.rowCases({})).toEqual([])
expect(config.rowCases({ nId: "n1" })).toEqual(["c1"])
expect(config.rowCases({ nId: "n1" })).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.columnCases({})).toEqual([])
expect(config.columnCases({ nId: "n1" })).toEqual(["c1"])
expect(config.columnCases({ nId: "n1" })).toEqual(caseIdsFromItemIds(["c1"]))

config.setAttribute("x")
expect(config.allPlottedCases()).toEqual(["c1", "c3"])
expect(config.subPlotCases({})).toEqual(["c1", "c3"])
expect(config.cellCases({ nId: "n1" })).toEqual(["c1"])
expect(config.cellCases({ nId: "n3" })).toEqual(["c3"])
expect(config.allPlottedCases()).toEqual(caseIdsFromItemIds(["c1", "c3"]))
expect(config.subPlotCases({})).toEqual(caseIdsFromItemIds(["c1", "c3"]))
expect(config.cellCases({ nId: "n1" })).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.cellCases({ nId: "n3" })).toEqual(caseIdsFromItemIds(["c3"]))
expect(config.rowCases({})).toEqual([])
expect(config.rowCases({ nId: "n1" })).toEqual(["c1"])
expect(config.rowCases({ nId: "n3" })).toEqual(["c3"])
expect(config.rowCases({ nId: "n1" })).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.rowCases({ nId: "n3" })).toEqual(caseIdsFromItemIds(["c3"]))
expect(config.columnCases({})).toEqual([])
expect(config.columnCases({ nId: "n1" })).toEqual(["c1"])
expect(config.columnCases({ nId: "n3" })).toEqual(["c3"])
expect(config.columnCases({ nId: "n1" })).toEqual(caseIdsFromItemIds(["c1"]))
expect(config.columnCases({ nId: "n3" })).toEqual(caseIdsFromItemIds(["c3"]))
})

it("can create cell key", () => {
Expand Down
Loading

0 comments on commit d26ed76

Please sign in to comment.