fix: remove bug in addWitness() when target is the only child element…

…, add checks for null/array length, make cleaner the functions in annotations.js related to variants

* fix: remove the witness only when there is 'witnesses' html element in text panel

* fix: restructure addWitness() to avoid the bug when target is the only child of its parent

* fix: add checks on witnesses array to avoid bugs

* fix: remove bug when target is null

* fix: show the variants top bar only when there are filteredAnnotations for the variants tab

* fix: do not emit a click on text, when it is not an annotation target

* refactor: improve the code which updates the parentHtml el when we add a witness

* refactor: add early return statement in allocateWitnessColorInVariantItem

* chore: remove the if condition on showing the witnesses filter

* refactor: clean addWitness() function

* refactor: check the existence of witnesses Html in removeWitness()

* refactor: add early return in getWitnessesHtmlEl()

* refactor: restructure better addWitness and remove 'html' from variable names

* refactor: remove unused functions and added checks on (null or zero-length) arrays

* docs: reason why witnesses wrapper is on the same child level as target in addWitnesses()

* refactor: make createWitnessEl function consistent

* refactor: make several functions in annotations.js more consistent

---------

Co-authored-by: malkja <[email protected]>

src/components/annotations/variants/VariantsTopBar.vue

@@ -18,7 +18,9 @@ const showWitnessesDropdown = ref(false);
 const variantsDetailsDialogOpen = ref(false);
 
 watch(witnesses, (value) => {
-  value.forEach(witness => activeWitnessesIds[witness.idno] = true);
+  if(value?.length > 0) {
+   value.forEach(witness => activeWitnessesIds[witness.idno] = true);
+  }
 },
   { immediate: true }
 )

src/components/annotations/variants/VariantsView.vue

@@ -61,11 +61,14 @@ onBeforeUnmount(() => unsubscribe())
 
 function allocateWitnessColorInVariantItem() {
   const colors = {}
+  if(annotationStore.witnesses?.length === 0) return;
+
   annotationStore.witnesses.forEach((witness, i) => {
-    colors[witness.idno] = getItemColorBasedOnIndex(i)
-  })
+  colors[witness.idno] = getItemColorBasedOnIndex(i)
   annotationStore.setVariantItemsColors(colors)
-}
+  })
+  }
+
 </script>
 
 <template>

src/components/panels/Panel.vue

@@ -406,7 +406,7 @@ export default {
     function onViewChange(index) {
       activeTabIndex.value = index;
       if (index !==2) {
-        AnnotationUtils.removeChipsFromOtherViews()
+        AnnotationUtils.removeWitnessesWrappers()
       }
 
       emit('active-view', activeTabIndex.value);

src/stores/annotations.ts

@@ -15,7 +15,7 @@ export const useAnnotationsStore = defineStore('annotations', () => {
   const activeAnnotations = ref({})
   const variantItemsColors = ref({})
   const annotations = ref<Annotation[]>(null)
-  const witnesses = ref<Witness[]>(null)
+  const witnesses = ref<Witness[]>([])
   const filteredAnnotations = ref<Annotation[]>([])
   const isLoading = ref<boolean>(false);
   const isSingleSelectMode = ref<boolean>(false)
@@ -81,7 +81,6 @@ export const useAnnotationsStore = defineStore('annotations', () => {
   const selectFilteredAnnotations = (types: AnnotationType[]): void => {
     const configStore = useConfigStore()
     const activeContentType: string = configStore.activeContentType
-
     if (annotations.value !== null) {
       filteredAnnotations.value = types.length === 0 ? annotations.value : annotations.value.filter(
         (annotation) => {
@@ -94,6 +93,7 @@ export const useAnnotationsStore = defineStore('annotations', () => {
 
           // If the display is not dependent on displayWhen then we check if annotation's target exists in the content
           const selector: string = AnnotationUtils.generateTargetSelector(annotation);
+
           if (selector) {
             const el: HTMLElement = document.querySelector(selector);
             if (el) {
@@ -136,17 +136,20 @@ export const useAnnotationsStore = defineStore('annotations', () => {
 
     // When filtering by witness it can happen that a target is used for some other active annotation item,
     // In that case, we want to keep the level of highlighting it had and
-
+  
     filteredAnnotations.value
       .filter(annotation => !activeIds.includes(annotation.id))
       .forEach(annotation => {
         const selector = AnnotationUtils.generateTargetSelector(annotation);
         if (!selector) return;
         const selectorIsActive = activeIds.filter(id => selector === AnnotationUtils.generateTargetSelector(activeAnnotations.value[id])).length > 0;
-
-        if (!selectorIsActive && AnnotationUtils.getCurrentLevel(document.querySelector(selector)) < 0) {
+        const target = document.querySelector(selector)
+        if(!target) return;
+
+        if (!selectorIsActive && AnnotationUtils.getCurrentLevel(target) < 0) {
           AnnotationUtils.highlightTargets(selector, {level: 0});
         }
+
       })
   }
 
@@ -314,11 +317,10 @@ export const useAnnotationsStore = defineStore('annotations', () => {
       if (!target.dataset.annotation) {
         target = getNearestParentAnnotation(target);
       }
-
+      
       if (!target) {
         return;
       }
-
       TextEventBus.emit('click', { target })
     });
   };

src/utils/annotations.js

@@ -240,114 +240,108 @@ export function removeIcon(annotation) {
   }
 }
 
-export function addWitness(targetHtmlEl, witness, color) {
-  const parentEl = targetHtmlEl.parentElement
-  const indexOfTarget = [].slice.call(parentEl.children).indexOf(targetHtmlEl)
-  const witHtml = createCurrWitHtml(witness, color)
-
-  if(!parentEl.children[indexOfTarget-1].classList.contains("witnesses")) {
-    // if the previous element in DOM does not contains 'witnesses chips' then create the 'parent' span of the 'witnesses chips'
-    // Create another function - like create 'witnesses' Html element
-    const witnessesHtmlEl = document.createElement("span");
-    witnessesHtmlEl.classList.add('witnesses')
-
-    witnessesHtmlEl.prepend(witHtml)
-    parentEl.insertBefore(witnessesHtmlEl, targetHtmlEl)
-  }
-  else {
-    // get the target element and get the previous element - which we know is the 'witnesses' span list
-    // get the witnessesHtml element and append the witHtml element
-    let witnessesHtmlEl = parentEl.children[indexOfTarget-1]
-    witnessesHtmlEl.appendChild(witHtml)
+export function addWitness(target, witness, color) {
+  // we create a witnesses wrapper on the same child level as the target due to styling reasons (i.e the witnesses chips should not be underlined and get the highlight color of the target)
+  // therefore we need target index in order to find the witnesses wrapper element
+  let parentEl = target.parentElement
+  const targetIndex = [].slice.call(parentEl.children).indexOf(target)
+  if (targetIndex < 0) return;
+
+  const witnessEl = createWitnessEl(witness, color)
+  const isWrapper = parentEl.children[targetIndex-1].classList.contains("witnesses")
+
+  if (isWrapper) {
+    let wrapper = parentEl.children[targetIndex-1]
+    wrapper.appendChild(witnessEl)
+  } else {
+      // witnesses wrapper which holds the witnesses 'chips' is not yet created -> we add the first witness 
+      // create witnesses Html 
+      const wrapper = createWitnessesWrapper()
+      wrapper.appendChild(witnessEl)
+      parentEl.insertBefore(wrapper, target)
   }
 }
 
-function createCurrWitHtml(witness, witnessColor) {
-  // create an html element of the selected witness
-  const witHtml = document.createElement("span");
-  witHtml.innerHTML = witness
-  witHtml.classList.add('t-rounded-3xl', 't-box-border', 't-h-8', 't-py-0.5', 't-px-1.5', 't-text-xs', 't-font-semibold', 't-ml-[3px]')
-  witHtml.style.background = colors[witnessColor]['100']
-  witHtml.style.color = colors[witnessColor]['600']
 
+function createWitnessesWrapper() {
+  const el = document.createElement("span");
+  el.classList.add('witnesses')
 
-  return witHtml
+  return el
 }
 
-export function removeChipsFromOtherViews() {
+function createWitnessEl(witness, witnessColor) {
+  // create an html element of one witness
+  const el = document.createElement("span");
+  el.innerHTML = witness
+  el.classList.add('t-rounded-3xl', 't-box-border', 't-h-8', 't-py-0.5', 't-px-1.5', 't-text-xs', 't-font-semibold', 't-ml-[3px]')
+  el.style.background = colors[witnessColor]['100']
+  el.style.color = colors[witnessColor]['600']
+
+  return el
+}
+
+export function removeWitnessesWrappers() {
+  // remove witnesses in text Panel - it is used when switch off the variants tab
   const textPanelEl = document.querySelector('#text-content')
-  const witnessesHtmlElements = textPanelEl.getElementsByClassName('witnesses')
-  if( witnessesHtmlElements ) {
-    Array.from(witnessesHtmlElements).forEach((witnesses) => {
-      witnesses.remove()
-    })
-  }
+  const wrappers = textPanelEl.getElementsByClassName('witnesses')
+  if (!wrappers) return;
+  if(Array.from(wrappers).length === 0) return;
+
+  // each target has its witnesses wrapper; for every target we remove its witnesses wrapper
+  Array.from(wrappers).forEach((wrapper) => {
+    wrapper.remove()
+  })
 }
 
 export function removeWitness(selector, witness) {
   // find the witnesses span which contains each 'witness' span child element
   // find this witness inside the 'witnesses' html span and remove it
-  const witnessesHtmlEl = getWitnessesHtmlEl(selector)
-  const witHtml = Array.from(witnessesHtmlEl.children).filter(item => item.innerHTML === witness)
-  if (witHtml.length > 0) witHtml[0].remove()
+  const textPanel = document.querySelector('#text-content')
+  if (!textPanel.querySelector('.witnesses')) return;
+
+  const wrapper = getWitnessesWrapper(selector)
+  if (!wrapper) return;
+  if (Array.from(wrapper.children).length === 0) return;
+
+  const witnessEl = Array.from(wrapper.children).filter(item => item.innerHTML === witness)  
+  // witEl: refers to the current Witness chip that we will remove
+  if (witnessEl.length > 0) witnessEl[0].remove()
 }
 
-export function getWitnessesHtmlEl(selector) {
+export function getWitnessesWrapper(selector) {
   // selector represents the target text of a certain variant item
   // we aim to get the html element which contains the 'witnesses chips' related to the target.
   // this html element which contains the 'witnesses chips' is located before the target element
-  const targetHtmlEl = document.querySelector(selector)
-  const parentEl = targetHtmlEl.parentElement
-  const indexOfTarget = [].slice.call(parentEl.children).indexOf(targetHtmlEl)
-  const witnessesHtmlEl = parentEl.children[indexOfTarget-1]
+  const targetEl = document.querySelector(selector)
+  if (!targetEl) return null
 
-  return witnessesHtmlEl
-}
-
-export function getWitnessesList(witnessesHtml) {
-  // returns the list of witnesses(<string>) which are already selected
-  let witnessesList= []
-  Array.from(witnessesHtml.children).forEach((witnessHtml) => {
-    witnessesList.push(witnessHtml.innerHTML)
-  })
-  return witnessesList
-}
+  const parentEl = targetEl.parentElement
+  const targetIndex = [].slice.call(parentEl.children).indexOf(targetEl)
+  if(targetIndex < 1) return null
 
-export function unselectVariantItems(variantItemsSelection) {
-  let newVariantItemsSelection = {}
-  Object.keys(variantItemsSelection).forEach((wit) => {
-    newVariantItemsSelection[wit] = false
-  })
-  return newVariantItemsSelection
+  // witnesses el is placed before the target
+  return parentEl.children[targetIndex-1] 
 }
 
-export function addWitnessesChipsWhenSelectText(variantItemsSelection, selector, variantItemsColors) {
-  // variantItemsSelection: JSON object of 'witness name': 'true'
-  // this function aims to add all witnesses on the highlighted text when we click on the text
+export function getWitnessesList(wrapper) {
+  // wrapper: witnesses html wrapper which belongs to a target
+  // returns the list of witnesses(<string>) which belong to a witnesses wrapper
+  let list = []
+  if (!wrapper) return [];
+  if(Array.from(wrapper.children).length === 0) return [];
 
-  Object.keys(variantItemsSelection).forEach((witness) => {
-    addWitness(selector, witness, variantItemsColors)
+  Array.from(wrapper.children).forEach((witness) => {
+    list.push(witness.innerHTML)
   })
+  return list
 }
 
-export function removeWitnessesChipsWhenDeselectText(witnessesList, selector) {
-  witnessesList.forEach((witness) => {
-    removeWitness(selector, witness)
-  })
-}
 
 export function isVariant(annotation) {
-  return annotation.body['x-content-type'] === 'Variant';
+  return annotation?.body['x-content-type'] === 'Variant';
 }
 
-export function initVariantItemsSelection(annotation, value) {
-  // initialize with the boolean of 'value' variable
-  let variantItemsSelection = {}
-  annotation.body.value.forEach((variantItem) => {
-    variantItemsSelection[variantItem.witness] = value
-  } )
-  return variantItemsSelection
-}
 
 export function getAnnotationListElement(id, container) {
   return [...container.querySelectorAll('.q-item')].find((annotationItem) => {