From caa29c33dd48385d1d126250c5091f33a298978b Mon Sep 17 00:00:00 2001 From: lublagg Date: Mon, 30 Sep 2024 17:31:34 -0400 Subject: [PATCH 1/5] Fix selection issues + button text size. --- v3/src/components/case-card/card-view.scss | 2 +- v3/src/components/case-card/card-view.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/v3/src/components/case-card/card-view.scss b/v3/src/components/case-card/card-view.scss index 17f0ff55e..c9886b3be 100644 --- a/v3/src/components/case-card/card-view.scss +++ b/v3/src/components/case-card/card-view.scss @@ -30,7 +30,7 @@ color: #0592af; cursor: pointer; display: block; - font-size: 10px; + font-size: 12px; font-weight: bold; margin: 0 auto; padding: 6px 8px; diff --git a/v3/src/components/case-card/card-view.tsx b/v3/src/components/case-card/card-view.tsx index 4ab4bf22c..5edd92576 100644 --- a/v3/src/components/case-card/card-view.tsx +++ b/v3/src/components/case-card/card-view.tsx @@ -46,7 +46,7 @@ export const CardView = observer(function CardView({onNewCollectionDrop}: CardVi return mstReaction( () => selectedItems?.size, () => { - if (selectedItems?.size === data?.items.length) { + if (selectedItems?.size === data?.items.length || selectedItems?.size === 0) { cardModel?.summarizeAllCollections() } }, {name: "CardView.showSummaryOnAllCasesSelection"}, data From a5380e0c769516c7739a9148f8c8b24127123abf Mon Sep 17 00:00:00 2001 From: lublagg Date: Tue, 1 Oct 2024 17:46:54 -0400 Subject: [PATCH 2/5] Refactor summary from prop to view on case card model. --- v3/src/components/case-card/card-view.tsx | 43 ++++----- .../components/case-card/case-card-model.ts | 88 ++++++++++--------- v3/src/components/case-card/case-view.tsx | 10 +-- 3 files changed, 68 insertions(+), 73 deletions(-) diff --git a/v3/src/components/case-card/card-view.tsx b/v3/src/components/case-card/card-view.tsx index 5edd92576..de4259636 100644 --- a/v3/src/components/case-card/card-view.tsx +++ b/v3/src/components/case-card/card-view.tsx @@ -1,6 +1,5 @@ -import React, { useEffect, useRef } from "react" +import React, { useRef, useState } from "react" import { observer } from "mobx-react-lite" -import { mstReaction } from "../../utilities/mst-reaction" import { CollectionContext } from "../../hooks/use-collection-context" import { AttributeHeaderDividerContext } from "../case-tile-common/use-attribute-header-divider-context" import { CaseView } from "./case-view" @@ -18,40 +17,32 @@ export const CardView = observer(function CardView({onNewCollectionDrop}: CardVi const cardModel = useCaseCardModel() const data = cardModel?.data const collections = data?.collections - const areAllCollectionsSummarized = !!collections?.every(c => cardModel?.summarizedCollections.includes(c.id)) const rootCollection = collections?.[0] const selectedItems = data?.selection const selectedItemId = selectedItems && Array.from(selectedItems)[0] const selectedItemLineage = cardModel?.caseLineage(selectedItemId) const contentRef = useRef(null) + const summarizedCollections = cardModel?.summarizedCollections || [] + const [isInSummaryMode, setIsInSummaryMode] = useState(summarizedCollections.length !== 0) - const handleSelectCases = (caseIds: string[], collectionId: string) => { - cardModel?.setShowSummary(caseIds.length > 1, collectionId) + const handleSelectCases = (caseIds: string[]) => { data?.setSelectedCases(caseIds) } const handleSummaryButtonClick = () => { - cardModel?.setShowSummary(!areAllCollectionsSummarized) - } - - // The first time the card is rendered, summarize all collections unless there is a selection. - useEffect(function startWithAllCollectionsSummarized() { - if (data?.selection.size === 0) { - cardModel?.summarizeAllCollections() + if (isInSummaryMode) { + // select the first child-most case + const firstItemId = data?.itemIds[0] + const firstItemLineage = cardModel?.caseLineage(firstItemId) + if (firstItemLineage) { + data?.setSelectedCases([firstItemLineage[firstItemLineage.length - 1]]) + } + setIsInSummaryMode(false) + } else { + data?.setSelectedCases([]) + setIsInSummaryMode(true) } - }, [cardModel, data]) - - // When all cases are selected, show summary. - useEffect(function showSummaryOnAllCasesSelection() { - return mstReaction( - () => selectedItems?.size, - () => { - if (selectedItems?.size === data?.items.length || selectedItems?.size === 0) { - cardModel?.summarizeAllCollections() - } - }, {name: "CardView.showSummaryOnAllCasesSelection"}, data - ) - }, [cardModel, data, selectedItems]) + } return (
@@ -72,7 +63,7 @@ export const CardView = observer(function CardView({onNewCollectionDrop}: CardVi onClick={handleSummaryButtonClick} disabled={data?.items.length === 0} > - { areAllCollectionsSummarized + { isInSummaryMode ? t("V3.caseCard.summaryButton.showIndividualCases") : t("V3.caseCard.summaryButton.showSummary") } diff --git a/v3/src/components/case-card/case-card-model.ts b/v3/src/components/case-card/case-card-model.ts index 744aa0f45..68342f0d4 100644 --- a/v3/src/components/case-card/case-card-model.ts +++ b/v3/src/components/case-card/case-card-model.ts @@ -1,6 +1,5 @@ import { Instance, SnapshotIn, types } from "mobx-state-tree" import { getTileCaseMetadata, getTileDataSet } from "../../models/shared/shared-data-utils" -import { ISharedModel } from "../../models/shared/shared-model" import { ITileContentModel, TileContentModel } from "../../models/tiles/tile-content" import { kCaseCardTileType } from "./case-card-defs" import { ICollectionModel } from "../../models/data/collection" @@ -13,7 +12,6 @@ export const CaseCardModel = TileContentModel type: types.optional(types.literal(kCaseCardTileType), kCaseCardTileType), // key is collection id; value is width attributeColumnWidths: types.map(types.number), - summarizedCollections: types.optional(types.array(types.string), []) }) .views(self => ({ get data() { @@ -26,6 +24,40 @@ export const CaseCardModel = TileContentModel return self.attributeColumnWidths.get(collectionId) } })) + .views(self => ({ + get summarizedCollections() { + const selectedItems = self.data?.selection + const items = self.data?.items + const collections = self.data?.collections + + if (!collections || !items || selectedItems?.size === 1) { + return [] + } else if (!selectedItems || selectedItems.size === 0 || selectedItems.size === items.length) { + return collections?.map(c => c.id) ?? [] + } else { + const collectionIdsToSummarize: string[] = [] + const summarizeCollectionsRecursively = (index = 0) => { + if (index >= collections.length - 1) return + const collection = collections[index] + const collectionCases = collection.cases + const selectedChildrenPerCase = collectionCases.map(collectionCase => { + const caseGroup = self.data?.caseInfoMap.get(collectionCase.__id__) + if (!caseGroup?.childCaseIds) return 0 + return caseGroup.childCaseIds.filter(childCaseId => self.data?.isCaseSelected(childCaseId)).length + }) + if (selectedChildrenPerCase.filter(v => v > 0).length > 1) { + collectionIdsToSummarize.push(collection.id) + } + summarizeCollectionsRecursively(index + 1) + } + + summarizeCollectionsRecursively() + // always summarize the last collection + collectionIdsToSummarize.push(collections[collections.length - 1].id) + return collectionIdsToSummarize + } + } + })) .views(self => ({ caseLineage(itemId?: string) { if (!itemId) return undefined @@ -35,8 +67,8 @@ export const CaseCardModel = TileContentModel const parentCaseInfo = self.data?.caseInfoMap.get(parentCaseId) if (!parentCaseInfo?.childCaseIds) return undefined return parentCaseInfo.childCaseIds - .map(childCaseId => self.data?.caseInfoMap.get(childCaseId)?.groupedCase) - .filter(groupedCase => !!groupedCase) + .map(childCaseId => self.data?.caseInfoMap.get(childCaseId)?.groupedCase) + .filter(groupedCase => !!groupedCase) }, displayValues(collection: ICollectionModel, caseItem: IGroupedCase) { @@ -44,10 +76,10 @@ export const CaseCardModel = TileContentModel const minValue = Math.min(...numericValues) const maxValue = Math.max(...numericValues) return minValue === maxValue - ? `${minValue}${attrUnits ? ` ${attrUnits}` : ""}` - : `${minValue}-${maxValue}${attrUnits ? ` ${attrUnits}` : ""}` + ? `${minValue}${attrUnits ? ` ${attrUnits}` : ""}` + : `${minValue}-${maxValue}${attrUnits ? ` ${attrUnits}` : ""}` } - + const getCategoricalSummary = (uniqueValues: Set): string => { const uniqueValuesArray = Array.from(uniqueValues) if (uniqueValuesArray.length === 1) { @@ -58,15 +90,15 @@ export const CaseCardModel = TileContentModel return `${uniqueValuesArray.length} values` } } - + if (self.summarizedCollections.includes(collection.id)) { const summaryMap = collection?.attributes.reduce((acc: Record, attr) => { if (!attr || !attr.id) return acc const selectedCases = self.data?.selection const casesToUse = selectedCases && selectedCases.size >= 1 - ? Array.from(selectedCases).map((id) => ({ __id__: id })) - : collection.cases + ? Array.from(selectedCases).map((id) => ({ __id__: id })) + : collection.cases const allValues = casesToUse.map(c => self.data?.getValue(c.__id__, attr.id)) const uniqueValues = new Set(allValues) const isNumeric = attr.numValues?.some((v, i) => attr.isNumeric(i)) @@ -78,7 +110,7 @@ export const CaseCardModel = TileContentModel summary = getNumericSummary(numericValues, attrUnits) } else { summary = getCategoricalSummary(uniqueValues) - } + } return { ...acc, [attr.id]: summary } }, {}) return collection?.attributes.map(attr => attr?.id && summaryMap[attr.id]) ?? [] @@ -87,11 +119,6 @@ export const CaseCardModel = TileContentModel } } })) - .actions(self => ({ - setSummarizedCollections(collections: string[]) { - self.summarizedCollections.replace(collections) - } - })) .actions(self => ({ setAttributeColumnWidth(collectionId: string, width?: number) { if (width) { @@ -116,33 +143,12 @@ export const CaseCardModel = TileContentModel // TODO: Figure out why this call to validateCases is necessary. The Cypress test for adding a case // fails if it isn't here, though adding a case seems to work fine when tested manually. self.data?.validateCases() - - return newCaseId - }, - setShowSummary(show: boolean, collectionId?: string) { - if (show) { - self.data?.setSelectedCases([]) - } - - const updatedSummarizedCollections = show - ? collectionId - ? [...self.summarizedCollections, collectionId] - : self.data?.collections.map(c => c.id) ?? [] - : collectionId - ? self.summarizedCollections.filter(cid => cid !== collectionId) - : [] - self.setSummarizedCollections(updatedSummarizedCollections) - }, - summarizeAllCollections() { - self.setSummarizedCollections(self.data?.collections.map(c => c.id) ?? []) - }, - updateAfterSharedModelChanges(sharedModel?: ISharedModel) { - // TODO - }, + return newCaseId + } })) -export interface ICaseCardModel extends Instance {} -export interface ICaseCardSnapshot extends SnapshotIn {} +export interface ICaseCardModel extends Instance { } +export interface ICaseCardSnapshot extends SnapshotIn { } export function isCaseCardModel(model?: ITileContentModel): model is ICaseCardModel { return model?.type === kCaseCardTileType diff --git a/v3/src/components/case-card/case-view.tsx b/v3/src/components/case-card/case-view.tsx index 4c87e94f2..bd9293f1f 100644 --- a/v3/src/components/case-card/case-view.tsx +++ b/v3/src/components/case-card/case-view.tsx @@ -22,7 +22,7 @@ import "./case-view.scss" interface ICaseViewProps { cases: IGroupedCase[] level: number - onSelectCases: (caseIds: string[], collection: string) => void + onSelectCases: (caseIds: string[]) => void displayedCaseLineage?: readonly string[] onNewCollectionDrop: (dataSet: IDataSet, attrId: string, beforeCollectionId: string) => void } @@ -69,9 +69,8 @@ export const CaseView = observer(function CaseView(props: ICaseViewProps) { : displayedCaseIndex + delta const newCase = cases[selectedCaseIndex] if (!newCase.__id__) return - - onSelectCases([newCase.__id__], collectionId) - }, [isCollectionSummarized, cases, displayedCaseIndex, onSelectCases, collectionId]) + onSelectCases([newCase.__id__]) + }, [isCollectionSummarized, cases, displayedCaseIndex, onSelectCases]) const handleAddNewCase = () => { if (collection) { @@ -79,7 +78,7 @@ export const CaseView = observer(function CaseView(props: ICaseViewProps) { data?.applyModelChange(() => { const newItemId = cardModel?.addNewCase(cases, collection, displayedCaseId) newCaseId = newItemId && data?.getItemCaseIds(newItemId)[level] - newCaseId && onSelectCases([newCaseId], collectionId) + newCaseId && onSelectCases([newCaseId]) }, { notify: () => { if (newCaseId) { @@ -89,7 +88,6 @@ export const CaseView = observer(function CaseView(props: ICaseViewProps) { undoStringKey: "DG.Undo.caseTable.createNewCase", redoStringKey: "DG.Redo.caseTable.createNewCase" }) - cardModel?.setShowSummary(false) } } From 2750d7882e4aff464a28057f646f73df3b616e68 Mon Sep 17 00:00:00 2001 From: lublagg Date: Wed, 2 Oct 2024 15:11:49 -0400 Subject: [PATCH 3/5] Refactor addNewCase to use selectedCases. --- .../components/case-card/case-card-model.ts | 32 +++++++++++++++---- v3/src/components/case-card/case-view.tsx | 2 +- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/v3/src/components/case-card/case-card-model.ts b/v3/src/components/case-card/case-card-model.ts index 68342f0d4..675987436 100644 --- a/v3/src/components/case-card/case-card-model.ts +++ b/v3/src/components/case-card/case-card-model.ts @@ -128,15 +128,35 @@ export const CaseCardModel = TileContentModel self.attributeColumnWidths.delete(collectionId) } }, - addNewCase(cases: IGroupedCase[], collection: ICollectionModel, displayedCaseId: string) { + addNewCase(collection: ICollectionModel) { const newCase: ICaseCreation = {} + const selectedCases = self.data?.selection - collection.allParentDataAttrs.forEach(attr => { - if (attr?.id) { - const value = self.data?.getValue(displayedCaseId, attr.id) - newCase[attr.id] = value + function findCommonCases(lineages: (readonly string[])[]) { + if (lineages.length === 0) return []; + let commonValues = lineages[0]; + for (let i = 1; i < lineages.length; i++) { + commonValues = commonValues.filter(value => lineages[i].includes(value)); + if (commonValues.length === 0) { + return []; + } + } + return commonValues; + } + + if (selectedCases) { + const caseLineages = Array.from(selectedCases).map(caseId => self.caseLineage(caseId) || []) + const commonCaseIds = findCommonCases(caseLineages) + if (commonCaseIds.length === 0) { + const nearestCommonParentCaseId = commonCaseIds[commonCaseIds.length - 1] + const parentAttrs = collection.allParentAttrs + parentAttrs.forEach(attr => { + const attrId = attr.id + const parentValue = nearestCommonParentCaseId && self.data?.getValue(nearestCommonParentCaseId, attrId) + newCase[attrId] = parentValue + }) } - }) + } const [newCaseId] = self.data?.addCases([newCase]) ?? [] diff --git a/v3/src/components/case-card/case-view.tsx b/v3/src/components/case-card/case-view.tsx index bd9293f1f..d79c03ea4 100644 --- a/v3/src/components/case-card/case-view.tsx +++ b/v3/src/components/case-card/case-view.tsx @@ -76,7 +76,7 @@ export const CaseView = observer(function CaseView(props: ICaseViewProps) { if (collection) { let newCaseId: string | undefined data?.applyModelChange(() => { - const newItemId = cardModel?.addNewCase(cases, collection, displayedCaseId) + const newItemId = cardModel?.addNewCase(collection) newCaseId = newItemId && data?.getItemCaseIds(newItemId)[level] newCaseId && onSelectCases([newCaseId]) }, { From 7f0931dc77d73e8d8b3c29e28e9ea2b0a7648dad Mon Sep 17 00:00:00 2001 From: lublagg Date: Wed, 2 Oct 2024 17:17:56 -0400 Subject: [PATCH 4/5] Fix failing tests. --- v3/cypress/e2e/case-card.spec.ts | 15 ++++++++------- v3/src/components/case-card/case-card-model.ts | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/v3/cypress/e2e/case-card.spec.ts b/v3/cypress/e2e/case-card.spec.ts index 964ad9370..501785f3b 100644 --- a/v3/cypress/e2e/case-card.spec.ts +++ b/v3/cypress/e2e/case-card.spec.ts @@ -243,6 +243,7 @@ context("case card", () => { table.toggleCaseView() cy.wait(500) cy.get('[data-testid="case-card-view"]').should("have.length", 3) + cy.get('[data-testid="case-card-view-next-button"]').eq(0).click() cy.log("Add new case to 'middle' collection.") cy.get('[data-testid="case-card-view"]').eq(1).find('[data-testid="case-card-view-index"]') .eq(0).should("have.text", "4 cases") @@ -264,7 +265,7 @@ context("case card", () => { toolbar.getUndoTool().click() toolbar.getUndoTool().click() cy.get('[data-testid="case-card-view"]').eq(1).find('[data-testid="case-card-view-index"]') - .eq(0).should("have.text", "4 cases") + .eq(0).should("have.text", "1 of 4") toolbar.getRedoTool().click() toolbar.getRedoTool().click() cy.get('[data-testid="case-card-view"]').eq(1).find('[data-testid="case-card-view-index"]') @@ -328,16 +329,16 @@ context("case card", () => { cy.wait(500) cy.get('[data-testid="trash-menu-list"]').should("be.visible") card.getDeleteSelectedCasesButton().click() + cy.get('[data-testid="case-card-view-index"]').should("have.text", "26 cases") + cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "26 values") + cy.get('[data-testid="case-card-view-next-button"]').click() cy.get('[data-testid="case-card-view-index"]').should("have.text", "1 of 26") cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Asian Elephant") - cy.get('[data-testid="case-card-view-next-button"]').click() - cy.get('[data-testid="case-card-view-index"]').should("have.text", "2 of 26") - cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Big Brown Bat") card.getDeleteCasesButton().click() cy.wait(500) card.getDeleteUnselectedCasesButton().click() cy.get('[data-testid="case-card-view-index"]').should("have.text", "1 of 1") - cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Big Brown Bat") + cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Asian Elephant") }) it("allows user to set aside and restore cases from inspector panel", () => { table.toggleCaseView() @@ -353,8 +354,8 @@ context("case card", () => { card.getRestoreSetAsideCasesButton().should("be.disabled").and("have.text", "Restore 0 Set Aside Cases") // resorting to {force: true} because this is failing in CI, though it passes locally card.getSetAsideSelectedCasesButton().click({force: true}) - cy.get('[data-testid="case-card-view-index"]').should("have.text", "1 of 26") - cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Asian Elephant") + cy.get('[data-testid="case-card-view-index"]').should("have.text", "26 cases") + cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "26 values") card.getHideShowButton().click() cy.wait(500) card.getRestoreSetAsideCasesButton().should("not.be.disabled").and("have.text", "Restore 1 Set Aside Cases") diff --git a/v3/src/components/case-card/case-card-model.ts b/v3/src/components/case-card/case-card-model.ts index 675987436..57b0fe4e6 100644 --- a/v3/src/components/case-card/case-card-model.ts +++ b/v3/src/components/case-card/case-card-model.ts @@ -147,7 +147,7 @@ export const CaseCardModel = TileContentModel if (selectedCases) { const caseLineages = Array.from(selectedCases).map(caseId => self.caseLineage(caseId) || []) const commonCaseIds = findCommonCases(caseLineages) - if (commonCaseIds.length === 0) { + if (commonCaseIds.length > 0) { const nearestCommonParentCaseId = commonCaseIds[commonCaseIds.length - 1] const parentAttrs = collection.allParentAttrs parentAttrs.forEach(attr => { From 993c6e734a42d2ec0ffbb4afea224562643ef79e Mon Sep 17 00:00:00 2001 From: lublagg Date: Wed, 2 Oct 2024 17:20:06 -0400 Subject: [PATCH 5/5] Fix lint errors. --- v3/src/components/case-card/case-card-model.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/v3/src/components/case-card/case-card-model.ts b/v3/src/components/case-card/case-card-model.ts index 57b0fe4e6..4bcfd0dd7 100644 --- a/v3/src/components/case-card/case-card-model.ts +++ b/v3/src/components/case-card/case-card-model.ts @@ -133,15 +133,15 @@ export const CaseCardModel = TileContentModel const selectedCases = self.data?.selection function findCommonCases(lineages: (readonly string[])[]) { - if (lineages.length === 0) return []; - let commonValues = lineages[0]; + if (lineages.length === 0) return [] + let commonValues = lineages[0] for (let i = 1; i < lineages.length; i++) { - commonValues = commonValues.filter(value => lineages[i].includes(value)); + commonValues = commonValues.filter(value => lineages[i].includes(value)) if (commonValues.length === 0) { - return []; + return [] } } - return commonValues; + return commonValues } if (selectedCases) {