From aaed72f88ac394bcd15a61ddb4e5755217e7e8b4 Mon Sep 17 00:00:00 2001 From: andrewwallacespeckle Date: Fri, 3 Oct 2025 15:03:08 +0100 Subject: [PATCH] Fixes from code review. mainly lodash --- .../viewer/composables/filtering/dataStore.ts | 4 +- .../viewer/composables/filtering/filtering.ts | 4 +- .../lib/viewer/helpers/filters/utils.ts | 40 ++++++++++++------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/packages/frontend-2/lib/viewer/composables/filtering/dataStore.ts b/packages/frontend-2/lib/viewer/composables/filtering/dataStore.ts index 3f8a2f008..cfef43a5e 100644 --- a/packages/frontend-2/lib/viewer/composables/filtering/dataStore.ts +++ b/packages/frontend-2/lib/viewer/composables/filtering/dataStore.ts @@ -1,5 +1,5 @@ import type { SpeckleObject, TreeNode, Viewer } from '@speckle/viewer' -import { uniq, flatten, compact } from 'lodash-es' +import { uniq, flatten, compact, isString } from 'lodash-es' import { FilterLogic, FilterType, @@ -57,7 +57,7 @@ function processBatchedPropertyUpdates( // Property exists - check if we need to update type due to conflicting evidence if ( existingProperty.type === FilterType.Numeric && - typeof update.value === 'string' && + isString(update.value) && !isValueNumeric(update.value) ) { existingProperty.type = FilterType.String diff --git a/packages/frontend-2/lib/viewer/composables/filtering/filtering.ts b/packages/frontend-2/lib/viewer/composables/filtering/filtering.ts index bf036ebc9..c17a12364 100644 --- a/packages/frontend-2/lib/viewer/composables/filtering/filtering.ts +++ b/packages/frontend-2/lib/viewer/composables/filtering/filtering.ts @@ -231,13 +231,11 @@ export function useFilterUtilities( } const uniqueValues = Array.from(valueToObjectIds.keys()) - const firstValue = uniqueValues[0] const isBooleanProperty = uniqueValues.every((v) => isValueBoolean(v)) && uniqueValues.length <= 2 - const isNumeric = - typeof firstValue === 'number' || uniqueValues.every((v) => isValueNumeric(v)) + const isNumeric = uniqueValues.every((v) => isValueNumeric(v)) if (isBooleanProperty) { return { diff --git a/packages/frontend-2/lib/viewer/helpers/filters/utils.ts b/packages/frontend-2/lib/viewer/helpers/filters/utils.ts index 9bdcfc9de..075e431a9 100644 --- a/packages/frontend-2/lib/viewer/helpers/filters/utils.ts +++ b/packages/frontend-2/lib/viewer/helpers/filters/utils.ts @@ -1,4 +1,5 @@ import { isStringPropertyInfo } from '~/lib/viewer/helpers/sceneExplorer' +import { isNumber, isString, isBoolean, toNumber } from 'lodash-es' import { ExistenceFilterCondition, FilterType, @@ -270,37 +271,44 @@ export const isBooleanProperty = (filter: ExtendedPropertyInfo): boolean => { * Determines if a value should be treated as numeric for filtering */ export const isValueNumeric = (value: unknown): boolean => { - return ( - typeof value === 'number' || - (!isNaN(Number(value)) && String(value) !== '' && !/[a-zA-Z-]/.test(String(value))) - ) + if (isNumber(value)) return Number.isFinite(value) + + if (isString(value)) { + const trimmed = value.trim() + if (trimmed === '') return false + if (/[a-zA-Z-]/.test(trimmed)) return false + + const converted = toNumber(trimmed) + return Number.isFinite(converted) + } + + return false } /** * Determines if a value should be treated as boolean for filtering (case-insensitive) */ export const isValueBoolean = (value: unknown): boolean => { - const str = String(value).toLowerCase() - return str === 'true' || str === 'false' + if (isBoolean(value)) return true + if (isString(value)) { + const str = value.toLowerCase() + return str === 'true' || str === 'false' + } + return false } /** * Checks if a value represents boolean true (case-insensitive) */ export const isValueBooleanTrue = (value: unknown): boolean => { - return ( - value === true || (isValueBoolean(value) && String(value).toLowerCase() === 'true') - ) + return value === true || (isString(value) && value.toLowerCase() === 'true') } /** * Checks if a value represents boolean false (case-insensitive) */ export const isValueBooleanFalse = (value: unknown): boolean => { - return ( - value === false || - (isValueBoolean(value) && String(value).toLowerCase() === 'false') - ) + return value === false || (isString(value) && value.toLowerCase() === 'false') } /** @@ -504,7 +512,11 @@ export const extractNestedProperties = ( function getValueType(value: unknown): string { if (value === null) return 'null' if (Array.isArray(value)) return 'array' - return typeof value + + if (isValueBoolean(value)) return 'boolean' + if (isValueNumeric(value)) return 'number' + + return 'string' } function extractMaterialProperties(