From 23be217fae3cf8895e56f541f5fd017b96bbf7f3 Mon Sep 17 00:00:00 2001 From: eireland Date: Thu, 14 Nov 2024 12:36:41 -0800 Subject: [PATCH 1/3] Implements precision for both numeric and dates. --- v3/src/components/case-table/use-columns.tsx | 4 +- v3/src/components/case-table/use-rows.ts | 4 +- .../attribute-format-utils.tsx | 14 ++-- .../edit-attribute-properties-modal.tsx | 64 +++++++++++++------ v3/src/models/data/attribute-types.ts | 2 +- v3/src/models/data/attribute.test.ts | 4 +- v3/src/models/data/attribute.ts | 10 +-- 7 files changed, 67 insertions(+), 35 deletions(-) diff --git a/v3/src/components/case-table/use-columns.tsx b/v3/src/components/case-table/use-columns.tsx index 752870c2a6..3a859e2242 100644 --- a/v3/src/components/case-table/use-columns.tsx +++ b/v3/src/components/case-table/use-columns.tsx @@ -31,8 +31,8 @@ export const useColumns = ({ data, indexColumn }: IUseColumnsProps) => { const collection = data?.getCollection(collectionId) const attrs: IAttribute[] = collection ? getCollectionAttrs(collection, data) : [] const visible: IAttribute[] = attrs.filter(attr => attr && !caseMetadata?.isHidden(attr.id)) - return visible.map(({ id, name, type, userType, isEditable, hasFormula }) => - ({ id, name, type, userType, isEditable, hasFormula })) + return visible.map(({ id, name, type, userType, isEditable, hasFormula, precision }) => + ({ id, name, type, userType, isEditable, hasFormula, precision })) }, entries => { // column definitions diff --git a/v3/src/components/case-table/use-rows.ts b/v3/src/components/case-table/use-rows.ts index a87f3e0a0b..9a7e3e8bdc 100644 --- a/v3/src/components/case-table/use-rows.ts +++ b/v3/src/components/case-table/use-rows.ts @@ -8,7 +8,7 @@ import { useDataSetContext } from "../../hooks/use-data-set-context" import { useLoggingContext } from "../../hooks/use-log-context" import { logMessageWithReplacement } from "../../lib/log-message" import { appState } from "../../models/app-state" -import { kDefaultFormatStr } from "../../models/data/attribute-types" +import { kDefaultFormatNum } from "../../models/data/attribute-types" import { isAddCasesAction, isRemoveCasesAction, isSetCaseValuesAction } from "../../models/data/data-set-actions" import { createCasesNotification } from "../../models/data/data-set-notifications" import { @@ -86,7 +86,7 @@ export const useRows = (gridElement: HTMLDivElement | null) => { if (data && caseId && attr && cellSpan) { const strValue = data.getStrValue(caseId, attr.id) const numValue = data.getNumeric(caseId, attr.id) - const formatStr = attr.format || kDefaultFormatStr + const formatStr = attr.format || `${kDefaultFormatNum}~f` const formatted = (numValue != null) && isFinite(numValue) ? format(formatStr)(numValue) : strValue cellSpan.textContent = formatted ?? "" setCachedDomAttr(caseId, attr.id) diff --git a/v3/src/components/case-tile-common/attribute-format-utils.tsx b/v3/src/components/case-tile-common/attribute-format-utils.tsx index 0daef12465..d58436572b 100644 --- a/v3/src/components/case-tile-common/attribute-format-utils.tsx +++ b/v3/src/components/case-tile-common/attribute-format-utils.tsx @@ -1,7 +1,7 @@ import { format } from "d3-format" import React from "react" import { IAttribute } from "../../models/data/attribute" -import { kDefaultFormatStr } from "../../models/data/attribute-types" +import { kDefaultFormatNum } from "../../models/data/attribute-types" import { parseColor } from "../../utilities/color-utils" import { isStdISODateString } from "../../utilities/date-iso-utils" import { parseDate } from "../../utilities/date-parser" @@ -24,8 +24,7 @@ export const getNumFormatter = (formatStr: string) => { } export function renderAttributeValue(str = "", num = NaN, attr?: IAttribute, key?: number) { - const { type, userType } = attr || {} - + const { type, userType, precision } = attr || {} // colors const color = type === "color" || !userType ? parseColor(str, { colorNames: type === "color" }) : "" if (color) { @@ -41,7 +40,11 @@ export function renderAttributeValue(str = "", num = NaN, attr?: IAttribute, key // numbers if (isFinite(num)) { - const formatStr = attr?.format ?? kDefaultFormatStr + const formatStr = typeof precision === "number" + ? `.${precision}~f` + : typeof precision === "string" && attr?.format + ? attr.format + : `.${kDefaultFormatNum}~f` const formatter = getNumFormatter(formatStr) if (formatter) str = formatter(num) } @@ -57,8 +60,7 @@ export function renderAttributeValue(str = "", num = NaN, attr?: IAttribute, key if (isStdISODateString(str) || userType === "date" && str !== "") { const date = parseDate(str, true) if (date) { - // TODO: add precision support for date formatting - const formattedDate = formatDate(date, DatePrecision.None) + const formattedDate = formatDate(date, precision as DatePrecision) return { value: str, content: {formattedDate || `"${str}"`} diff --git a/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx b/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx index 4ebf015bab..d3f910d76c 100644 --- a/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx +++ b/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx @@ -8,6 +8,7 @@ import { updateAttributesNotification } from "../../../models/data/data-set-noti import { uniqueName } from "../../../utilities/js-utils" import { t } from "../../../utilities/translation/translate" import { CodapModal } from "../../codap-modal" +import { DatePrecision } from "../../../utilities/date-utils" import AttributeIcon from "../../../assets/icons/attribute-icon.svg" import "./attribute-menu.scss" @@ -31,7 +32,7 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I const [attributeName, setAttributeName] = useState(columnName) const [description, setDescription] = useState("") const [units, setUnits] = useState("") - const [precision, setPrecision] = useState("") + const [precision, setPrecision] = useState(attribute?.precision) const [userType, setUserType] = useState("none") const [editable, setEditable] = useState("yes") @@ -40,7 +41,7 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I setAttributeName(attribute?.name || "attribute") setDescription(attribute?.description ?? "") setUnits(attribute?.units ?? "") - setPrecision(`${attribute?.precision ?? ""}`) + setPrecision(attribute?.precision) setUserType(attribute?.userType ?? "none") setEditable(attribute?.editable ? "yes" : "no") }, [attribute, isOpen]) @@ -63,8 +64,8 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I if (userType !== (attribute.userType ?? "none")) { attribute.setUserType(userType === "none" ? undefined : userType) } - if (precision !== `${attribute?.precision ?? ""}`) { - attribute.setPrecision(precision ? +precision : undefined) + if (precision !== (attribute.precision ?? "")) { + attribute.setPrecision(precision) } if ((editable === "yes") !== attribute.editable) { attribute.setEditable(editable === "yes") @@ -100,6 +101,46 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I { label: t("DG.AttrFormView.applyBtnTitle"), onClick: applyChanges, default: true } ] + const getPrecisionMenu = () => { + if (attribute?.type === "numeric" || userType === "numeric") { + return ( + + ) + } else if (attribute?.type === "date" || userType === "date") { + return ( + + ) + } else { + return null + } + } + return ( {t("DG.CaseTable.attributeEditor.precision")} - + {getPrecisionMenu()} {t("DG.CaseTable.attributeEditor.editable")} process.env.NODE_ENV !== "production" export const isProduction = () => process.env.NODE_ENV === "production" diff --git a/v3/src/models/data/attribute.test.ts b/v3/src/models/data/attribute.test.ts index ac0295e048..3917dd33db 100644 --- a/v3/src/models/data/attribute.test.ts +++ b/v3/src/models/data/attribute.test.ts @@ -4,7 +4,7 @@ import { getSnapshot } from "mobx-state-tree" import { Attribute, IAttributeSnapshot, importValueToString, isAttributeType, isFormulaAttr, isValidFormulaAttr } from "./attribute" -import { kDefaultFormatStr } from "./attribute-types" +import { kDefaultFormatNum } from "./attribute-types" describe("Attribute", () => { @@ -202,7 +202,7 @@ describe("Attribute", () => { expect(attribute.strValues).toEqual(["", "", "", "", "", ""]) expect(attribute.numValues).toEqual([NaN, NaN, NaN, NaN, NaN, NaN]) - expect(attribute.format).toBe(kDefaultFormatStr) + expect(attribute.format).toBe(`${kDefaultFormatNum}~f`) attribute.setPrecision(2) expect(attribute.format).toBe(".2~f") diff --git a/v3/src/models/data/attribute.ts b/v3/src/models/data/attribute.ts index 67185875b0..4f4ac485c9 100644 --- a/v3/src/models/data/attribute.ts +++ b/v3/src/models/data/attribute.ts @@ -34,8 +34,9 @@ import { cachedFnFactory } from "../../utilities/mst-utils" import { Formula, IFormula } from "../formula/formula" import { applyModelChange } from "../history/apply-model-change" import { withoutUndo } from "../history/without-undo" -import { isDevelopment, isProduction, IValueType, kDefaultFormatStr } from "./attribute-types" +import { isDevelopment, isProduction, IValueType, kDefaultFormatNum } from "./attribute-types" import { V2Model } from "./v2-model" +import { DatePrecision } from "../../utilities/date-utils" export interface ISetValueOptions { noInvalidate?: boolean @@ -71,7 +72,7 @@ export const Attribute = V2Model.named("Attribute").props({ userType: types.maybe(types.enumeration([...attributeTypes])), // userFormat: types.maybe(types.string), units: types.maybe(types.string), - precision: types.maybe(types.number), + precision: types.maybe(types.union(types.number, types.enumeration(Object.values(DatePrecision)))), deleteable: true, editable: true, formula: types.maybe(Formula), @@ -241,7 +242,8 @@ export const Attribute = V2Model.named("Attribute").props({ return "categorical" }, get format() { - return self.precision != null ? `.${self.precision}~f` : kDefaultFormatStr + const precision = self.precision ?? kDefaultFormatNum + return typeof precision === "number" ? `.${precision}~f` : precision }, get isEditable() { return self.editable && !self.hasFormula @@ -279,7 +281,7 @@ export const Attribute = V2Model.named("Attribute").props({ // setUserFormat(precision: string) { // self.userFormat = `.${precision}~f` // }, - setPrecision(precision?: number) { + setPrecision(precision?: number | DatePrecision) { self.precision = precision }, setDeleteable(deleteable: boolean) { From 406aae35297fa5ddd434980d24217c09a02fb703 Mon Sep 17 00:00:00 2001 From: eireland Date: Thu, 14 Nov 2024 12:40:19 -0800 Subject: [PATCH 2/3] fix attr format string for numerical --- v3/src/components/case-table/use-rows.ts | 2 +- v3/src/models/data/attribute.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/v3/src/components/case-table/use-rows.ts b/v3/src/components/case-table/use-rows.ts index 9a7e3e8bdc..71801661f2 100644 --- a/v3/src/components/case-table/use-rows.ts +++ b/v3/src/components/case-table/use-rows.ts @@ -86,7 +86,7 @@ export const useRows = (gridElement: HTMLDivElement | null) => { if (data && caseId && attr && cellSpan) { const strValue = data.getStrValue(caseId, attr.id) const numValue = data.getNumeric(caseId, attr.id) - const formatStr = attr.format || `${kDefaultFormatNum}~f` + const formatStr = attr.format || `.${kDefaultFormatNum}~f` const formatted = (numValue != null) && isFinite(numValue) ? format(formatStr)(numValue) : strValue cellSpan.textContent = formatted ?? "" setCachedDomAttr(caseId, attr.id) diff --git a/v3/src/models/data/attribute.test.ts b/v3/src/models/data/attribute.test.ts index 3917dd33db..fefd679c39 100644 --- a/v3/src/models/data/attribute.test.ts +++ b/v3/src/models/data/attribute.test.ts @@ -202,7 +202,7 @@ describe("Attribute", () => { expect(attribute.strValues).toEqual(["", "", "", "", "", ""]) expect(attribute.numValues).toEqual([NaN, NaN, NaN, NaN, NaN, NaN]) - expect(attribute.format).toBe(`${kDefaultFormatNum}~f`) + expect(attribute.format).toBe(`.${kDefaultFormatNum}~f`) attribute.setPrecision(2) expect(attribute.format).toBe(".2~f") From 985c1ec899a1bcc42f3904bea6165e500596a6be Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Fri, 15 Nov 2024 09:28:02 -0800 Subject: [PATCH 3/3] chore: code review tweaks --- v3/src/components/case-table/use-rows.ts | 8 +- .../attribute-format-utils.tsx | 14 ++-- .../edit-attribute-properties-modal.tsx | 73 ++++++++++--------- v3/src/models/data/attribute-types.ts | 3 +- v3/src/models/data/attribute.test.ts | 5 -- v3/src/models/data/attribute.ts | 12 +-- 6 files changed, 57 insertions(+), 58 deletions(-) diff --git a/v3/src/components/case-table/use-rows.ts b/v3/src/components/case-table/use-rows.ts index 71801661f2..5f5b30237d 100644 --- a/v3/src/components/case-table/use-rows.ts +++ b/v3/src/components/case-table/use-rows.ts @@ -1,4 +1,3 @@ -import { format } from "d3" import { reaction } from "mobx" import { useCallback, useEffect, useRef } from "react" import { useDebouncedCallback } from "use-debounce" @@ -8,7 +7,6 @@ import { useDataSetContext } from "../../hooks/use-data-set-context" import { useLoggingContext } from "../../hooks/use-log-context" import { logMessageWithReplacement } from "../../lib/log-message" import { appState } from "../../models/app-state" -import { kDefaultFormatNum } from "../../models/data/attribute-types" import { isAddCasesAction, isRemoveCasesAction, isSetCaseValuesAction } from "../../models/data/data-set-actions" import { createCasesNotification } from "../../models/data/data-set-notifications" import { @@ -18,6 +16,7 @@ import { isSetIsCollapsedAction } from "../../models/shared/shared-case-metadata import { mstReaction } from "../../utilities/mst-reaction" import { onAnyAction } from "../../utilities/mst-utils" import { prf } from "../../utilities/profiler" +import { renderAttributeValue } from "../case-tile-common/attribute-format-utils" import { applyCaseValueChanges } from "../case-tile-common/case-tile-utils" import { kInputRowKey, symDom, TRow, TRowsChangeData } from "./case-table-types" import { useCollectionTableModel } from "./use-collection-table-model" @@ -86,9 +85,8 @@ export const useRows = (gridElement: HTMLDivElement | null) => { if (data && caseId && attr && cellSpan) { const strValue = data.getStrValue(caseId, attr.id) const numValue = data.getNumeric(caseId, attr.id) - const formatStr = attr.format || `.${kDefaultFormatNum}~f` - const formatted = (numValue != null) && isFinite(numValue) ? format(formatStr)(numValue) : strValue - cellSpan.textContent = formatted ?? "" + const { value } = renderAttributeValue(strValue, numValue, attr) + cellSpan.textContent = value setCachedDomAttr(caseId, attr.id) } }) diff --git a/v3/src/components/case-tile-common/attribute-format-utils.tsx b/v3/src/components/case-tile-common/attribute-format-utils.tsx index d58436572b..7e6f53ddaa 100644 --- a/v3/src/components/case-tile-common/attribute-format-utils.tsx +++ b/v3/src/components/case-tile-common/attribute-format-utils.tsx @@ -1,11 +1,11 @@ import { format } from "d3-format" import React from "react" import { IAttribute } from "../../models/data/attribute" -import { kDefaultFormatNum } from "../../models/data/attribute-types" +import { kDefaultNumPrecision } from "../../models/data/attribute-types" import { parseColor } from "../../utilities/color-utils" import { isStdISODateString } from "../../utilities/date-iso-utils" import { parseDate } from "../../utilities/date-parser" -import { DatePrecision, formatDate } from "../../utilities/date-utils" +import { formatDate } from "../../utilities/date-utils" import { kCaseTableBodyFont, kCaseTableHeaderFont, kMaxAutoColumnWidth, kMinAutoColumnWidth } from "../case-table/case-table-types" import { measureText } from "../../hooks/use-measure-text" @@ -24,7 +24,7 @@ export const getNumFormatter = (formatStr: string) => { } export function renderAttributeValue(str = "", num = NaN, attr?: IAttribute, key?: number) { - const { type, userType, precision } = attr || {} + const { type, userType, numPrecision, datePrecision } = attr || {} // colors const color = type === "color" || !userType ? parseColor(str, { colorNames: type === "color" }) : "" if (color) { @@ -40,11 +40,7 @@ export function renderAttributeValue(str = "", num = NaN, attr?: IAttribute, key // numbers if (isFinite(num)) { - const formatStr = typeof precision === "number" - ? `.${precision}~f` - : typeof precision === "string" && attr?.format - ? attr.format - : `.${kDefaultFormatNum}~f` + const formatStr = `.${numPrecision ?? kDefaultNumPrecision}~f` const formatter = getNumFormatter(formatStr) if (formatter) str = formatter(num) } @@ -60,7 +56,7 @@ export function renderAttributeValue(str = "", num = NaN, attr?: IAttribute, key if (isStdISODateString(str) || userType === "date" && str !== "") { const date = parseDate(str, true) if (date) { - const formattedDate = formatDate(date, precision as DatePrecision) + const formattedDate = formatDate(date, datePrecision) return { value: str, content: {formattedDate || `"${str}"`} diff --git a/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx b/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx index d3f910d76c..ccadf55273 100644 --- a/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx +++ b/v3/src/components/case-tile-common/attribute-menu/edit-attribute-properties-modal.tsx @@ -5,10 +5,10 @@ import { useDataSetContext } from "../../../hooks/use-data-set-context" import { logMessageWithReplacement } from "../../../lib/log-message" import { AttributeType, attributeTypes } from "../../../models/data/attribute" import { updateAttributesNotification } from "../../../models/data/data-set-notifications" +import { DatePrecision } from "../../../utilities/date-utils" import { uniqueName } from "../../../utilities/js-utils" import { t } from "../../../utilities/translation/translate" import { CodapModal } from "../../codap-modal" -import { DatePrecision } from "../../../utilities/date-utils" import AttributeIcon from "../../../assets/icons/attribute-icon.svg" import "./attribute-menu.scss" @@ -32,7 +32,7 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I const [attributeName, setAttributeName] = useState(columnName) const [description, setDescription] = useState("") const [units, setUnits] = useState("") - const [precision, setPrecision] = useState(attribute?.precision) + const [precision, setPrecision] = useState(attribute?.precision) const [userType, setUserType] = useState("none") const [editable, setEditable] = useState("yes") @@ -64,7 +64,7 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I if (userType !== (attribute.userType ?? "none")) { attribute.setUserType(userType === "none" ? undefined : userType) } - if (precision !== (attribute.precision ?? "")) { + if (precision !== attribute.precision) { attribute.setPrecision(precision) } if ((editable === "yes") !== attribute.editable) { @@ -101,43 +101,50 @@ export const EditAttributePropertiesModal = ({ attributeId, isOpen, onClose }: I { label: t("DG.AttrFormView.applyBtnTitle"), onClick: applyChanges, default: true } ] + function toDatePrecision(pStr: string) { + return !pStr || isFinite(Number(pStr)) ? undefined : pStr as DatePrecision + } + function toDatePrecisionStr(p: typeof precision) { + return p == null || typeof p === "number" ? "" : p + } + + function toNumPrecision(pStr: string) { + return isFinite(Number(pStr)) ? Number(pStr) : undefined + } + function toNumPrecisionStr(p: typeof precision) { + return p == null || typeof p === "string" ? "" : `${p}` + } + const getPrecisionMenu = () => { - if (attribute?.type === "numeric" || userType === "numeric") { + if (attribute?.type === "date" || userType === "date") { return ( - setPrecision(toDatePrecision(e.target.value))}> + {Object.values(DatePrecision).map(p => { + return ( + + ) + })} ) - } else if (attribute?.type === "date" || userType === "date") { + } else { return ( - setPrecision(toNumPrecision(e.target.value))}> + + {[...Array(10).keys()].map(pNum => { + const precisionStr = `${pNum}` + return ( + + ) + })} ) - } else { - return null } } diff --git a/v3/src/models/data/attribute-types.ts b/v3/src/models/data/attribute-types.ts index 88759a2c55..5408b7eabf 100644 --- a/v3/src/models/data/attribute-types.ts +++ b/v3/src/models/data/attribute-types.ts @@ -1,4 +1,5 @@ -export const kDefaultFormatNum = 3 +export const kDefaultNumPrecision = 3 +export const kDefaultNumFormatStr = `.${kDefaultNumPrecision}~f` export const isDevelopment = () => process.env.NODE_ENV !== "production" export const isProduction = () => process.env.NODE_ENV === "production" diff --git a/v3/src/models/data/attribute.test.ts b/v3/src/models/data/attribute.test.ts index fefd679c39..98dbe4b784 100644 --- a/v3/src/models/data/attribute.test.ts +++ b/v3/src/models/data/attribute.test.ts @@ -4,7 +4,6 @@ import { getSnapshot } from "mobx-state-tree" import { Attribute, IAttributeSnapshot, importValueToString, isAttributeType, isFormulaAttr, isValidFormulaAttr } from "./attribute" -import { kDefaultFormatNum } from "./attribute-types" describe("Attribute", () => { @@ -202,10 +201,6 @@ describe("Attribute", () => { expect(attribute.strValues).toEqual(["", "", "", "", "", ""]) expect(attribute.numValues).toEqual([NaN, NaN, NaN, NaN, NaN, NaN]) - expect(attribute.format).toBe(`.${kDefaultFormatNum}~f`) - attribute.setPrecision(2) - expect(attribute.format).toBe(".2~f") - expect(attribute.description).toBeUndefined() attribute.setDescription("description") expect(attribute.description).toBe("description") diff --git a/v3/src/models/data/attribute.ts b/v3/src/models/data/attribute.ts index 4f4ac485c9..db9e16b4ee 100644 --- a/v3/src/models/data/attribute.ts +++ b/v3/src/models/data/attribute.ts @@ -34,7 +34,7 @@ import { cachedFnFactory } from "../../utilities/mst-utils" import { Formula, IFormula } from "../formula/formula" import { applyModelChange } from "../history/apply-model-change" import { withoutUndo } from "../history/without-undo" -import { isDevelopment, isProduction, IValueType, kDefaultFormatNum } from "./attribute-types" +import { isDevelopment, isProduction, IValueType } from "./attribute-types" import { V2Model } from "./v2-model" import { DatePrecision } from "../../utilities/date-utils" @@ -115,6 +115,12 @@ export const Attribute = V2Model.named("Attribute").props({ if (value == null || value === "") return NaN return Number(value) }, + get numPrecision() { + return typeof self.precision === "number" ? self.precision : undefined + }, + get datePrecision() { + return typeof self.precision === "string" ? self.precision : undefined + }, getEmptyCount: cachedFnFactory(() => { // Note that `self.changeCount` is absolutely not necessary here. However, historically, this function used to be // a MobX computed property, and `self.changeCount` was used to invalidate the cache. Also, there are tests @@ -241,10 +247,6 @@ export const Attribute = V2Model.named("Attribute").props({ return "categorical" }, - get format() { - const precision = self.precision ?? kDefaultFormatNum - return typeof precision === "number" ? `.${precision}~f` : precision - }, get isEditable() { return self.editable && !self.hasFormula },