Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(fe2): Improve property name display and search functionality in filtering #2396

65 changes: 37 additions & 28 deletions packages/frontend-2/components/viewer/explorer/Filters.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
refreshColorsIfSetOrActiveFilterIsNumeric()
"
>
{{ filter.key }}
{{ getPropertyName(filter.key) }}
</button>
</div>
<div v-if="itemCount < relevantFiltersSearched.length" class="mb-2">
Expand All @@ -95,7 +95,7 @@
<script setup lang="ts">
import { ChevronDownIcon, ChevronUpIcon } from '@heroicons/vue/24/solid'
import { ArrowPathIcon } from '@heroicons/vue/24/outline'
import type { PropertyInfo, StringPropertyInfo } from '@speckle/viewer'
import type { PropertyInfo } from '@speckle/viewer'
import { useFilterUtilities } from '~~/lib/viewer/composables/ui'
import { useMixpanel } from '~~/lib/core/composables/mp'
import {
Expand All @@ -111,6 +111,8 @@ const {
filters: { propertyFilter }
} = useFilterUtilities()

const revitPropertyRegex = /^parameters\./

const showAllFilters = ref(false)

const props = defineProps<{
Expand Down Expand Up @@ -142,7 +144,7 @@ const relevantFilters = computed(() => {
return false
}
// handle revit params: the actual one single value we're interested is in paramters.HOST_BLA BLA_.value, the rest are not needed
if (f.key.startsWith('parameters')) {
if (isRevitProperty(f.key)) {
if (f.key.endsWith('.value')) return true
else return false
}
Expand Down Expand Up @@ -178,11 +180,16 @@ const numericActiveFilter = computed(() =>
const searchString = ref<string | undefined>(undefined)
const relevantFiltersSearched = computed(() => {
if (!searchString.value) return relevantFilters.value
const searchLower = searchString.value.toLowerCase()
// eslint-disable-next-line vue/no-side-effects-in-computed-properties
itemCount.value = 30 // nasty, but yolo - reset max limit on search change
return relevantFilters.value.filter((f) =>
f.key.toLowerCase().includes((searchString.value as string).toLowerCase())
)
return relevantFilters.value.filter((f) => {
const userFriendlyName = getPropertyName(f.key).toLowerCase()
return (
f.key.toLowerCase().includes(searchLower) ||
userFriendlyName.includes(searchLower)
)
})
})

const itemCount = ref(30)
Expand All @@ -192,28 +199,7 @@ const relevantFiltersLimited = computed(() => {
.sort((a, b) => a.key.length - b.key.length)
})

// Too lazy to follow up in here for now, as i think we need a bit of a better strategy in connectors first :/
const title = computed(() => {
const currentFilterKey = activeFilter.value?.key
if (!currentFilterKey) return 'Loading'

if (currentFilterKey === 'level.name') return 'Level Name'
if (currentFilterKey === 'speckle_type') return 'Object Type'

// Handle revit names :/
if (
currentFilterKey.startsWith('parameters.') &&
currentFilterKey.endsWith('.value')
) {
return (
props.filters.find(
(f) => f.key === currentFilterKey.replace('.value', '.name')
) as StringPropertyInfo
).valueGroups[0].value
}

return currentFilterKey
})
const title = computed(() => getPropertyName(activeFilter.value?.key ?? ''))

const colors = computed(() => !!propertyFilter.isApplied.value)

Expand Down Expand Up @@ -248,4 +234,27 @@ const refreshColorsIfSetOrActiveFilterIsNumeric = () => {
// removePropertyFilter()
applyPropertyFilter()
}

const isRevitProperty = (key: string): boolean => {
andrewwallacespeckle marked this conversation as resolved.
Show resolved Hide resolved
return revitPropertyRegex.test(key)
}

const getPropertyName = (key: string): string => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like everywhere where we use this fn we have access to the entire filter object. If we pass that in, instead of just the key, we don't have to do the props.filters.find() bit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing the entire filter object introduced complexity with a fallback mechanism and error handling in title. Let me know if this is enough to stop us merging and I can take another look on Monday but I'm running out of time today

if (!key) return 'Loading'

if (key === 'level.name') return 'Level Name'
if (key === 'speckle_type') return 'Object Type'

if (isRevitProperty(key) && key.endsWith('.value')) {
const correspondingProperty = props.filters.find(
(f) => f.key === key.replace('.value', '.name')
)
if (correspondingProperty && isStringPropertyInfo(correspondingProperty)) {
return correspondingProperty.valueGroups[0]?.value || key
}
}

// Return the key as is for non-Revit properties
return key
}
</script>