From 7525970ddb0af82fdc58bc4f918d59ea20b122ab Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 8 Sep 2023 16:09:46 +0100 Subject: [PATCH 01/16] fix: LEAP-33: Mimic labeling Taxonomy result as Label - `findLabel()` for starters to use in Label context - fix `selectedLabels` back to act the same for Labels and Taxonomy - fix styling detection --- src/mixins/HighlightMixin.js | 10 ++-------- src/regions/Result.js | 15 +++++---------- src/tags/control/Taxonomy/Taxonomy.js | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/mixins/HighlightMixin.js b/src/mixins/HighlightMixin.js index 7d604be1bf..2223861209 100644 --- a/src/mixins/HighlightMixin.js +++ b/src/mixins/HighlightMixin.js @@ -153,14 +153,8 @@ export const HighlightMixin = types updateSpans() { if (self._hasSpans || (isFF(FF_LSDV_4620_3) && self._spans?.length)) { const lastSpan = self._spans[self._spans.length - 1]; - const label = self.getLabels(); - // label is array, string or null, so check for length - if (!label?.length) { - lastSpan.removeAttribute('data-label'); - } else { - lastSpan.setAttribute('data-label', label); - } + Utils.Selection.applySpanStyles(lastSpan, { label: self.getLabels() }); } }, @@ -274,7 +268,7 @@ export const HighlightMixin = types }, getLabels() { - return self.labeling?.mainValue ?? []; + return (self.labeling?.selectedLabels ?? []).map(label => label.value).join(','); }, getLabelColor() { diff --git a/src/regions/Result.js b/src/regions/Result.js index 4be89f4a53..d1756fa6fd 100644 --- a/src/regions/Result.js +++ b/src/regions/Result.js @@ -140,19 +140,14 @@ const Result = types return self.mainValue?.join(joinstr) || ''; }, + // @todo check all usages of selectedLabels: + // — check usages of non-array values (like `if selectedValues ...`) + // - check empty labels, they should be returned as an array get selectedLabels() { - if (self.type === 'taxonomy') { - const sep = self.from_name.pathseparator; - const join = self.from_name.showfullpath; - - return (self.mainValue || []) - .map(v => join ? v.join(sep) : v.at(-1)) - .map(v => ({ value: v, id: v })); - } if (self.mainValue?.length === 0 && self.from_name.allowempty) { return self.from_name.findLabel(null); } - return self.mainValue?.map(value => self.from_name.findLabel(value)).filter(Boolean); + return self.mainValue?.map(value => self.from_name.findLabel(value)).filter(Boolean) ?? []; }, /** @@ -212,7 +207,7 @@ const Result = types get style() { if (!self.tag) return null; - const fillcolor = self.tag.background || self.tag.parent.fillcolor; + const fillcolor = self.tag.background || self.tag.parent?.fillcolor; if (!fillcolor) return null; const strokecolor = self.tag.background || self.tag.parent.strokecolor; diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index bc127d5baa..8d6e867842 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -266,6 +266,26 @@ const Model = types return findItem(self.items); }, + + /** + * @param {string[]} path saved value from Taxonomy + * @returns quazi-label object to act as Label in most places + */ + findLabel(path) { + let title = ''; + let items = self._items; + + for (const value of path) { + const item = items?.find(item => item.path.at(-1) === value); + + if (!item) return null; + + items = item.children; + title = self.showfullpath && title ? title + self.pathseparator + item.label : item.label; + } + + return { value: title, id: path.join(self.pathseparator) }; + }, })) .actions(self => ({ afterAttach() { From db922ba6acd05e720dac03f56ea0b3a31d4eb639 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 8 Sep 2023 18:13:11 +0100 Subject: [PATCH 02/16] Attempt to respect `showFullPath` in New Taxonomy It should work, but it doesn't --- src/components/NewTaxonomy/NewTaxonomy.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/NewTaxonomy/NewTaxonomy.tsx b/src/components/NewTaxonomy/NewTaxonomy.tsx index 55ce8857ea..617612ba2f 100644 --- a/src/components/NewTaxonomy/NewTaxonomy.tsx +++ b/src/components/NewTaxonomy/NewTaxonomy.tsx @@ -70,6 +70,7 @@ const NewTaxonomy = ({ const [treeData, setTreeData] = useState([]); const separator = options.pathSeparator; const style = { minWidth: options.minWidth ?? 200, maxWidth: options.maxWidth }; + const value = selected.map(path => path.join(separator)).map(path => ({ title: path, value: path, id: path })); useEffect(() => { setTreeData(convert(items, options)); @@ -82,7 +83,7 @@ const NewTaxonomy = ({ return ( path.join(separator))} + value={value} onChange={items => onChange(null, items.map(item => item.value.split(separator)))} loadData={loadData} treeCheckable From 5330810c98cc4c22b1144080f1a090076bf37e21 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Wed, 13 Sep 2023 16:16:11 +0100 Subject: [PATCH 03/16] Fix node type detection temporary fix, should find a proper fix and source of problem later --- src/components/Node/Node.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/Node/Node.tsx b/src/components/Node/Node.tsx index d48cef6709..0785c979af 100644 --- a/src/components/Node/Node.tsx +++ b/src/components/Node/Node.tsx @@ -188,6 +188,9 @@ const NodeMinimal: FC = observer(({ node }) => { }); const useNodeName = (node: any) => { + // @todo sometimes node is control tag, not a region + // @todo and for new taxonomy it can be plain object + if (!node.$treenode) return null; return getType(node).name as keyof typeof NodeViews; }; From 851c2ffcd5d5d7e267694333419db4ac2623d048 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 14 Sep 2023 14:02:13 +0100 Subject: [PATCH 04/16] Fix labeling=true for usual taxonomy (no api) --- src/tags/control/Taxonomy/Taxonomy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index 8d6e867842..40383edfbb 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -273,7 +273,7 @@ const Model = types */ findLabel(path) { let title = ''; - let items = self._items; + let items = self.items; for (const value of path) { const item = items?.find(item => item.path.at(-1) === value); From e96449edd97309c5857b2c80ddb7db61e7036148 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 14 Sep 2023 14:03:27 +0100 Subject: [PATCH 05/16] Fix region removal upon deleting last label Last label couldn't be deleted now, the same as for Labels --- src/tags/control/Taxonomy/Taxonomy.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index 40383edfbb..964dcd6772 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -383,6 +383,9 @@ const Model = types }, onChange(_node, checked) { + // don't remove last label from region + if (self.isLabeling && !checked.length) return; + self.selected = checked.map(s => s.path ?? s); self.maxUsagesReached = self.selected.length >= self.maxusages; self.updateResult(); From f3445eeff3303fb4831d9c1c666cb983f2a1f9a6 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 14 Sep 2023 16:15:38 +0100 Subject: [PATCH 06/16] Add color to Choice for labeling Works only on regions, taxonomy item has no color yet --- src/tags/control/Choice.js | 4 +++- src/tags/control/Taxonomy/Taxonomy.js | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/tags/control/Choice.js b/src/tags/control/Choice.js index 64ad18f7f0..01d2a33f09 100644 --- a/src/tags/control/Choice.js +++ b/src/tags/control/Choice.js @@ -44,8 +44,9 @@ import { HintTooltip } from '../../components/Taxonomy/Taxonomy'; * @param {string} [alias] - Alias for the choice. If used, the alias replaces the choice value in the annotation results. Alias does not display in the interface. * @param {style} [style] - CSS style of the checkbox element * @param {string} [hotkey] - Hotkey for the selection - * @param {string} [html] - can be used to show enriched content[^FF_DEV_2007], it has higher priority than `value`, however `value` will be used in the exported result (should be properly escaped) + * @param {string} [html] - Can be used to show enriched content[^FF_DEV_2007], it has higher priority than `value`, however `value` will be used in the exported result (should be properly escaped) * @param {string} [hint] - Hint for choice on hover[^FF_PROD_309] + * @param {string} [color] - Color for Taxonomy item */ const TagAttrs = types.model({ ...(isFF(FF_DEV_3391) ? { id: types.identifier } : {}), @@ -54,6 +55,7 @@ const TagAttrs = types.model({ value: types.maybeNull(types.string), hotkey: types.maybeNull(types.string), style: types.maybeNull(types.string), + color: types.maybeNull(types.string), ...(isFF(FF_DEV_2007) ? { html: types.maybeNull(types.string) } : {}), ...(isFF(FF_PROD_309) ? { hint: types.maybeNull(types.string) } : {}), }); diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index 964dcd6772..a2cc57a158 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -102,6 +102,7 @@ function traverse(root) { const depth = parents.length; const obj = { label, path, depth, hint }; + if (node.color) obj.color = node.color; if (node.children) { obj.children = visitUnique(node.children, path); } @@ -274,9 +275,10 @@ const Model = types findLabel(path) { let title = ''; let items = self.items; + let item; for (const value of path) { - const item = items?.find(item => item.path.at(-1) === value); + item = items?.find(item => item.path.at(-1) === value); if (!item) return null; @@ -284,7 +286,15 @@ const Model = types title = self.showfullpath && title ? title + self.pathseparator + item.label : item.label; } - return { value: title, id: path.join(self.pathseparator) }; + const label = { value: title, id: path.join(self.pathseparator) }; + + if (item.color) { + // to conform the current format of our Result#style (and it requires parent) + label.background = item.color; + label.parent = {}; + } + + return label; }, })) .actions(self => ({ From bea20270dd866015058815ff48aea55c25da037d Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 15 Sep 2023 15:32:24 +0100 Subject: [PATCH 07/16] Use colors in taxonomy as well --- src/components/NewTaxonomy/NewTaxonomy.tsx | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/components/NewTaxonomy/NewTaxonomy.tsx b/src/components/NewTaxonomy/NewTaxonomy.tsx index 6e486bcbc2..9fef7c3811 100644 --- a/src/components/NewTaxonomy/NewTaxonomy.tsx +++ b/src/components/NewTaxonomy/NewTaxonomy.tsx @@ -15,6 +15,7 @@ type TaxonomyItem = { children?: TaxonomyItem[], origin?: 'config' | 'user' | 'session', hint?: string, + color?: string, }; type AntTaxonomyItem = { @@ -48,12 +49,23 @@ type TaxonomyProps = { }; const convert = (items: TaxonomyItem[], options: TaxonomyOptions): AntTaxonomyItem[] => { - return items.map(item => ({ - title: item.hint ? ( + const enrich = (item: TaxonomyItem) => { + // @todo marginLeft: -4 is good to align labels, but cats them in selected list + const color = (item: TaxonomyItem) => ( + {item.label} + ); + + if (!item.hint) return item.color ? color(item) : item.label; + + return ( - {item.label} + {item.color ? color(item) : {item.label}} - ) : item.label, + ); + }; + + return items.map(item => ({ + title: enrich(item), value: item.path.join(options.pathSeparator), key: item.path.join(options.pathSeparator), isLeaf: item.isLeaf !== false && !item.children, From 9298c956fd4f8fd87c405ac649d6c9837a1e6ff1 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 22 Sep 2023 16:29:02 +0100 Subject: [PATCH 08/16] Simplify styles, make global; move to .styl --- src/components/NewTaxonomy/NewTaxonomy.styl | 3 +++ src/components/NewTaxonomy/NewTaxonomy.tsx | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 src/components/NewTaxonomy/NewTaxonomy.styl diff --git a/src/components/NewTaxonomy/NewTaxonomy.styl b/src/components/NewTaxonomy/NewTaxonomy.styl new file mode 100644 index 0000000000..f034aa4b34 --- /dev/null +++ b/src/components/NewTaxonomy/NewTaxonomy.styl @@ -0,0 +1,3 @@ +:global(.htx-taxonomy-item-color) + padding 4px 4px + border-radius 2px diff --git a/src/components/NewTaxonomy/NewTaxonomy.tsx b/src/components/NewTaxonomy/NewTaxonomy.tsx index 9fef7c3811..3c7a6e0057 100644 --- a/src/components/NewTaxonomy/NewTaxonomy.tsx +++ b/src/components/NewTaxonomy/NewTaxonomy.tsx @@ -3,6 +3,8 @@ import React, { useCallback, useEffect, useState } from 'react'; import { Tooltip } from '../../common/Tooltip/Tooltip'; +import './NewTaxonomy.styl'; + type TaxonomyPath = string[]; type onAddLabelCallback = (path: string[]) => any; type onDeleteLabelCallback = (path: string[]) => any; @@ -52,7 +54,11 @@ const convert = (items: TaxonomyItem[], options: TaxonomyOptions): AntTaxonomyIt const enrich = (item: TaxonomyItem) => { // @todo marginLeft: -4 is good to align labels, but cats them in selected list const color = (item: TaxonomyItem) => ( - {item.label} + // no BEM here to make it more lightweight + // global classname to allow to change it in Style tag + + {item.label} + ); if (!item.hint) return item.color ? color(item) : item.label; From 3398db3c982a75ecbfd69ca32bca385ef0d173a6 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 22 Sep 2023 18:34:13 +0100 Subject: [PATCH 09/16] typo --- src/components/NewTaxonomy/NewTaxonomy.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/NewTaxonomy/NewTaxonomy.tsx b/src/components/NewTaxonomy/NewTaxonomy.tsx index 3c7a6e0057..b4233e5b76 100644 --- a/src/components/NewTaxonomy/NewTaxonomy.tsx +++ b/src/components/NewTaxonomy/NewTaxonomy.tsx @@ -52,7 +52,7 @@ type TaxonomyProps = { const convert = (items: TaxonomyItem[], options: TaxonomyOptions): AntTaxonomyItem[] => { const enrich = (item: TaxonomyItem) => { - // @todo marginLeft: -4 is good to align labels, but cats them in selected list + // @todo marginLeft: -4 is good to align labels, but cuts them in selected list const color = (item: TaxonomyItem) => ( // no BEM here to make it more lightweight // global classname to allow to change it in Style tag From d49468e4c2add9cc6abb1f2d735835ac080b1179 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 29 Sep 2023 11:56:24 +0100 Subject: [PATCH 10/16] Fix last item removal for old taxonomy as well --- src/components/Taxonomy/Taxonomy.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Taxonomy/Taxonomy.tsx b/src/components/Taxonomy/Taxonomy.tsx index d649a22e2c..a15e7433c6 100644 --- a/src/components/Taxonomy/Taxonomy.tsx +++ b/src/components/Taxonomy/Taxonomy.tsx @@ -15,7 +15,7 @@ import { LsChevron } from '../../assets/icons'; import TreeStructure from '../TreeStructure/TreeStructure'; import styles from './Taxonomy.module.scss'; -import { FF_DEV_4075, FF_PROD_309, isFF } from '../../utils/feature-flags'; +import { FF_DEV_4075, FF_PROD_309, FF_TAXONOMY_LABELING, isFF } from '../../utils/feature-flags'; import { Tooltip } from '../../common/Tooltip/Tooltip'; import { CNTagName } from '../../utils/bem'; @@ -504,6 +504,9 @@ const Taxonomy = ({ const setSelected = (path: TaxonomyPath, value: boolean) => { const newSelected = value ? [...selected, path] : selected.filter(current => !isArraysEqual(current, path)); + // don't remove last item when taxonomy is used as labeling tool + if (isFF(FF_TAXONOMY_LABELING) && !newSelected.length) return; + setInternalSelected(newSelected); onChange && onChange(null, newSelected); }; From fa640a9bb58bf1735322f5d3fea179b401a04c50 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Mon, 2 Oct 2023 15:05:02 +0100 Subject: [PATCH 11/16] Allow to remove items when region is not selected --- src/components/Taxonomy/Taxonomy.tsx | 4 +++- src/tags/control/Taxonomy/Taxonomy.js | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/Taxonomy/Taxonomy.tsx b/src/components/Taxonomy/Taxonomy.tsx index a15e7433c6..38dc4384c6 100644 --- a/src/components/Taxonomy/Taxonomy.tsx +++ b/src/components/Taxonomy/Taxonomy.tsx @@ -33,6 +33,7 @@ type TaxonomyItem = { }; type TaxonomyOptions = { + canRemoveItems?: boolean, leafsOnly?: boolean, showFullPath?: boolean, pathSeparator?: string, @@ -505,7 +506,8 @@ const Taxonomy = ({ const newSelected = value ? [...selected, path] : selected.filter(current => !isArraysEqual(current, path)); // don't remove last item when taxonomy is used as labeling tool - if (isFF(FF_TAXONOMY_LABELING) && !newSelected.length) return; + // canRemoveItems is undefined when FF is off; false only when region is active + if (options.canRemoveItems === false && !newSelected.length) return; setInternalSelected(newSelected); onChange && onChange(null, newSelected); diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index 8ddd26da9a..a1c2e4a0e2 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -142,6 +142,10 @@ const TaxonomyLabelingResult = types return self.annotation.results.find(r => r.from_name === self && r.area === area); }, + get canRemoveItems() { + if (!self.isLabeling) return true; + return !self.result; + }, })) .actions(self => { const Super = { @@ -504,6 +508,7 @@ const HtxTaxonomy = observer(({ item }) => { minWidth: item.minwidth, dropdownWidth: item.dropdownwidth, placeholder: item.placeholder, + canRemoveItems: item.canRemoveItems, }; return ( From a8751e0cd6c183b3beac344d4f6833ce3f38b527 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 3 Oct 2023 16:00:54 +0100 Subject: [PATCH 12/16] Also fix this last item for new taxonomy --- src/tags/control/Taxonomy/Taxonomy.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index a1c2e4a0e2..c8023c6e00 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -409,8 +409,9 @@ const Model = types }, onChange(_node, checked) { - // don't remove last label from region - if (self.isLabeling && !checked.length) return; + // don't remove last label from region if region is selected (so canRemoveItems is false) + // should be checked only for Taxonomy as labbeling tool + if (self.canRemoveItems === false && !checked.length) return; self.selected = checked.map(s => s.path ?? s); self.maxUsagesReached = self.selected.length >= self.maxusages; From c07d59f6e28ec9554406c43e60b71acd1ee49e3b Mon Sep 17 00:00:00 2001 From: hlomzik Date: Wed, 4 Oct 2023 00:38:00 +0100 Subject: [PATCH 13/16] Remove excess @todo --- src/components/NewTaxonomy/NewTaxonomy.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/NewTaxonomy/NewTaxonomy.tsx b/src/components/NewTaxonomy/NewTaxonomy.tsx index b4233e5b76..abe3d1e0d2 100644 --- a/src/components/NewTaxonomy/NewTaxonomy.tsx +++ b/src/components/NewTaxonomy/NewTaxonomy.tsx @@ -52,7 +52,6 @@ type TaxonomyProps = { const convert = (items: TaxonomyItem[], options: TaxonomyOptions): AntTaxonomyItem[] => { const enrich = (item: TaxonomyItem) => { - // @todo marginLeft: -4 is good to align labels, but cuts them in selected list const color = (item: TaxonomyItem) => ( // no BEM here to make it more lightweight // global classname to allow to change it in Style tag From 695320336bd12d0ccbf2c796a2c8984fba34a032 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 5 Oct 2023 14:30:07 +0100 Subject: [PATCH 14/16] Reorder imports and remove unused one --- src/components/Taxonomy/Taxonomy.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Taxonomy/Taxonomy.tsx b/src/components/Taxonomy/Taxonomy.tsx index 348b57747e..90f7f26503 100644 --- a/src/components/Taxonomy/Taxonomy.tsx +++ b/src/components/Taxonomy/Taxonomy.tsx @@ -9,15 +9,15 @@ import React, { } from 'react'; import { Dropdown, Menu } from 'antd'; +import { LsChevron } from '../../assets/icons'; +import { Tooltip } from '../../common/Tooltip/Tooltip'; import { useToggle } from '../../hooks/useToggle'; +import { CNTagName } from '../../utils/bem'; +import { FF_DEV_4075, FF_PROD_309, isFF } from '../../utils/feature-flags'; import { isArraysEqual } from '../../utils/utilities'; -import { LsChevron } from '../../assets/icons'; import TreeStructure from '../TreeStructure/TreeStructure'; import styles from './Taxonomy.module.scss'; -import { FF_DEV_4075, FF_PROD_309, FF_TAXONOMY_LABELING, isFF } from '../../utils/feature-flags'; -import { Tooltip } from '../../common/Tooltip/Tooltip'; -import { CNTagName } from '../../utils/bem'; type TaxonomyPath = string[]; type onAddLabelCallback = (path: string[]) => any; From 71253e85d0414e49b45f9bff0423a2a02eb6ac02 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Mon, 9 Oct 2023 14:51:08 +0100 Subject: [PATCH 15/16] Simplify Visibility checks That also should improve performance for huge Taxonomies --- src/mixins/SelectedChoiceMixin.js | 31 ++++--------------------------- src/mixins/Visibility.js | 2 +- 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/mixins/SelectedChoiceMixin.js b/src/mixins/SelectedChoiceMixin.js index 99f5fb84b7..75d3a9f46f 100644 --- a/src/mixins/SelectedChoiceMixin.js +++ b/src/mixins/SelectedChoiceMixin.js @@ -21,35 +21,12 @@ const SelectedChoiceMixin = types return isDefined(choice1) && isDefined(choice2) && choice1 === choice2; }, - hasChoiceSelection(choiceValue, selectedValues = []) { + hasChoiceSelection(choiceValue) { if (choiceValue?.length) { - // @todo Revisit this and make it more consistent, and refactor this - // behaviour out of the SelectedModel mixin and use a singular approach. - // This is the original behaviour of other SelectedModel mixin usages - // as they are using alias lookups for choices. For now we will keep it as is since it works for all the - // other cases currently. - if (self.findLabel) { - return choiceValue - .map(v => self.findLabel(v)) - .some(c => c && c.sel); - } + // grab the string value; for taxonomy, it's the last value in the array + const selectedValues = self.selectedValues().map(s => Array.isArray(s) ? s.at(-1) : s); - // Check the selected values of the choices for existence of the choiceValue(s) - if (selectedValues.length) { - const includesValue = v => { - if (self.findItemByValueOrAlias) { - const item = self.findItemByValueOrAlias(v); - - v = item?.alias || item?.value || v; - } - - return selectedValues.map(s => Array.isArray(s) ? s.at(-1) : s).includes(v); - }; - - return choiceValue.some(includesValue); - } - - return false; + return choiceValue.some(value => selectedValues.includes(value)); } return self.isSelected; diff --git a/src/mixins/Visibility.js b/src/mixins/Visibility.js index 450079a184..7d74e707f7 100644 --- a/src/mixins/Visibility.js +++ b/src/mixins/Visibility.js @@ -50,7 +50,7 @@ const VisibilityMixin = types if (!tag?.hasChoiceSelection && !choiceValue?.length) return false; - return tag.hasChoiceSelection(choiceValue?.split(','), tag.selectedValues()); + return tag.hasChoiceSelection(choiceValue?.split(',')); }, 'no-region-selected': () => !self.annotation.highlightedNode, From 65e758842ea7ef35fed539f2f3edb8152503d89c Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 10 Oct 2023 13:43:50 +0100 Subject: [PATCH 16/16] Currently revert old code and wrap new one with FF Final decision about using stored values everywhere wil s=come soon. For now stable bahaviour is always active with FF off. FF on has regress for now. --- src/mixins/SelectedChoiceMixin.js | 38 +++++++++++++++++- src/mixins/Visibility.js | 2 +- src/tags/control/Taxonomy/Taxonomy.js | 58 +++++++++++++-------------- 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/src/mixins/SelectedChoiceMixin.js b/src/mixins/SelectedChoiceMixin.js index 75d3a9f46f..5ef40a7b95 100644 --- a/src/mixins/SelectedChoiceMixin.js +++ b/src/mixins/SelectedChoiceMixin.js @@ -21,7 +21,10 @@ const SelectedChoiceMixin = types return isDefined(choice1) && isDefined(choice2) && choice1 === choice2; }, - hasChoiceSelection(choiceValue) { + // @todo it's better to only take final values into account + // @todo (meaning alias only, not alias + value when alias is present) + // @todo so this should be the final and simpliest method + hasChoiceSelectionSimple(choiceValue) { if (choiceValue?.length) { // grab the string value; for taxonomy, it's the last value in the array const selectedValues = self.selectedValues().map(s => Array.isArray(s) ? s.at(-1) : s); @@ -29,6 +32,39 @@ const SelectedChoiceMixin = types return choiceValue.some(value => selectedValues.includes(value)); } + return self.isSelected; + }, + hasChoiceSelection(choiceValue, selectedValues = []) { + if (choiceValue?.length) { + // @todo Revisit this and make it more consistent, and refactor this + // behaviour out of the SelectedModel mixin and use a singular approach. + // This is the original behaviour of other SelectedModel mixin usages + // as they are using alias lookups for choices. For now we will keep it as is since it works for all the + // other cases currently. + if (self.findLabel) { + return choiceValue + .map(v => self.findLabel(v)) + .some(c => c && c.sel); + } + + // Check the selected values of the choices for existence of the choiceValue(s) + if (selectedValues.length) { + const includesValue = v => { + if (self.findItemByValueOrAlias) { + const item = self.findItemByValueOrAlias(v); + + v = item?.alias || item?.value || v; + } + + return selectedValues.map(s => Array.isArray(s) ? s.at(-1) : s).includes(v); + }; + + return choiceValue.some(includesValue); + } + + return false; + } + return self.isSelected; }, })); diff --git a/src/mixins/Visibility.js b/src/mixins/Visibility.js index 7d74e707f7..450079a184 100644 --- a/src/mixins/Visibility.js +++ b/src/mixins/Visibility.js @@ -50,7 +50,7 @@ const VisibilityMixin = types if (!tag?.hasChoiceSelection && !choiceValue?.length) return false; - return tag.hasChoiceSelection(choiceValue?.split(',')); + return tag.hasChoiceSelection(choiceValue?.split(','), tag.selectedValues()); }, 'no-region-selected': () => !self.annotation.highlightedNode, diff --git a/src/tags/control/Taxonomy/Taxonomy.js b/src/tags/control/Taxonomy/Taxonomy.js index 351655f05b..8e6542183a 100644 --- a/src/tags/control/Taxonomy/Taxonomy.js +++ b/src/tags/control/Taxonomy/Taxonomy.js @@ -168,6 +168,35 @@ const TaxonomyLabelingResult = types self.result.area.setValue(self); } }, + + /** + * @param {string[]} path saved value from Taxonomy + * @returns quazi-label object to act as Label in most places + */ + findLabel(path) { + let title = ''; + let items = self.items; + let item; + + for (const value of path) { + item = items?.find(item => item.path.at(-1) === value); + + if (!item) return null; + + items = item.children; + title = self.showfullpath && title ? title + self.pathseparator + item.label : item.label; + } + + const label = { value: title, id: path.join(self.pathseparator) }; + + if (item.color) { + // to conform the current format of our Result#style (and it requires parent) + label.background = item.color; + label.parent = {}; + } + + return label; + }, }; }); @@ -289,35 +318,6 @@ const Model = types return findItem(self.items); }, - - /** - * @param {string[]} path saved value from Taxonomy - * @returns quazi-label object to act as Label in most places - */ - findLabel(path) { - let title = ''; - let items = self.items; - let item; - - for (const value of path) { - item = items?.find(item => item.path.at(-1) === value); - - if (!item) return null; - - items = item.children; - title = self.showfullpath && title ? title + self.pathseparator + item.label : item.label; - } - - const label = { value: title, id: path.join(self.pathseparator) }; - - if (item.color) { - // to conform the current format of our Result#style (and it requires parent) - label.background = item.color; - label.parent = {}; - } - - return label; - }, })) .actions(self => ({ afterAttach() {