Add controlled keywords to score set search filter options.#649
Add controlled keywords to score set search filter options.#649EstelleDa wants to merge 3 commits intorelease-2026.2.1from
Conversation
bencap
left a comment
bencap
left a comment
There was a problem hiding this comment.
Thanks for working on this Estelle. The main comment I have is about the behavior when a user selects a keyword whose value is present across keyword groups, such as 'Other'. We'll need some additional backend changes to fully remedy it.
| @update:model-value="$emit('update:publicationDatabases', $event)" | ||
| /> | ||
| </MvCollapsible> | ||
| <MvCollapsible :open="false" title="Keyword"> |
There was a problem hiding this comment.
| <MvCollapsible :open="false" title="Keyword"> | |
| <MvCollapsible :open="false" title="Keywords"> |
| groupedControlledKeywordOptions(): { key: string; options: FilterOption[] }[] { | ||
| const groups: Record<string, FilterOption[]> = {} | ||
| for (const opt of this.controlledKeywordOptions) { | ||
| const key = opt.groupKey || 'Other' | ||
| if (!groups[key]) { | ||
| groups[key] = [] | ||
| } | ||
| groups[key].push(opt) | ||
| } | ||
| return Object.entries(groups).map(([key, options]) => ({key, options})) | ||
| } |
There was a problem hiding this comment.
I think we should consider ordering this list alphabetically. Right now, selecting a keyword can change the group order and it's a bit jarring from a user perspective.
| watch: { | ||
| // Keep local selections in sync when parent controlledKeywords changes, | ||
| // e.g. from URL or Clear All | ||
| controlledKeywords: { | ||
| immediate: true, | ||
| handler(newVals: string[]) { | ||
| const byGroup: Record<string, string[]> = {} | ||
| for (const opt of this.controlledKeywordOptions) { | ||
| if (newVals.includes(opt.value)) { | ||
| const key = opt.groupKey || 'Other' | ||
| if (!byGroup[key]) byGroup[key] = [] | ||
| byGroup[key].push(opt.value) | ||
| } | ||
| } | ||
| this.groupSelections = byGroup | ||
| } | ||
| } | ||
| }, | ||
|
|
There was a problem hiding this comment.
I believe this block is responsible for a bug where selecting any keyword Other also selects all the other Other keywords from each keyword group.
I think that the most precise way around many Other options existing is to make the a composite identifier. This would look like key::value or MolecularMechanismAssessed::Other. Then before we send to the API, we split on the :: so we can send both the key and the value.
We'd need some additional changes, which I'll put in other comments.
|
|
||
| this.controlledKeywordOptions = (data.controlledKeywords || []).map((option) => ({ | ||
| value: option.value, | ||
| badge: option.count, | ||
| groupKey: option.key |
There was a problem hiding this comment.
Here we could add a title entry which holds just options.value. Then value becomes ${option.key}::${option.value} to disambiguate the keywords across groups.
| databases: filters.filterPublicationDatabases.length > 0 ? filters.filterPublicationDatabases : undefined, | ||
| journals: filters.filterPublicationJournals.length > 0 ? filters.filterPublicationJournals : undefined, | ||
| keywords: filters.filterKeywords.length > 0 ? filters.filterKeywords : undefined | ||
| controlledKeywords: filters.filterControlledKeywords.length > 0 ? filters.filterControlledKeywords : undefined, |
There was a problem hiding this comment.
Before sending to the backend, we'd now need to split on ::
| filters.push({key, value, label: labelFn ? labelFn(value) : value}) | ||
| } | ||
| } | ||
| addFilters('controlledKeywords', this.filterControlledKeywords) |
There was a problem hiding this comment.
We'd need a custom labelFn here that strips the prefix before :: so that we get plain values for display or we could pass through the {key, value} object and extract just value.
2 participants
Front end of VariantEffect/mavedb-api#693.