From d1e5bc1261e38719e761480fa4cee8956290cee8 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 15 Nov 2024 11:55:40 -0800 Subject: [PATCH 01/22] Fix whitespace --- src/actions/tree.ts | 2 +- src/store.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/actions/tree.ts b/src/actions/tree.ts index 34967f0e4..a6e5c67d9 100644 --- a/src/actions/tree.ts +++ b/src/actions/tree.ts @@ -277,7 +277,7 @@ export const applyFilter = ( * - "set" -> set the values of the filter to be those provided. All disabled filters will be removed. XXX TODO. */ mode: "add" | "inactivate" | "remove" | "set", - + /** the trait name of the filter ("authors", "country" etcetera) */ trait: string | symbol, diff --git a/src/store.ts b/src/store.ts index d68e74d09..da2238dc7 100644 --- a/src/store.ts +++ b/src/store.ts @@ -44,7 +44,7 @@ const store = configureStore({ if (process.env.NODE_ENV !== 'production' && module.hot) { // console.log("hot reducer reload"); module.hot.accept('./reducers', () => { - const nextRootReducer = require('./reducers/index'); + const nextRootReducer = require('./reducers/index'); store.replaceReducer(nextRootReducer); }); } From 4eb9c3c046d378c6046fe2d777f6bdf674458ff8 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 15 Nov 2024 11:55:56 -0800 Subject: [PATCH 02/22] Move type ThunkFunction to `src/store.ts` Moving in preparation for using this type for the measurements thunk functions as well. I had considered adding a new `src/actions/types.ts` file, but that will be confusing with the action types in `src/actions/types.js`. --- src/actions/tree.ts | 5 +---- src/store.ts | 3 +++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/actions/tree.ts b/src/actions/tree.ts index a6e5c67d9..6b7bbac57 100644 --- a/src/actions/tree.ts +++ b/src/actions/tree.ts @@ -13,7 +13,7 @@ import { warningNotification } from "./notifications"; import { calcFullTipCounts, calcTipCounts } from "../util/treeCountingHelpers"; import { PhyloNode } from "../components/tree/phyloTree/types"; import { Metadata } from "../metadata"; -import { AppDispatch, RootState } from "../store"; +import { ThunkFunction } from "../store"; import { ReduxNode, TreeState } from "../reducers/tree/types"; type RootIndex = number | undefined @@ -21,9 +21,6 @@ type RootIndex = number | undefined /** [root idx tree1, root idx tree2] */ export type Root = [RootIndex, RootIndex] -/** A function to be handled by redux (thunk) */ -type ThunkFunction = (dispatch: AppDispatch, getState: () => RootState) => void - /** * Updates the `inView` property of nodes which depends on the currently selected * root index (i.e. what node the tree is zoomed into). diff --git a/src/store.ts b/src/store.ts index da2238dc7..69c87306b 100644 --- a/src/store.ts +++ b/src/store.ts @@ -54,4 +54,7 @@ if (process.env.NODE_ENV !== 'production' && module.hot) { export type RootState = ReturnType export type AppDispatch = typeof store.dispatch +/** A function to be handled by redux (thunk) */ +export type ThunkFunction = (dispatch: AppDispatch, getState: () => RootState) => void + export default store; From 975b7546bfe0ed4aa3e35ecc64d7cca3bba8872a Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 15 Nov 2024 16:20:51 -0800 Subject: [PATCH 03/22] Add @types/lodash --- package-lock.json | 14 ++++++++++++++ package.json | 1 + 2 files changed, 15 insertions(+) diff --git a/package-lock.json b/package-lock.json index d383145ac..072d8b188 100644 --- a/package-lock.json +++ b/package-lock.json @@ -110,6 +110,7 @@ "@types/d3-transition": "^1.2.0", "@types/d3-zoom": "^1.1.3", "@types/leaflet": "^1.9.3", + "@types/lodash": "^4.17.13", "@types/node": "^18.15.11", "@types/webpack-env": "^1.18.2", "@typescript-eslint/eslint-plugin": "^5.57.0", @@ -3275,6 +3276,13 @@ "@types/geojson": "*" } }, + "node_modules/@types/lodash": { + "version": "4.17.13", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.13.tgz", + "integrity": "sha512-lfx+dftrEZcdBPczf9d0Qv0x+j/rfNCMuC6OcfXmO8gkfeNAY88PgKUbvG56whcN23gc27yenwF6oJZXGFpYxg==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/minimatch": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-5.1.2.tgz", @@ -15630,6 +15638,12 @@ "@types/geojson": "*" } }, + "@types/lodash": { + "version": "4.17.13", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.13.tgz", + "integrity": "sha512-lfx+dftrEZcdBPczf9d0Qv0x+j/rfNCMuC6OcfXmO8gkfeNAY88PgKUbvG56whcN23gc27yenwF6oJZXGFpYxg==", + "dev": true + }, "@types/minimatch": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-5.1.2.tgz", diff --git a/package.json b/package.json index b8497ca8f..ca1fb35c2 100644 --- a/package.json +++ b/package.json @@ -131,6 +131,7 @@ "@types/d3-transition": "^1.2.0", "@types/d3-zoom": "^1.1.3", "@types/leaflet": "^1.9.3", + "@types/lodash": "^4.17.13", "@types/node": "^18.15.11", "@types/webpack-env": "^1.18.2", "@typescript-eslint/eslint-plugin": "^5.57.0", From 07ba5e17266abe3009ecee5dee2ad6d8fd350f4c Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 15 Nov 2024 17:12:31 -0800 Subject: [PATCH 04/22] Rename for TypeScript --- src/reducers/{measurements.js => measurements/index.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/reducers/{measurements.js => measurements/index.ts} (100%) diff --git a/src/reducers/measurements.js b/src/reducers/measurements/index.ts similarity index 100% rename from src/reducers/measurements.js rename to src/reducers/measurements/index.ts From 8219fa332664fe164ccd2f1f7637c541b2bd504b Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 15 Nov 2024 17:07:52 -0800 Subject: [PATCH 05/22] Add types to measurements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add types for most measurements specific files. I'm delaying converting the measurementsD3.js file because the import of `event` from d3-selection causes an error. This is likely due to the d3 types not matching the version of d3-selection.¹ I decided to place the measurements JSON types in the same file as the measurements state types so that it's easy to compare the data structures before and after parsing. There are many type errors that will be fixed in subsequent commits. ¹ --- .../{measurements.js => measurements.ts} | 141 +++++++++++++----- src/components/controls/filter.js | 4 +- ...entsOptions.js => measurementsOptions.tsx} | 27 ++-- src/components/info/filtersSummary.js | 2 +- .../{hoverPanel.js => hoverPanel.tsx} | 18 ++- .../measurements/{index.js => index.tsx} | 102 ++++++++----- src/reducers/controls.ts | 10 +- src/reducers/measurements/index.ts | 11 +- src/reducers/measurements/types.ts | 79 ++++++++++ 9 files changed, 294 insertions(+), 100 deletions(-) rename src/actions/{measurements.js => measurements.ts} (86%) rename src/components/controls/{measurementsOptions.js => measurementsOptions.tsx} (79%) rename src/components/measurements/{hoverPanel.js => hoverPanel.tsx} (89%) rename src/components/measurements/{index.js => index.tsx} (78%) create mode 100644 src/reducers/measurements/types.ts diff --git a/src/actions/measurements.js b/src/actions/measurements.ts similarity index 86% rename from src/actions/measurements.js rename to src/actions/measurements.ts index 7e43a8800..4852b6bb5 100644 --- a/src/actions/measurements.js +++ b/src/actions/measurements.ts @@ -1,6 +1,7 @@ import { cloneDeep, pick } from "lodash"; +import { AppDispatch, ThunkFunction } from "../store"; import { measurementIdSymbol } from "../util/globals"; -import { defaultMeasurementsControlState } from "../reducers/controls"; +import { ControlsState, defaultMeasurementsControlState, MeasurementsControlState } from "../reducers/controls"; import { getDefaultMeasurementsState } from "../reducers/measurements"; import { warningNotification } from "./notifications"; import { @@ -11,6 +12,42 @@ import { TOGGLE_MEASUREMENTS_OVERALL_MEAN, TOGGLE_MEASUREMENTS_THRESHOLD, } from "./types"; +import { + Collection, + MeasurementMetadata, + MeasurementsDisplay, + MeasurementsJson, + MeasurementsState +} from "../reducers/measurements/types"; + +/** + * Temp object for groupings to keep track of values and their counts so that + * we can create a stable default order for grouping field values + */ +interface GroupingValues { + [key: string]: { + [key: string]: number + } +} + + +/* mf_ correspond to active measurements filters */ +const filterQueryPrefix = "mf_"; +type MeasurementsFilterQuery = `mf_${string}` +type QueryBoolean = "show" | "hide" +/* Measurements query parameters that are constructed and/or parsed here. */ +interface MeasurementsQuery { + m_collection?: string + m_display?: MeasurementsDisplay + m_groupBy?: string + m_overallMean?: QueryBoolean + m_threshold?: QueryBoolean + [key: MeasurementsFilterQuery]: string[] +} +/* Central Query type placeholder! */ +interface Query extends MeasurementsQuery { + [key: string]: unknown +} /** * Find the collection within collections that has a key matching the provided @@ -19,13 +56,12 @@ import { * If collectionKey is not provided, returns the default collection. * If no matches are found, returns the default collection. * If multiple matches are found, only returns the first matching collection. - * - * @param {Array} collections - * @param {string} collectionKey - * @param {string} defaultKey - * @returns {Object} */ -const getCollectionToDisplay = (collections, collectionKey, defaultKey) => { +const getCollectionToDisplay = ( + collections: Collection[], + collectionKey: string, + defaultKey: string +): Collection => { const defaultCollection = collections.filter((collection) => collection.key === defaultKey)[0]; if (!collectionKey) return defaultCollection; const potentialCollections = collections.filter((collection) => collection.key === collectionKey); @@ -39,11 +75,11 @@ const getCollectionToDisplay = (collections, collectionKey, defaultKey) => { /** * Map the controlKey to the default value in collectionDefaults * Checks if the collection default is a valid value for the control - * @param {string} controlKey - * @param {Object} collection - * @returns {*} */ -function getCollectionDefaultControl(controlKey, collection) { +function getCollectionDefaultControl( + controlKey: string, + collection: Collection +): string | boolean | undefined { const collectionControlToDisplayDefaults = { measurementsGroupBy: 'group_by', measurementsDisplay: 'measurements_display', @@ -106,10 +142,8 @@ function getCollectionDefaultControl(controlKey, collection) { /** * Returns the default control state for the provided collection * Returns teh default control state for the app if the collection is not loaded yet - * @param {Object} collection - * @returns {MeasurementsControlState} */ -function getCollectionDefaultControls(collection) { +function getCollectionDefaultControls(collection: Collection): MeasurementsControlState { const defaultControls = {...defaultMeasurementsControlState}; if (Object.keys(collection).length) { for (const [key, value] of Object.entries(defaultControls)) { @@ -127,10 +161,11 @@ function getCollectionDefaultControls(collection) { * If no display defaults are provided, uses the current controls redux state. * If the current `measurementsGrouping` does not exist in the collection, then * defaults to the first grouping option. - * @param {Object} collection - * @returns {MeasurementsControlState} */ -const getCollectionDisplayControls = (controls, collection) => { +const getCollectionDisplayControls = ( + controls: ControlsState, + collection: Collection +): MeasurementsControlState => { // Copy current control options for measurements const newControls = cloneDeep(pick(controls, Object.keys(defaultMeasurementsControlState))); // Checks the current group by is available as a grouping in collection @@ -142,7 +177,7 @@ const getCollectionDisplayControls = (controls, collection) => { // Verify that current filters are valid for the new collection newControls.measurementsFilters = Object.fromEntries( Object.entries(newControls.measurementsFilters) - .map(([field, valuesMap]) => { + .map(([field, valuesMap]): [string, Map] => { // Clone nested Map to avoid changing redux state in place // Delete filter for values that do not exist within the field of the new collection const newValuesMap = new Map([...valuesMap].filter(([value]) => { @@ -169,7 +204,7 @@ const getCollectionDisplayControls = (controls, collection) => { return newControls; }; -const parseMeasurementsJSON = (json) => { +const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Avoid editing the original json values since they are cached for narratives const collections = cloneDeep(json["collections"]); if (!collections || collections.length === 0) { @@ -211,7 +246,7 @@ const parseMeasurementsJSON = (json) => { // Create a temp object for groupings to keep track of values and their // counts so that we can create a stable default order for grouping field values - const groupingsValues = collection.groupings.reduce((tempObject, {key}) => { + const groupingsValues: GroupingValues = collection.groupings.reduce((tempObject, {key}) => { tempObject[key] = {}; return tempObject; }, {}); @@ -277,7 +312,10 @@ const parseMeasurementsJSON = (json) => { } }; -export const loadMeasurements = (measurementsData, dispatch) => { +export const loadMeasurements = ( + measurementsData: MeasurementsJson | Error, + dispatch: AppDispatch +): MeasurementsState => { let measurementState = getDefaultMeasurementsState(); /* Just return default state there are no measurements data to load */ if (!measurementsData) { @@ -305,7 +343,9 @@ export const loadMeasurements = (measurementsData, dispatch) => { return measurementState; }; -export const changeMeasurementsCollection = (newCollectionKey) => (dispatch, getState) => { +export const changeMeasurementsCollection = ( + newCollectionKey: string +): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const collectionToDisplay = getCollectionToDisplay(measurements.collections, newCollectionKey, measurements.defaultCollectionKey); const newControls = getCollectionDisplayControls(controls, collectionToDisplay); @@ -325,7 +365,11 @@ export const changeMeasurementsCollection = (newCollectionKey) => (dispatch, get * Tried to use lodash.cloneDeep(), but it did not work for the nested Map * - Jover, 19 January 2022 */ -export const applyMeasurementFilter = (field, value, active) => (dispatch, getState) => { +export const applyMeasurementFilter = ( + field: string, + value: MeasurementMetadata, + active: boolean +): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const measurementsFilters = {...controls.measurementsFilters}; measurementsFilters[field] = new Map(measurementsFilters[field]); @@ -338,7 +382,10 @@ export const applyMeasurementFilter = (field, value, active) => (dispatch, getSt }); }; -export const removeSingleFilter = (field, value) => (dispatch, getState) => { +export const removeSingleFilter = ( + field: string, + value: MeasurementMetadata +): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const measurementsFilters = {...controls.measurementsFilters}; measurementsFilters[field] = new Map(measurementsFilters[field]); @@ -357,7 +404,9 @@ export const removeSingleFilter = (field, value) => (dispatch, getState) => { }); }; -export const removeAllFieldFilters = (field) => (dispatch, getState) => { +export const removeAllFieldFilters = ( + field: string +): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const measurementsFilters = {...controls.measurementsFilters}; delete measurementsFilters[field]; @@ -369,7 +418,10 @@ export const removeAllFieldFilters = (field) => (dispatch, getState) => { }); }; -export const toggleAllFieldFilters = (field, active) => (dispatch, getState) => { +export const toggleAllFieldFilters = ( + field: string, + active: boolean +): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const measurementsFilters = {...controls.measurementsFilters}; measurementsFilters[field] = new Map(measurementsFilters[field]); @@ -383,7 +435,7 @@ export const toggleAllFieldFilters = (field, active) => (dispatch, getState) => }); }; -export const toggleOverallMean = () => (dispatch, getState) => { +export const toggleOverallMean = (): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const controlKey = "measurementsShowOverallMean"; const newControls = { [controlKey]: !controls[controlKey] }; @@ -395,7 +447,7 @@ export const toggleOverallMean = () => (dispatch, getState) => { }); } -export const toggleThreshold = () => (dispatch, getState) => { +export const toggleThreshold = (): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const controlKey = "measurementsShowThreshold"; const newControls = { [controlKey]: !controls[controlKey] }; @@ -407,7 +459,9 @@ export const toggleThreshold = () => (dispatch, getState) => { }); }; -export const changeMeasurementsDisplay = (newDisplay) => (dispatch, getState) => { +export const changeMeasurementsDisplay = ( + newDisplay: MeasurementsDisplay +): ThunkFunction => (dispatch, getState) => { const { measurements } = getState(); const controlKey = "measurementsDisplay"; const newControls = { [controlKey]: newDisplay }; @@ -419,7 +473,9 @@ export const changeMeasurementsDisplay = (newDisplay) => (dispatch, getState) => }); } -export const changeMeasurementsGroupBy = (newGroupBy) => (dispatch, getState) => { +export const changeMeasurementsGroupBy = ( + newGroupBy: string +): ThunkFunction => (dispatch, getState) => { const { measurements } = getState(); const controlKey = "measurementsGroupBy"; const newControls = { [controlKey]: newGroupBy }; @@ -438,9 +494,10 @@ const controlToQueryParamMap = { measurementsShowThreshold: "m_threshold", }; -/* mf_ correspond to active measurements filters */ -const filterQueryPrefix = "mf_"; -export function removeInvalidMeasurementsFilterQuery(query, newQueryParams) { +export function removeInvalidMeasurementsFilterQuery( + query: Query, + newQueryParams: {[key: MeasurementsFilterQuery]: string} +): Query { const newQuery = cloneDeep(query); // Remove measurements filter query params that are not included in the newQueryParams Object.keys(query) @@ -449,7 +506,11 @@ export function removeInvalidMeasurementsFilterQuery(query, newQueryParams) { return newQuery } -function createMeasurementsQueryFromControls(measurementControls, collection, defaultCollectionKey) { +function createMeasurementsQueryFromControls( + measurementControls: Partial, + collection: Collection, + defaultCollectionKey: string +): MeasurementsQuery { const newQuery = { m_collection: collection.key === defaultCollectionKey ? "" : collection.key }; @@ -505,11 +566,15 @@ function createMeasurementsQueryFromControls(measurementControls, collection, de * * In cases where the query param is invalid, the query param is removed from the * returned query object. - * @param {Object} measurements - * @param {Object} query - * @returns {Object} */ -export const combineMeasurementsControlsAndQuery = (measurements, query) => { +export const combineMeasurementsControlsAndQuery = ( + measurements: MeasurementsState, + query: Query +): { + collectionToDisplay: Collection, + collectionControls: MeasurementsControlState, + updatedQuery: Query +} => { const updatedQuery = cloneDeep(query); const collectionKeys = measurements.collections.map((collection) => collection.key); // Remove m_collection query if it's invalid or the default collection key diff --git a/src/components/controls/filter.js b/src/components/controls/filter.js index de94cac8f..f27e949b3 100644 --- a/src/components/controls/filter.js +++ b/src/components/controls/filter.js @@ -28,8 +28,8 @@ const DEBOUNCE_TIME = 200; nodes: state.tree.nodes, nodesSecondTree: state.treeToo?.nodes, totalStateCountsSecondTree: state.treeToo?.totalStateCounts, - measurementsFieldsMap: state.measurements.collectionToDisplay.fields, - measurementsFiltersMap: state.measurements.collectionToDisplay.filters, + measurementsFieldsMap: state.measurements.collectionToDisplay?.fields, + measurementsFiltersMap: state.measurements.collectionToDisplay?.filters, measurementsFilters: state.controls.measurementsFilters }; }) diff --git a/src/components/controls/measurementsOptions.js b/src/components/controls/measurementsOptions.tsx similarity index 79% rename from src/components/controls/measurementsOptions.js rename to src/components/controls/measurementsOptions.tsx index 78ace0c37..725f06fc9 100644 --- a/src/components/controls/measurementsOptions.js +++ b/src/components/controls/measurementsOptions.tsx @@ -13,15 +13,22 @@ import { controlsWidth } from "../../util/globals"; import { SidebarSubtitle, SidebarButton } from "./styles"; import Toggle from "./toggle"; import CustomSelect from "./customSelect"; +import { Collection } from "../../reducers/measurements/types"; +import { RootState } from "../../store"; + +interface SelectOption { + value: string + label: string +} /** * React Redux selector function that takes the key and title for the * available collections to create the object expected for the Select library. * The label defaults to the key if a collection does not have a set title. - * @param {Array} collections - * @returns {Array} */ -const collectionOptionsSelector = (collections) => { +const collectionOptionsSelector = ( + collections: Collection[] +): SelectOption[] => { return collections.map((collection) => { return { value: collection.key, @@ -32,15 +39,15 @@ const collectionOptionsSelector = (collections) => { const MeasurementsOptions = () => { const dispatch = useAppDispatch(); - const collection = useSelector((state) => state.measurements.collectionToDisplay); - const collectionOptions = useSelector((state) => collectionOptionsSelector(state.measurements.collections), isEqual); - const groupBy = useSelector((state) => state.controls.measurementsGroupBy); - const display = useSelector((state) => state.controls.measurementsDisplay); - const showOverallMean = useSelector((state) => state.controls.measurementsShowOverallMean); - const showThreshold = useSelector((state) => state.controls.measurementsShowThreshold); + const collection = useSelector((state: RootState) => state.measurements.collectionToDisplay); + const collectionOptions = useSelector((state: RootState) => collectionOptionsSelector(state.measurements.collections), isEqual); + const groupBy = useSelector((state: RootState) => state.controls.measurementsGroupBy); + const display = useSelector((state: RootState) => state.controls.measurementsDisplay); + const showOverallMean = useSelector((state: RootState) => state.controls.measurementsShowOverallMean); + const showThreshold = useSelector((state: RootState) => state.controls.measurementsShowThreshold); // Create grouping options for the Select library - let groupingOptions = []; + let groupingOptions: SelectOption[] = []; if (collection.groupings) { groupingOptions = [...collection.groupings.keys()].map((grouping) => { return { diff --git a/src/components/info/filtersSummary.js b/src/components/info/filtersSummary.js index ecdcd6335..8854082d9 100644 --- a/src/components/info/filtersSummary.js +++ b/src/components/info/filtersSummary.js @@ -42,7 +42,7 @@ const closeBracketSmall = { +export interface HoverData { + hoverTitle: string + mouseX: number + mouseY: number + containerId: string + data: Map +} + +const HoverPanel = ({ + hoverData +}: { + hoverData: HoverData +}) => { if (hoverData === null) return null; const { hoverTitle, mouseX, mouseY, containerId, data } = hoverData; - const panelStyle = { + const panelStyle: CSSProperties = { position: "absolute", minWidth: 200, padding: "5px", diff --git a/src/components/measurements/index.js b/src/components/measurements/index.tsx similarity index 78% rename from src/components/measurements/index.js rename to src/components/measurements/index.tsx index 9d6fe6dee..aec8c0ea2 100644 --- a/src/components/measurements/index.js +++ b/src/components/measurements/index.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useRef, useEffect, useMemo, useState } from "react"; +import React, { CSSProperties, MutableRefObject, useCallback, useRef, useEffect, useMemo, useState } from "react"; import { useSelector } from "react-redux"; import { isEqual, orderBy } from "lodash"; import { NODE_VISIBLE } from "../../util/globals"; @@ -8,7 +8,7 @@ import ErrorBoundary from "../../util/errorBoundary"; import Flex from "../framework/flex"; import Card from "../framework/card"; import Legend from "../tree/legend/legend"; -import HoverPanel from "./hoverPanel"; +import HoverPanel, { HoverData } from "./hoverPanel"; import { createXScale, createYScale, @@ -25,16 +25,31 @@ import { layout, jitterRawMeansByColorBy } from "./measurementsD3"; +import { RootState } from "../../store"; +import { MeasurementFilters } from "../../reducers/controls"; +import { Visibility } from "../../reducers/tree/types"; +import { Measurement, MeasurementMetadata } from "../../reducers/measurements/types"; + +interface TreeStrainVisibility { + [strain: string]: Visibility +} +interface TreeStrainProperties { + treeStrainVisibility: TreeStrainVisibility + treeStrainColors: { + [strain: string]: { + attribute: string + color: string + } + } +} /** * A custom React Hook that returns a memoized value that will only change * if a deep comparison using lodash.isEqual determines the value is not * equivalent to the previous value. - * @param {*} value - * @returns {*} */ -const useDeepCompareMemo = (value) => { - const ref = useRef(); +function useDeepCompareMemo(value: T): T { + const ref: MutableRefObject = useRef(); if (!isEqual(value, ref.current)) { ref.current = value; } @@ -42,7 +57,7 @@ const useDeepCompareMemo = (value) => { }; // Checks visibility against global NODE_VISIBLE -const isVisible = (visibility) => visibility === NODE_VISIBLE; +const isVisible = (visibility: Visibility): boolean => visibility === NODE_VISIBLE; /** * A custom React Redux Selector that reduces the tree redux state to an object @@ -52,13 +67,13 @@ const isVisible = (visibility) => visibility === NODE_VISIBLE; * * tree.visibility and tree.nodeColors need to be arrays that have the same * order as tree.nodes - * @param {Object} state - * @returns {Object} */ -const treeStrainPropertySelector = (state) => { +const treeStrainPropertySelector = ( + state: RootState +): TreeStrainProperties => { const { tree, controls } = state; const { colorScale } = controls; - const intitialTreeStrainProperty = { + const initialTreeStrainProperty: TreeStrainProperties = { treeStrainVisibility: {}, treeStrainColors: {} }; @@ -85,7 +100,7 @@ const treeStrainPropertySelector = (state) => { } return treeStrainProperty; - }, intitialTreeStrainProperty); + }, initialTreeStrainProperty); }; /** @@ -96,14 +111,17 @@ const treeStrainPropertySelector = (state) => { * treeStrainVisibility object for strain. * * Returns the active filters object and the filtered measurements - * @param {Array} measurements - * @param {Object} treeStrainVisibility - * @param {Object} filters - * @returns {Object} */ -const filterMeasurements = (measurements, treeStrainVisibility, filters) => { +const filterMeasurements = ( + measurements: Measurement[], + treeStrainVisibility: TreeStrainVisibility, + filters: MeasurementFilters +): { + activeFilters: {string?: MeasurementMetadata[]} + filteredMeasurements: Measurement[] +} => { // Find active filters to filter measurements - const activeFilters = {}; + const activeFilters: {string?: MeasurementMetadata[]} = {}; Object.entries(filters).forEach(([field, valuesMap]) => { activeFilters[field] = activeFilters[field] || []; valuesMap.forEach(({active}, fieldValue) => { @@ -128,25 +146,25 @@ const filterMeasurements = (measurements, treeStrainVisibility, filters) => { const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { // Use `lodash.isEqual` to deep compare object states to prevent unnecessary re-renderings of the component - const { treeStrainVisibility, treeStrainColors } = useSelector((state) => treeStrainPropertySelector(state), isEqual); - const legendValues = useSelector((state) => state.controls.colorScale.legendValues, isEqual); - const colorings = useSelector((state) => state.metadata.colorings); - const colorBy = useSelector((state) => state.controls.colorBy); - const groupBy = useSelector((state) => state.controls.measurementsGroupBy); - const filters = useSelector((state) => state.controls.measurementsFilters); - const display = useSelector((state) => state.controls.measurementsDisplay); - const showOverallMean = useSelector((state) => state.controls.measurementsShowOverallMean); - const showThreshold = useSelector((state) => state.controls.measurementsShowThreshold); - const collection = useSelector((state) => state.measurements.collectionToDisplay, isEqual); + const { treeStrainVisibility, treeStrainColors } = useSelector((state: RootState) => treeStrainPropertySelector(state), isEqual); + const legendValues = useSelector((state: RootState) => state.controls.colorScale.legendValues, isEqual); + const colorings = useSelector((state: RootState) => state.metadata.colorings); + const colorBy = useSelector((state: RootState) => state.controls.colorBy); + const groupBy = useSelector((state: RootState) => state.controls.measurementsGroupBy); + const filters = useSelector((state: RootState) => state.controls.measurementsFilters); + const display = useSelector((state: RootState) => state.controls.measurementsDisplay); + const showOverallMean = useSelector((state: RootState) => state.controls.measurementsShowOverallMean); + const showThreshold = useSelector((state: RootState) => state.controls.measurementsShowThreshold); + const collection = useSelector((state: RootState) => state.measurements.collectionToDisplay, isEqual); const { title, x_axis_label, thresholds, fields, measurements, groupings } = collection; // Ref to access the D3 SVG - const svgContainerRef = useRef(null); - const d3Ref = useRef(null); - const d3XAxisRef = useRef(null); + const svgContainerRef: MutableRefObject = useRef(null); + const d3Ref: MutableRefObject = useRef(null); + const d3XAxisRef: MutableRefObject = useRef(null); // State for storing data for the HoverPanel - const [hoverData, setHoverData] = useState(null); + const [hoverData, setHoverData] = useState(null); // Filter and group measurements const {activeFilters, filteredMeasurements} = filterMeasurements(measurements, treeStrainVisibility, filters); @@ -182,7 +200,13 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { groupedMeasurements }); // Cache handleHover function to avoid extra useEffect calls - const handleHover = useCallback((data, dataType, mouseX, mouseY, colorByAttr=null) => { + const handleHover = useCallback(( + data: Measurement | { mean: number, standardDeviation: number }, + dataType: "measurement" | "mean", + mouseX: number, + mouseY: number, + colorByAttr: string = null + ): void => { let newHoverData = null; if (data !== null) { // Set color-by attribute as title if provided @@ -255,7 +279,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { toggleDisplay(d3Ref.current, "threshold", showThreshold); }, [svgData, showThreshold]); - const getSVGContainerStyle = () => { + const getSVGContainerStyle = (): CSSProperties => { return { overflowY: "auto", position: "relative", @@ -268,7 +292,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { * Sticky x-axis with a set height to make sure the x-axis is always * at the bottom of the measurements panel */ - const getStickyXAxisSVGStyle = () => { + const getStickyXAxisSVGStyle = (): CSSProperties => { return { width: "100%", height: layout.xAxisHeight, @@ -282,7 +306,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { * allow x-axis to fit in the bottom of the panel when scrolling all the way * to the bottom of the measurements SVG */ - const getMainSVGStyle = () => { + const getMainSVGStyle = (): CSSProperties => { return { width: "100%", position: "relative", @@ -320,9 +344,9 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { }; const Measurements = ({height, width, showLegend}) => { - const measurementsLoaded = useSelector((state) => state.measurements.loaded); - const measurementsError = useSelector((state) => state.measurements.error); - const showOnlyPanels = useSelector((state) => state.controls.showOnlyPanels); + const measurementsLoaded = useSelector((state: RootState) => state.measurements.loaded); + const measurementsError = useSelector((state: RootState) => state.measurements.error); + const showOnlyPanels = useSelector((state: RootState) => state.controls.showOnlyPanels); const [title, setTitle] = useState("Measurements"); diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index c04271d0c..81c3c6c9f 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -13,6 +13,7 @@ import { calcBrowserDimensionsInitialState } from "./browserDimensions"; import { doesColorByHaveConfidence } from "../actions/recomputeReduxState"; import { hasMultipleGridPanels } from "../actions/panelDisplay"; import { Distance } from "../components/tree/phyloTree/types"; +import { MeasurementMetadata, MeasurementsDisplay } from "./measurements/types"; export interface ColorScale { @@ -151,14 +152,15 @@ export interface BasicControlsState { zoomMin?: number } +export interface MeasurementFilters { + [key: string]: Map +} export interface MeasurementsControlState { measurementsGroupBy: string | undefined, - measurementsDisplay: string | undefined, + measurementsDisplay: MeasurementsDisplay | undefined, measurementsShowOverallMean: boolean | undefined, measurementsShowThreshold: boolean | undefined, - measurementsFilters: { - [key: string]: Map - } + measurementsFilters: MeasurementFilters } export interface ControlsState extends BasicControlsState, MeasurementsControlState {} diff --git a/src/reducers/measurements/index.ts b/src/reducers/measurements/index.ts index f7805e951..c8d551669 100644 --- a/src/reducers/measurements/index.ts +++ b/src/reducers/measurements/index.ts @@ -1,10 +1,12 @@ +import { AnyAction } from "@reduxjs/toolkit"; import { CHANGE_MEASUREMENTS_COLLECTION, CLEAN_START, URL_QUERY_CHANGE_WITH_COMPUTED_STATE -} from "../actions/types"; +} from "../../actions/types"; +import { MeasurementsState } from "./types"; -export const getDefaultMeasurementsState = () => ({ +export const getDefaultMeasurementsState = (): MeasurementsState => ({ error: undefined, loaded: false, defaultCollectionKey: "", @@ -12,7 +14,10 @@ export const getDefaultMeasurementsState = () => ({ collectionToDisplay: {} }); -const measurements = (state = getDefaultMeasurementsState(), action) => { +const measurements = ( + state: MeasurementsState = getDefaultMeasurementsState(), + action: AnyAction, +): MeasurementsState => { switch (action.type) { case CLEAN_START: // fallthrough case URL_QUERY_CHANGE_WITH_COMPUTED_STATE: diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts new file mode 100644 index 000000000..d4c9edce7 --- /dev/null +++ b/src/reducers/measurements/types.ts @@ -0,0 +1,79 @@ +import { measurementIdSymbol } from "../../util/globals"; + +// -- Measurements JSON types -- // + +/** + * Measurements are allowed to have arbitrary metadata. + * Matching types allowed in Augur's measurements schema + * + */ +export type MeasurementMetadata = string | number | boolean +export type MeasurementsDisplay = 'raw' | 'mean' + +interface JsonMeasurement { + readonly strain: string + readonly value: number + readonly [key: string]: MeasurementMetadata +} + +interface JsonCollectionDisplayDefaults { + readonly group_by?: string + readonly measurements_display?: MeasurementsDisplay + readonly show_overall_mean?: boolean + readonly show_threshold?: boolean +} + +interface JsonCollectionField { + readonly key: string + readonly title?: string +} + +interface JsonCollectionGrouping { + readonly key: string + readonly order?: MeasurementMetadata[] +} + +export interface JsonCollection { + readonly display_defaults?: JsonCollectionDisplayDefaults + readonly fields?: JsonCollectionField[] + readonly filters?: string[] + readonly groupings: JsonCollectionGrouping[] + readonly key: string + readonly measurements: JsonMeasurement[] + readonly threshold?: number + readonly thresholds?: number[] + readonly title?: string + readonly x_axis_label: string +} + +export interface MeasurementsJson { + readonly collections: JsonCollection[] + readonly default_collection?: string +} + +// -- Measurements state types -- // + +export interface Measurement extends JsonMeasurement { + [measurementIdSymbol]: number +} + +export interface Collection { + // TODO: Convert this to MeasurementsControlState during parseMeasurementsJSON + display_defaults?: JsonCollectionDisplayDefaults + fields: Map + filters: Map}> + groupings: Map + key: string + measurements: Measurement[] + thresholds?: number[] + title?: string + x_axis_label: string +} + +export interface MeasurementsState { + loaded: boolean + error: string | undefined + collections: Collection[] | undefined + collectionToDisplay: Collection | undefined + defaultCollectionKey: string | undefined +} From 2361e6747c00b58cb0ab1a343f587016d7972ec4 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 6 Dec 2024 16:03:04 -0800 Subject: [PATCH 06/22] Update default measurements state to `undefined` values Instead of using empty placeholders, be explicit with `undefined` to show lack of collections data. --- src/reducers/measurements/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/reducers/measurements/index.ts b/src/reducers/measurements/index.ts index c8d551669..4acfa0cdd 100644 --- a/src/reducers/measurements/index.ts +++ b/src/reducers/measurements/index.ts @@ -9,9 +9,9 @@ import { MeasurementsState } from "./types"; export const getDefaultMeasurementsState = (): MeasurementsState => ({ error: undefined, loaded: false, - defaultCollectionKey: "", - collections: [], - collectionToDisplay: {} + defaultCollectionKey: undefined, + collections: undefined, + collectionToDisplay: undefined }); const measurements = ( From 1b5926364b484a24667d4746894a5fc2399eeea5 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 6 Dec 2024 16:08:55 -0800 Subject: [PATCH 07/22] CHANGE_MEASUREMENTS_COLLECTION: only update if `loaded` Reflect reality where we only expect to change the measurement collection after all of the measurement data have been properly `loaded`. --- src/reducers/measurements/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/reducers/measurements/index.ts b/src/reducers/measurements/index.ts index 4acfa0cdd..6236c484d 100644 --- a/src/reducers/measurements/index.ts +++ b/src/reducers/measurements/index.ts @@ -23,11 +23,13 @@ const measurements = ( case URL_QUERY_CHANGE_WITH_COMPUTED_STATE: return { ...action.measurements }; case CHANGE_MEASUREMENTS_COLLECTION: - return { - ...state, - loaded: true, - collectionToDisplay: action.collectionToDisplay - }; + if (state.loaded) { + return { + ...state, + collectionToDisplay: action.collectionToDisplay + }; + } + return state; default: return state; } From 21ef2b432c7e48469d2bf0c5f2f204e326b540d3 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 15 Nov 2024 17:27:59 -0800 Subject: [PATCH 08/22] measurementsD3: Fix JSDoc for type error --- src/components/measurements/measurementsD3.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/measurements/measurementsD3.js b/src/components/measurements/measurementsD3.js index fe69e0be4..a8a6ffab9 100644 --- a/src/components/measurements/measurementsD3.js +++ b/src/components/measurements/measurementsD3.js @@ -104,7 +104,7 @@ export const createYScale = () => { * The groups are sorted by the order of values in the provided groupByValueOrder. * @param {Array} measurements * @param {string} groupBy - * @param {Array} groupByValueOrder + * @param {Array} groupByValueOrder * @returns {Array>} */ export const groupMeasurements = (measurements, groupBy, groupByValueOrder) => { From c9a309e4b85ca306874465622b80afdc5fe51668 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 14:09:33 -0800 Subject: [PATCH 09/22] Add proper type checking in handleHover --- src/components/measurements/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/measurements/index.tsx b/src/components/measurements/index.tsx index aec8c0ea2..a8585dcc3 100644 --- a/src/components/measurements/index.tsx +++ b/src/components/measurements/index.tsx @@ -1,7 +1,7 @@ import React, { CSSProperties, MutableRefObject, useCallback, useRef, useEffect, useMemo, useState } from "react"; import { useSelector } from "react-redux"; import { isEqual, orderBy } from "lodash"; -import { NODE_VISIBLE } from "../../util/globals"; +import { measurementIdSymbol, NODE_VISIBLE } from "../../util/globals"; import { getColorByTitle, getTipColorAttribute } from "../../util/colorHelpers"; import { determineLegendMatch } from "../../util/tipRadiusHelpers"; import ErrorBoundary from "../../util/errorBoundary"; @@ -213,7 +213,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { const hoverTitle = colorByAttr !== null ? `Color by ${getColorByTitle(colorings, colorBy)} : ${colorByAttr}` : null; // Create a Map of data to save order of fields const newData = new Map(); - if (dataType === "measurement") { + if (dataType === "measurement" && measurementIdSymbol in data) { // Handle single measurement data // Filter out internal auspice fields (i.e. measurementsJitter and measurementsId) const displayFields = Object.keys(data).filter((field) => fields.has(field)); @@ -223,7 +223,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { orderedFields.forEach((field) => { newData.set(fields.get(field).title, data[field]); }); - } else if (dataType === "mean") { + } else if (dataType === "mean" && !(measurementIdSymbol in data)) { // Handle mean and standard deviation data newData.set("mean", data.mean.toFixed(2)); newData.set("standard deviation", data.standardDeviation ? data.standardDeviation.toFixed(2) : "N/A"); From a932abbb6074d8913b9510f9b0dfc4fba46b9572 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 10:28:19 -0800 Subject: [PATCH 10/22] Replace `lodash.pick` for proper type assignment Use of `lodash.pick` causes type error ``` src/actions/measurements.ts(211,3): error TS2322: Type 'Partial' is not assignable to type 'MeasurementsControlState'. Property 'measurementsGroupBy' is optional in type 'Partial' but required in type 'MeasurementsControlState'. ``` Instead of using type casting, just replace `lodash.pick` for the proper type assignment. --- src/actions/measurements.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 4852b6bb5..e276e9642 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -1,4 +1,4 @@ -import { cloneDeep, pick } from "lodash"; +import { cloneDeep } from "lodash"; import { AppDispatch, ThunkFunction } from "../store"; import { measurementIdSymbol } from "../util/globals"; import { ControlsState, defaultMeasurementsControlState, MeasurementsControlState } from "../reducers/controls"; @@ -167,7 +167,12 @@ const getCollectionDisplayControls = ( collection: Collection ): MeasurementsControlState => { // Copy current control options for measurements - const newControls = cloneDeep(pick(controls, Object.keys(defaultMeasurementsControlState))); + const newControls = cloneDeep(defaultMeasurementsControlState); + Object.entries(controls).forEach(([key, value]) => { + if (key in newControls) { + newControls[key] = cloneDeep(value); + } + }) // Checks the current group by is available as a grouping in collection // If it doesn't exist, set to undefined so it will get filled in with collection's default if (!collection.groupings.has(newControls.measurementsGroupBy)) { From 78e4761d2c555512591b4112dc70c882ade723ec Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 12:42:22 -0800 Subject: [PATCH 11/22] Build parsed collection instead of directly editing object Fix many type errors that resulted from mismatched types in `JsonCollection` and `Collection`. --- src/actions/measurements.ts | 48 ++++++++++++++++++++---------- src/reducers/measurements/types.ts | 11 +++++++ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index e276e9642..108de0f90 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -14,6 +14,7 @@ import { } from "./types"; import { Collection, + isCollection, MeasurementMetadata, MeasurementsDisplay, MeasurementsJson, @@ -210,22 +211,30 @@ const getCollectionDisplayControls = ( }; const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { - // Avoid editing the original json values since they are cached for narratives - const collections = cloneDeep(json["collections"]); - if (!collections || collections.length === 0) { + const jsonCollections = json["collections"]; + if (!jsonCollections || jsonCollections.length === 0) { throw new Error("Measurements JSON does not have collections"); } - collections.forEach((collection) => { + // Collection properties with the same type as JsonCollection properties. + const propertiesWithSameType = ["key", "x_axis_label", "display_defaults", "thresholds", "title"]; + + const collections = jsonCollections.map((jsonCollection) => { + const collection: Partial = {}; + // Check for properties with the same type that can be directly copied + for (const collectionProp of propertiesWithSameType) { + if (collectionProp in jsonCollection) { + collection[collectionProp] = cloneDeep(jsonCollection[collectionProp]); + } + } /** * Keep backwards compatibility with single value threshold. * Make sure thresholds are an array of values so that we don't have to * check the data type in the D3 drawing process * `collection.thresholds` takes precedence over the deprecated `collection.threshold` */ - if (typeof collection.threshold === "number") { - collection.thresholds = collection.thresholds || [collection.threshold]; - delete collection.threshold; + if (typeof jsonCollection.threshold === "number") { + collection.thresholds = collection.thresholds || [jsonCollection.threshold]; } /* * Create fields Map for easier access of titles and to keep ordering @@ -233,7 +242,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { * Then loop over measurements to add any remaining fields */ collection.fields = new Map( - (collection.fields || []) + (jsonCollection.fields || []) .map(({key, title}) => [key, {title: title || key}]) ); @@ -243,21 +252,21 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { * Then loop over measurements to add values * If there are no JSON defined filters, then add all fields as filters */ - const collectionFiltersArray = collection.filters; + const collectionFiltersArray = jsonCollection.filters; collection.filters = new Map( - (collection.filters || []) + (jsonCollection.filters || []) .map((filterField) => [filterField, {values: new Set()}]) ); // Create a temp object for groupings to keep track of values and their // counts so that we can create a stable default order for grouping field values - const groupingsValues: GroupingValues = collection.groupings.reduce((tempObject, {key}) => { + const groupingsValues: GroupingValues = jsonCollection.groupings.reduce((tempObject, {key}) => { tempObject[key] = {}; return tempObject; }, {}); - collection.measurements.forEach((measurement, index) => { - Object.entries(measurement).forEach(([field, fieldValue]) => { + collection.measurements = jsonCollection.measurements.map((jsonMeasurement, index) => { + Object.entries(jsonMeasurement).forEach(([field, fieldValue]) => { // Add remaining field titles if (!collection.fields.has(field)) { collection.fields.set(field, {title: field}); @@ -279,13 +288,16 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { }); // Add stable id for each measurement to help visualization - measurement[measurementIdSymbol] = index; + return { + ...cloneDeep(jsonMeasurement), + [measurementIdSymbol]: index + } }); // Create groupings Map for easier access of sorted values and to keep groupings ordering // Must be done after looping through measurements to build `groupingsValues` object collection.groupings = new Map( - collection.groupings.map(({key, order}) => { + jsonCollection.groupings.map(({key, order}) => { const valuesByCount = Object.entries(groupingsValues[key]) // Use the grouping values' counts to sort the values, highest count first .sort(([, valueCountA], [, valueCountB]) => valueCountB - valueCountA) @@ -300,6 +312,12 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { ]; }) ); + + if (isCollection(collection)) { + return collection; + } else { + throw new Error("Could not parse a valid collection"); + } }); const collectionKeys = collections.map((collection) => collection.key); diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts index d4c9edce7..c27eabea2 100644 --- a/src/reducers/measurements/types.ts +++ b/src/reducers/measurements/types.ts @@ -70,6 +70,17 @@ export interface Collection { x_axis_label: string } +export function isCollection(x: any): x is Collection { + return Boolean( + x.fields && + x.filters && + x.groupings && + x.key && + x.measurements && + x.x_axis_label + ); +} + export interface MeasurementsState { loaded: boolean error: string | undefined From adec1ae90ed6886fde829ef8854fd79b8a305eda Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 12:50:20 -0800 Subject: [PATCH 12/22] Update `GroupingValues` type to use Map objects Resolves type error when using a plain object to keep counts of grouping values: ``` src/actions/measurements.ts(292,56): error TS2538: Type 'false' cannot be used as an index type. ``` --- src/actions/measurements.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 108de0f90..83763a2e9 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -26,9 +26,7 @@ import { * we can create a stable default order for grouping field values */ interface GroupingValues { - [key: string]: { - [key: string]: number - } + [key: string]: Map } @@ -261,7 +259,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Create a temp object for groupings to keep track of values and their // counts so that we can create a stable default order for grouping field values const groupingsValues: GroupingValues = jsonCollection.groupings.reduce((tempObject, {key}) => { - tempObject[key] = {}; + tempObject[key] = new Map(); return tempObject; }, {}); @@ -282,8 +280,8 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Save grouping field values and counts if (field in groupingsValues) { - const previousValue = groupingsValues[field][fieldValue]; - groupingsValues[field][fieldValue] = previousValue ? previousValue + 1 : 1; + const previousValue = groupingsValues[field].get(fieldValue); + groupingsValues[field].set(fieldValue, previousValue ? previousValue + 1 : 1); } }); @@ -298,7 +296,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Must be done after looping through measurements to build `groupingsValues` object collection.groupings = new Map( jsonCollection.groupings.map(({key, order}) => { - const valuesByCount = Object.entries(groupingsValues[key]) + const valuesByCount = [...groupingsValues[key].entries()] // Use the grouping values' counts to sort the values, highest count first .sort(([, valueCountA], [, valueCountB]) => valueCountB - valueCountA) // Filter out values that already exist in provided order from JSON From 728682d149a74f75efa1284851064737aaefbaf8 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 13:26:12 -0800 Subject: [PATCH 13/22] Add proper type check of `queryValue` --- src/actions/measurements.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 83763a2e9..25021e64a 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -621,7 +621,7 @@ export const combineMeasurementsControlsAndQuery = ( break; case "m_groupBy": // Verify value is a valid grouping of collection - if (collectionGroupings.includes(queryValue)) { + if (typeof queryValue === "string" && collectionGroupings.includes(queryValue)) { newControlState = queryValue; } break; From de428f17a8669159c31118daf425f5da75c137c2 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 13:56:52 -0800 Subject: [PATCH 14/22] Update `Query` type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the `Query` type to reflect the returned type of `querystring.parse`.¹ This led me to realize that there's a currently a bug in the measurement filter query param matching when filtering using values that are _not_ a string the raw measurement metadata. This will be fixed in the following commit. ¹ --- src/actions/measurements.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 25021e64a..0aa29bfbc 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -43,9 +43,13 @@ interface MeasurementsQuery { m_threshold?: QueryBoolean [key: MeasurementsFilterQuery]: string[] } -/* Central Query type placeholder! */ +/** + * Central Query type placeholder! + * Expected to be the returned object from querystring.parse() + * https://nodejs.org/docs/latest-v22.x/api/querystring.html#querystringparsestr-sep-eq-options + */ interface Query extends MeasurementsQuery { - [key: string]: unknown + [key: string]: string | string[] } /** @@ -658,8 +662,11 @@ export const combineMeasurementsControlsAndQuery = ( } // Remove and ignore query for invalid field values + let filterValues = updatedQuery[filterKey]; + if (typeof filterValues === "string") { + filterValues = Array(filterValues); + } const collectionFieldValues = collectionToDisplay.filters.get(field).values; - const filterValues = Array.isArray(updatedQuery[filterKey]) ? updatedQuery[filterKey] : [updatedQuery[filterKey]]; const validFilterValues = filterValues.filter((value) => collectionFieldValues.has(value)); if (!validFilterValues.length) { delete updatedQuery[filterKey]; From 7a3cc108146303f3e20087f1b4e25b82fe42eb4a Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 16:13:14 -0800 Subject: [PATCH 15/22] Convert all measurements metadata to string during parsing of JSON Because the query params are always parsed as strings, the measurements URL query `mf_*` will fail if the measurements' raw data are not strings. Instead of doing string transformations throughout the code, just convert all measurements metadata to strings during parsing of the JSON. I've left the measurement `value` field as a number because converting it to string resulted in a lot of calculation errors in D3. I will revisit this in the future when I add types to the measurementsD3 file. Renamed `MeasurementMetadata` to `JsonMeasurementMetadata` to distinguish from `Measurement.[key: string]` as suggested by @victorlin in review --- src/actions/measurements.ts | 73 ++++++++++++------- src/components/measurements/index.tsx | 12 ++- src/components/measurements/measurementsD3.js | 2 +- src/reducers/controls.ts | 4 +- src/reducers/measurements/types.ts | 24 ++++-- 5 files changed, 75 insertions(+), 40 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 0aa29bfbc..4c21873d8 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -15,7 +15,8 @@ import { import { Collection, isCollection, - MeasurementMetadata, + isMeasurement, + Measurement, MeasurementsDisplay, MeasurementsJson, MeasurementsState @@ -26,7 +27,7 @@ import { * we can create a stable default order for grouping field values */ interface GroupingValues { - [key: string]: Map + [key: string]: Map } @@ -185,7 +186,7 @@ const getCollectionDisplayControls = ( // Verify that current filters are valid for the new collection newControls.measurementsFilters = Object.fromEntries( Object.entries(newControls.measurementsFilters) - .map(([field, valuesMap]): [string, Map] => { + .map(([field, valuesMap]): [string, Map] => { // Clone nested Map to avoid changing redux state in place // Delete filter for values that do not exist within the field of the new collection const newValuesMap = new Map([...valuesMap].filter(([value]) => { @@ -268,31 +269,49 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { }, {}); collection.measurements = jsonCollection.measurements.map((jsonMeasurement, index) => { + const parsedMeasurement: Partial = { + [measurementIdSymbol]: index + } Object.entries(jsonMeasurement).forEach(([field, fieldValue]) => { - // Add remaining field titles - if (!collection.fields.has(field)) { - collection.fields.set(field, {title: field}); - } + /** + * Convert all measurements metadata (except the `value`) to strings + * for proper matching with filter queries. + * This does mean the the `value` cannot be used as a field filter. + * We can revisit this decision when adding types to measurementsD3 + * because converting `value` to string resulted in a lot of calculation errors + */ + if (field === "value") { + parsedMeasurement[field] = Number(fieldValue); + } else { + const fieldValueString = fieldValue.toString(); + parsedMeasurement[field] = fieldValueString; + + // Add remaining field titles + if (!collection.fields.has(field)) { + collection.fields.set(field, {title: field}); + } - // Only save the unique values if the field is in defined filters - // OR there are no JSON defined filters, so all fields are filters - if ((collection.filters.has(field)) || !collectionFiltersArray) { - const filterObject = collection.filters.get(field) || { values: new Set()}; - filterObject.values.add(fieldValue); - collection.filters.set(field, filterObject); - } + // Only save the unique values if the field is in defined filters + // OR there are no JSON defined filters, so all fields are filters + if ((collection.filters.has(field)) || !collectionFiltersArray) { + const filterObject = collection.filters.get(field) || { values: new Set()}; + filterObject.values.add(fieldValueString); + collection.filters.set(field, filterObject); + } - // Save grouping field values and counts - if (field in groupingsValues) { - const previousValue = groupingsValues[field].get(fieldValue); - groupingsValues[field].set(fieldValue, previousValue ? previousValue + 1 : 1); + // Save grouping field values and counts + if (field in groupingsValues) { + const previousValue = groupingsValues[field].get(fieldValueString); + groupingsValues[field].set(fieldValueString, previousValue ? previousValue + 1 : 1); + } } }); - // Add stable id for each measurement to help visualization - return { - ...cloneDeep(jsonMeasurement), - [measurementIdSymbol]: index + + if (isMeasurement(parsedMeasurement)) { + return parsedMeasurement; + } else { + throw new Error("Could not parse a valid measurement"); } }); @@ -300,17 +319,19 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Must be done after looping through measurements to build `groupingsValues` object collection.groupings = new Map( jsonCollection.groupings.map(({key, order}) => { + const defaultOrder = order ? order.map(x => x.toString()) : []; const valuesByCount = [...groupingsValues[key].entries()] // Use the grouping values' counts to sort the values, highest count first .sort(([, valueCountA], [, valueCountB]) => valueCountB - valueCountA) // Filter out values that already exist in provided order from JSON - .filter(([fieldValue]) => order ? !order.includes(fieldValue) : true) + .filter(([fieldValue]) => !defaultOrder.includes(fieldValue)) // Create array of field values .map(([fieldValue]) => fieldValue); + return [ key, // Prioritize the provided values order then list values by count - {values: (order || []).concat(valuesByCount)} + {values: (defaultOrder).concat(valuesByCount)} ]; }) ); @@ -392,7 +413,7 @@ export const changeMeasurementsCollection = ( */ export const applyMeasurementFilter = ( field: string, - value: MeasurementMetadata, + value: string, active: boolean ): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); @@ -409,7 +430,7 @@ export const applyMeasurementFilter = ( export const removeSingleFilter = ( field: string, - value: MeasurementMetadata + value: string ): ThunkFunction => (dispatch, getState) => { const { controls, measurements } = getState(); const measurementsFilters = {...controls.measurementsFilters}; diff --git a/src/components/measurements/index.tsx b/src/components/measurements/index.tsx index a8585dcc3..998c9e7b9 100644 --- a/src/components/measurements/index.tsx +++ b/src/components/measurements/index.tsx @@ -28,7 +28,7 @@ import { import { RootState } from "../../store"; import { MeasurementFilters } from "../../reducers/controls"; import { Visibility } from "../../reducers/tree/types"; -import { Measurement, MeasurementMetadata } from "../../reducers/measurements/types"; +import { Measurement } from "../../reducers/measurements/types"; interface TreeStrainVisibility { [strain: string]: Visibility @@ -117,11 +117,11 @@ const filterMeasurements = ( treeStrainVisibility: TreeStrainVisibility, filters: MeasurementFilters ): { - activeFilters: {string?: MeasurementMetadata[]} + activeFilters: {string?: string[]} filteredMeasurements: Measurement[] } => { // Find active filters to filter measurements - const activeFilters: {string?: MeasurementMetadata[]} = {}; + const activeFilters: {string?: string[]} = {}; Object.entries(filters).forEach(([field, valuesMap]) => { activeFilters[field] = activeFilters[field] || []; valuesMap.forEach(({active}, fieldValue) => { @@ -137,7 +137,11 @@ const filterMeasurements = ( if (!isVisible(treeStrainVisibility[measurement.strain])) return false; // Then check that the measurement contains values for all active filters for (const [field, values] of Object.entries(activeFilters)) { - if (values.length > 0 && !values.includes(measurement[field])) return false; + const measurementValue = measurement[field]; + if (values.length > 0 && + ((typeof measurementValue === "string") && !values.includes(measurementValue))){ + return false; + } } return true; }) diff --git a/src/components/measurements/measurementsD3.js b/src/components/measurements/measurementsD3.js index a8a6ffab9..fe69e0be4 100644 --- a/src/components/measurements/measurementsD3.js +++ b/src/components/measurements/measurementsD3.js @@ -104,7 +104,7 @@ export const createYScale = () => { * The groups are sorted by the order of values in the provided groupByValueOrder. * @param {Array} measurements * @param {string} groupBy - * @param {Array} groupByValueOrder + * @param {Array} groupByValueOrder * @returns {Array>} */ export const groupMeasurements = (measurements, groupBy, groupByValueOrder) => { diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index 81c3c6c9f..516fb8065 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -13,7 +13,7 @@ import { calcBrowserDimensionsInitialState } from "./browserDimensions"; import { doesColorByHaveConfidence } from "../actions/recomputeReduxState"; import { hasMultipleGridPanels } from "../actions/panelDisplay"; import { Distance } from "../components/tree/phyloTree/types"; -import { MeasurementMetadata, MeasurementsDisplay } from "./measurements/types"; +import { MeasurementsDisplay } from "./measurements/types"; export interface ColorScale { @@ -153,7 +153,7 @@ export interface BasicControlsState { } export interface MeasurementFilters { - [key: string]: Map + [key: string]: Map } export interface MeasurementsControlState { measurementsGroupBy: string | undefined, diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts index c27eabea2..aacae942d 100644 --- a/src/reducers/measurements/types.ts +++ b/src/reducers/measurements/types.ts @@ -1,5 +1,9 @@ import { measurementIdSymbol } from "../../util/globals"; +// -- Shared Measurements types -- // + +export type MeasurementsDisplay = 'raw' | 'mean' + // -- Measurements JSON types -- // /** @@ -7,13 +11,12 @@ import { measurementIdSymbol } from "../../util/globals"; * Matching types allowed in Augur's measurements schema * */ -export type MeasurementMetadata = string | number | boolean -export type MeasurementsDisplay = 'raw' | 'mean' +type JsonMeasurementMetadata = string | number | boolean interface JsonMeasurement { readonly strain: string readonly value: number - readonly [key: string]: MeasurementMetadata + readonly [key: string]: JsonMeasurementMetadata } interface JsonCollectionDisplayDefaults { @@ -30,7 +33,7 @@ interface JsonCollectionField { interface JsonCollectionGrouping { readonly key: string - readonly order?: MeasurementMetadata[] + readonly order?: JsonMeasurementMetadata[] } export interface JsonCollection { @@ -53,16 +56,23 @@ export interface MeasurementsJson { // -- Measurements state types -- // -export interface Measurement extends JsonMeasurement { +export interface Measurement { [measurementIdSymbol]: number + strain: string + value: number + [key: string]: string | number +} + +export function isMeasurement(x: any): x is Measurement { + return Boolean(x[measurementIdSymbol] !== undefined && x.strain && x.value !== undefined) } export interface Collection { // TODO: Convert this to MeasurementsControlState during parseMeasurementsJSON display_defaults?: JsonCollectionDisplayDefaults fields: Map - filters: Map}> - groupings: Map + filters: Map}> + groupings: Map key: string measurements: Measurement[] thresholds?: number[] From e4519632cf6db95365f5fdcf099756065a9a65e0 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 16:23:06 -0800 Subject: [PATCH 16/22] Consolidate query boolean checks in `isQueryBoolean` --- src/actions/measurements.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 4c21873d8..7e918e2dd 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -34,7 +34,10 @@ interface GroupingValues { /* mf_ correspond to active measurements filters */ const filterQueryPrefix = "mf_"; type MeasurementsFilterQuery = `mf_${string}` -type QueryBoolean = "show" | "hide" + +const queryBooleanValues = ["show", "hide"] as const; +type QueryBoolean = (typeof queryBooleanValues)[number] +export const isQueryBoolean = (x: any): x is QueryBoolean => queryBooleanValues.includes(x) /* Measurements query parameters that are constructed and/or parsed here. */ interface MeasurementsQuery { m_collection?: string @@ -651,13 +654,12 @@ export const combineMeasurementsControlsAndQuery = ( } break; case "m_overallMean": - if (queryValue === "show" || queryValue === "hide") { + if (isQueryBoolean(queryValue)) { newControlState = queryValue === "show"; } break; case "m_threshold": - if (collectionToDisplay.thresholds && - (queryValue === "show" || queryValue === "hide")) { + if (collectionToDisplay.thresholds && isQueryBoolean(queryValue)) { newControlState = queryValue === "show"; } break; From a34a604c9c4a00cbd3f9181879cdb05d19197f10 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 16:27:59 -0800 Subject: [PATCH 17/22] Consolidate measurements display values with MeasurementsDisplay type --- src/actions/measurements.ts | 9 +++++---- src/reducers/measurements/types.ts | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 7e918e2dd..2cf44ca5a 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -16,6 +16,8 @@ import { Collection, isCollection, isMeasurement, + isMeasurementsDisplay, + measurementsDisplayValues, Measurement, MeasurementsDisplay, MeasurementsJson, @@ -108,9 +110,8 @@ function getCollectionDefaultControl( break; } case 'measurementsDisplay': { - const expectedValues = ["mean", "raw"]; - if (defaultControl !== undefined && !expectedValues.includes(defaultControl)) { - console.error(`Ignoring invalid ${displayDefaultKey} value ${defaultControl}, must be one of ${expectedValues}`) + if (defaultControl !== undefined && !isMeasurementsDisplay(defaultControl)) { + console.error(`Ignoring invalid ${displayDefaultKey} value ${defaultControl}, must be one of ${measurementsDisplayValues}`) defaultControl = undefined; } break; @@ -643,7 +644,7 @@ export const combineMeasurementsControlsAndQuery = ( let newControlState = undefined; switch(queryKey) { case "m_display": - if (queryValue === "mean" || queryValue === "raw") { + if (isMeasurementsDisplay(queryValue)) { newControlState = queryValue; } break; diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts index aacae942d..9c882ec76 100644 --- a/src/reducers/measurements/types.ts +++ b/src/reducers/measurements/types.ts @@ -1,11 +1,11 @@ import { measurementIdSymbol } from "../../util/globals"; // -- Shared Measurements types -- // - -export type MeasurementsDisplay = 'raw' | 'mean' +export const measurementsDisplayValues = ["raw", "mean"]; +export type MeasurementsDisplay = (typeof measurementsDisplayValues)[number] +export const isMeasurementsDisplay = (x: any): x is MeasurementsDisplay => measurementsDisplayValues.includes(x); // -- Measurements JSON types -- // - /** * Measurements are allowed to have arbitrary metadata. * Matching types allowed in Augur's measurements schema From 600078f0d1e8fd9704bb279fff83888d8776c676 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 18 Nov 2024 17:11:17 -0800 Subject: [PATCH 18/22] Fix lint errors --- src/actions/measurements.ts | 2 +- src/components/measurements/index.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index 2cf44ca5a..c0b2e0510 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -323,7 +323,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Must be done after looping through measurements to build `groupingsValues` object collection.groupings = new Map( jsonCollection.groupings.map(({key, order}) => { - const defaultOrder = order ? order.map(x => x.toString()) : []; + const defaultOrder = order ? order.map((x) => x.toString()) : []; const valuesByCount = [...groupingsValues[key].entries()] // Use the grouping values' counts to sort the values, highest count first .sort(([, valueCountA], [, valueCountB]) => valueCountB - valueCountA) diff --git a/src/components/measurements/index.tsx b/src/components/measurements/index.tsx index 998c9e7b9..6c0b94b93 100644 --- a/src/components/measurements/index.tsx +++ b/src/components/measurements/index.tsx @@ -54,7 +54,7 @@ function useDeepCompareMemo(value: T): T { ref.current = value; } return ref.current; -}; +} // Checks visibility against global NODE_VISIBLE const isVisible = (visibility: Visibility): boolean => visibility === NODE_VISIBLE; From ec8a3133bf7c25d62da39607a5080fd9689df85c Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 22 Nov 2024 11:43:04 -0800 Subject: [PATCH 19/22] Add return types for various `map` Add explicit return types for various `map` as suggested in review --- src/actions/measurements.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index c0b2e0510..e395b1486 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -226,7 +226,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Collection properties with the same type as JsonCollection properties. const propertiesWithSameType = ["key", "x_axis_label", "display_defaults", "thresholds", "title"]; - const collections = jsonCollections.map((jsonCollection) => { + const collections = jsonCollections.map((jsonCollection): Collection => { const collection: Partial = {}; // Check for properties with the same type that can be directly copied for (const collectionProp of propertiesWithSameType) { @@ -250,7 +250,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { */ collection.fields = new Map( (jsonCollection.fields || []) - .map(({key, title}) => [key, {title: title || key}]) + .map(({key, title}): [string, {title: string}] => [key, {title: title || key}]) ); /** @@ -262,7 +262,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { const collectionFiltersArray = jsonCollection.filters; collection.filters = new Map( (jsonCollection.filters || []) - .map((filterField) => [filterField, {values: new Set()}]) + .map((filterField): [string, {values: Set}] => [filterField, {values: new Set()}]) ); // Create a temp object for groupings to keep track of values and their @@ -272,7 +272,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { return tempObject; }, {}); - collection.measurements = jsonCollection.measurements.map((jsonMeasurement, index) => { + collection.measurements = jsonCollection.measurements.map((jsonMeasurement, index): Measurement => { const parsedMeasurement: Partial = { [measurementIdSymbol]: index } @@ -322,7 +322,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { // Create groupings Map for easier access of sorted values and to keep groupings ordering // Must be done after looping through measurements to build `groupingsValues` object collection.groupings = new Map( - jsonCollection.groupings.map(({key, order}) => { + jsonCollection.groupings.map(({key, order}): [string, {values: string[]}] => { const defaultOrder = order ? order.map((x) => x.toString()) : []; const valuesByCount = [...groupingsValues[key].entries()] // Use the grouping values' counts to sort the values, highest count first From 49ee53c426408a85c20862ba16bdb4304b552e97 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 22 Nov 2024 11:48:28 -0800 Subject: [PATCH 20/22] Update MeasurementsDisplay and QueryBoolean types to be more readable Specifying the values again make them more readable as suggested in review --- src/actions/measurements.ts | 4 ++-- src/reducers/measurements/types.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index e395b1486..a996f84f6 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -37,8 +37,8 @@ interface GroupingValues { const filterQueryPrefix = "mf_"; type MeasurementsFilterQuery = `mf_${string}` -const queryBooleanValues = ["show", "hide"] as const; -type QueryBoolean = (typeof queryBooleanValues)[number] +type QueryBoolean = "show" | "hide" +const queryBooleanValues: QueryBoolean[] = ["show", "hide"]; export const isQueryBoolean = (x: any): x is QueryBoolean => queryBooleanValues.includes(x) /* Measurements query parameters that are constructed and/or parsed here. */ interface MeasurementsQuery { diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts index 9c882ec76..2363ff202 100644 --- a/src/reducers/measurements/types.ts +++ b/src/reducers/measurements/types.ts @@ -1,8 +1,8 @@ import { measurementIdSymbol } from "../../util/globals"; // -- Shared Measurements types -- // -export const measurementsDisplayValues = ["raw", "mean"]; -export type MeasurementsDisplay = (typeof measurementsDisplayValues)[number] +export type MeasurementsDisplay = "raw" | "mean" +export const measurementsDisplayValues: MeasurementsDisplay[] = ["raw", "mean"]; export const isMeasurementsDisplay = (x: any): x is MeasurementsDisplay => measurementsDisplayValues.includes(x); // -- Measurements JSON types -- // From 91a77de5b97b13f6c0b9748e2fbbbe274970df8c Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 22 Nov 2024 13:17:55 -0800 Subject: [PATCH 21/22] Use more robust type narrowing for `Measurement` and `Collection` As suggested in review --- src/actions/measurements.ts | 19 +++++-------------- src/reducers/measurements/types.ts | 29 ++++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/actions/measurements.ts b/src/actions/measurements.ts index a996f84f6..6e4bc5a91 100644 --- a/src/actions/measurements.ts +++ b/src/actions/measurements.ts @@ -14,14 +14,14 @@ import { } from "./types"; import { Collection, - isCollection, - isMeasurement, + asCollection, + asMeasurement, isMeasurementsDisplay, measurementsDisplayValues, Measurement, MeasurementsDisplay, MeasurementsJson, - MeasurementsState + MeasurementsState, } from "../reducers/measurements/types"; /** @@ -311,12 +311,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { } }); - - if (isMeasurement(parsedMeasurement)) { - return parsedMeasurement; - } else { - throw new Error("Could not parse a valid measurement"); - } + return asMeasurement(parsedMeasurement); }); // Create groupings Map for easier access of sorted values and to keep groupings ordering @@ -340,11 +335,7 @@ const parseMeasurementsJSON = (json: MeasurementsJson): MeasurementsState => { }) ); - if (isCollection(collection)) { - return collection; - } else { - throw new Error("Could not parse a valid collection"); - } + return asCollection(collection); }); const collectionKeys = collections.map((collection) => collection.key); diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts index 2363ff202..05616a3a8 100644 --- a/src/reducers/measurements/types.ts +++ b/src/reducers/measurements/types.ts @@ -63,8 +63,16 @@ export interface Measurement { [key: string]: string | number } -export function isMeasurement(x: any): x is Measurement { - return Boolean(x[measurementIdSymbol] !== undefined && x.strain && x.value !== undefined) +export function asMeasurement(x: Partial): Measurement { + if (x[measurementIdSymbol] !== undefined && x.strain && x.value !== undefined) { + return { + ...x, + [measurementIdSymbol]: x[measurementIdSymbol], + strain: x.strain, + value: x.value, + } + } + throw new Error("Measurement is partial."); } export interface Collection { @@ -80,15 +88,26 @@ export interface Collection { x_axis_label: string } -export function isCollection(x: any): x is Collection { - return Boolean( +export function asCollection(x: Partial): Collection { + if ( x.fields && x.filters && x.groupings && x.key && x.measurements && x.x_axis_label - ); + ){ + return { + ...x, + fields: x.fields, + filters: x.filters, + groupings: x.groupings, + key: x.key, + measurements: x.measurements, + x_axis_label: x.x_axis_label, + } + } + throw new Error("Collection is partial."); } export interface MeasurementsState { From 6dfb02a198ef60e9ae9e754b317df385a2c2542a Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 22 Nov 2024 13:50:00 -0800 Subject: [PATCH 22/22] Update `handleHover` data type check to be more readable Removes the redundant `dataType` param and just does type checking directly on the `data` param as discussed in review --- src/components/measurements/index.tsx | 23 +++++++++++++------ src/components/measurements/measurementsD3.js | 2 +- src/reducers/measurements/types.ts | 9 ++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/components/measurements/index.tsx b/src/components/measurements/index.tsx index 6c0b94b93..43be5b9c2 100644 --- a/src/components/measurements/index.tsx +++ b/src/components/measurements/index.tsx @@ -1,7 +1,7 @@ import React, { CSSProperties, MutableRefObject, useCallback, useRef, useEffect, useMemo, useState } from "react"; import { useSelector } from "react-redux"; import { isEqual, orderBy } from "lodash"; -import { measurementIdSymbol, NODE_VISIBLE } from "../../util/globals"; +import { NODE_VISIBLE } from "../../util/globals"; import { getColorByTitle, getTipColorAttribute } from "../../util/colorHelpers"; import { determineLegendMatch } from "../../util/tipRadiusHelpers"; import ErrorBoundary from "../../util/errorBoundary"; @@ -28,8 +28,18 @@ import { import { RootState } from "../../store"; import { MeasurementFilters } from "../../reducers/controls"; import { Visibility } from "../../reducers/tree/types"; -import { Measurement } from "../../reducers/measurements/types"; +import { Measurement, isMeasurement } from "../../reducers/measurements/types"; +interface MeanAndStandardDeviation { + mean: number + standardDeviation: number | undefined +} +function isMeanAndStandardDeviation(x: any): x is MeanAndStandardDeviation { + return ( + typeof x.mean === "number" && + (typeof x.standardDeviation === "number" || x.standardDeviation === undefined) + ) +} interface TreeStrainVisibility { [strain: string]: Visibility } @@ -205,8 +215,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { }); // Cache handleHover function to avoid extra useEffect calls const handleHover = useCallback(( - data: Measurement | { mean: number, standardDeviation: number }, - dataType: "measurement" | "mean", + data: Measurement | MeanAndStandardDeviation, mouseX: number, mouseY: number, colorByAttr: string = null @@ -217,7 +226,7 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { const hoverTitle = colorByAttr !== null ? `Color by ${getColorByTitle(colorings, colorBy)} : ${colorByAttr}` : null; // Create a Map of data to save order of fields const newData = new Map(); - if (dataType === "measurement" && measurementIdSymbol in data) { + if (isMeasurement(data)) { // Handle single measurement data // Filter out internal auspice fields (i.e. measurementsJitter and measurementsId) const displayFields = Object.keys(data).filter((field) => fields.has(field)); @@ -227,13 +236,13 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => { orderedFields.forEach((field) => { newData.set(fields.get(field).title, data[field]); }); - } else if (dataType === "mean" && !(measurementIdSymbol in data)) { + } else if (isMeanAndStandardDeviation(data)) { // Handle mean and standard deviation data newData.set("mean", data.mean.toFixed(2)); newData.set("standard deviation", data.standardDeviation ? data.standardDeviation.toFixed(2) : "N/A"); } else { // Catch unknown data types - console.error(`"Unknown data type for hover panel: ${dataType}`); + console.error(`"Unknown data type for hover panel: ${JSON.stringify(data)}`); // Display provided data without extra ordering or parsing Object.entries(data).forEach(([key, value]) => newData.set(key, value)); } diff --git a/src/components/measurements/measurementsD3.js b/src/components/measurements/measurementsD3.js index fe69e0be4..de4693ed5 100644 --- a/src/components/measurements/measurementsD3.js +++ b/src/components/measurements/measurementsD3.js @@ -453,7 +453,7 @@ export const addHoverPanelToMeasurementsAndMeans = (ref, handleHover, treeStrain } // sets hover data state to trigger the hover panel display - handleHover(d, dataType, clientX, clientY, colorByAttr); + handleHover(d, clientX, clientY, colorByAttr); }) .on("mouseout.hoverPanel", () => handleHover(null)); }; diff --git a/src/reducers/measurements/types.ts b/src/reducers/measurements/types.ts index 05616a3a8..e8c57789a 100644 --- a/src/reducers/measurements/types.ts +++ b/src/reducers/measurements/types.ts @@ -75,6 +75,15 @@ export function asMeasurement(x: Partial): Measurement { throw new Error("Measurement is partial."); } +export function isMeasurement(x: any): x is Measurement { + try { + asMeasurement(x); + return true; + } catch { + return false; + } +} + export interface Collection { // TODO: Convert this to MeasurementsControlState during parseMeasurementsJSON display_defaults?: JsonCollectionDisplayDefaults