fix: code review findings

lib/components/ActiveTextTruncate/ActiveTextTruncate.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { computed, ref, watch } from 'vue'
+import { computed, onUnmounted, ref, watch } from 'vue'
 import { useResizeObserver } from '@/composables/useResizeObserver'
 import { RequestAnimationFrameWrapper } from '@/utils/animation'
 
@@ -150,6 +150,24 @@ function resetTextLivePosition(): void {
    */
   emit('cancel')
 }
+
+onUnmounted(() => {
+  // Stop the Request Animation Frame loop so it doesn't reschedule forever
+  textLivePositionRequestAnimationFrame.value.stop()
+  // Remove the transition listeners added on the text element
+  textElement.value?.removeEventListener(
+    'transitionstart',
+    startTrackingTextLivePosition
+  )
+  textElement.value?.removeEventListener(
+    'transitionend',
+    endTrackingTextLivePosition
+  )
+  textElement.value?.removeEventListener(
+    'transitioncancel',
+    resetTextLivePosition
+  )
+})
 </script>
 
 <template>

lib/components/Form/FormAdvancedLink/FormAdvancedLink.vue

@@ -5,7 +5,7 @@ import { uniqueId } from 'lodash'
 import { ButtonVariant } from 'bootstrap-vue-next'
 
 import AdvancedLinkFormTab from './FormAdvancedLinkTab.vue'
-import { Tab } from '@/components/Form/FormAdvancedLink/FormAdvancedLinkTab.vue'
+import { AdvancedLinkTab } from '@/enums'
 
 /**
  * A form with tabs to offer several copy formats to users.
@@ -31,7 +31,7 @@ export interface FormAdvancedLinkProps {
   /**
    * The forms to display
    */
-  forms?: Tab[]
+  forms?: AdvancedLinkTab[]
   /**
    * Activate the card integration for the tabs
    */
@@ -65,7 +65,7 @@ export interface FormAdvancedLinkProps {
 const props = withDefaults(defineProps<FormAdvancedLinkProps>(), {
   link: undefined,
   title: 'Link',
-  forms: () => ['raw', 'markdown', 'rich', 'html'],
+  forms: () => [AdvancedLinkTab.raw, AdvancedLinkTab.markdown, AdvancedLinkTab.rich, AdvancedLinkTab.html],
   card: false,
   pills: false,
   small: false,
@@ -86,7 +86,7 @@ const formClasses = computed(() => {
 })
 
 // default tabs order
-const defaultTabs: Tab[] = ['raw', 'rich', 'markdown', 'html']
+const defaultTabs: AdvancedLinkTab[] = [AdvancedLinkTab.raw, AdvancedLinkTab.rich, AdvancedLinkTab.markdown, AdvancedLinkTab.html]
 
 const tabs = computed(() => {
   return defaultTabs

lib/components/Form/FormAdvancedLink/FormAdvancedLinkTab.vue

@@ -2,7 +2,6 @@
 import { BaseButtonVariant } from 'bootstrap-vue-next'
 import HapticCopy from '@/components/HapticCopy/HapticCopy.vue'
 import { computed, nextTick, type ShallowRef, useTemplateRef } from 'vue'
-import { uniqueId } from 'lodash'
 
 import { AdvancedLinkTab } from '@/enums'
 
@@ -28,8 +27,7 @@ interface HTMLElementSupportingCreateRange extends HTMLElement {
 const props = withDefaults(defineProps<AdvancedLinkFormTabProps>(), {
   type: AdvancedLinkTab.raw,
   compact: false,
-  variant: 'primary',
-  id: uniqueId('advanced-link-form-tab')
+  variant: 'primary'
 })
 
 const size = computed(() => (props.compact ? 'sm' : 'md'))

lib/components/Form/FormControl/FormControlSelectableDropdown.vue

@@ -5,6 +5,7 @@ import findIndex from 'lodash/findIndex'
 import filter from 'lodash/filter'
 import identity from 'lodash/identity'
 import isEqual from 'lodash/isEqual'
+import omit from 'lodash/omit'
 // @ts-expect-error no typings available
 import { RecycleScroller } from 'vue-virtual-scroller'
 
@@ -135,9 +136,20 @@ const keyField = computed(() => {
 })
 
 const firstActiveItemIndex = computed(() => {
-  return activeItems.value.length
-    ? items_.value.indexOf(activeItems.value[0])
-    : -1
+  if (!activeItems.value.length) {
+    return -1
+  }
+  const activeItem = activeItems.value[0]
+  if (typeof activeItem === 'string') {
+    return items_.value.indexOf(activeItem)
+  }
+  // activeItems may hold either a raw modelValue object (no recycle_scroller_id)
+  // or an items_ entry from a click/range-select (which carries it), so strip
+  // the injected key from both sides before comparing.
+  const target = omit(activeItem, 'recycle_scroller_id')
+  return items_.value.findIndex(it =>
+    isEqual(omit(it, 'recycle_scroller_id'), target)
+  )
 })
 
 // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type

lib/components/Form/FormDonate.vue

@@ -120,11 +120,8 @@ watch(installmentPeriod, () => {
 
 watch(
   () => amount.value,
-  (v: number) => {
+  () => {
     level.value = changeThe.value
-
-    // Set manual amount
-    return (amount.value = v)
   }
 )
 </script>

lib/components/Form/FormEmbed.vue

@@ -65,8 +65,8 @@ const currentUrl = computed(() => {
   return props.url || window?.location?.href
 })
 
-function iframeCodeFor(_url = currentUrl, width: string, height: string) {
-  const src = IframeResizer.deletePymParams(props.url)
+function iframeCodeFor(url = currentUrl, width: string, height: string) {
+  const src = IframeResizer.deletePymParams(url.value)
   return `<iframe width="${width}" height="${height}" src="${src}" frameborder="0" allowfullscreen></iframe>`
 }
 

lib/components/HapticCopy/HapticCopy.vue

@@ -147,6 +147,7 @@ async function openTooltip(msg = 'haptic-copy.tooltip.succeed') {
 }
 
 async function closeTooltip() {
+  clearTimeout(tooltipTimeout.value)
   showClipboardTooltip.value = false
   tooltipTimeout.value = undefined
   emit('hideClipboardTooltip')

lib/components/Pagination/PaginationTiny.vue

@@ -155,6 +155,7 @@
 </template>
 
 <script lang="ts" setup>
+import clamp from 'lodash/clamp'
 import { BButton, BFormInput } from 'bootstrap-vue-next'
 import { computed, ref, watch, type Component } from 'vue'
 import { directive as vInputAutowidth } from 'vue-input-autowidth'
@@ -376,14 +377,14 @@ const title = computed(() => {
 function applyPageForm(): void {
   const { value } = currentPageInput
   if (!isNaN(value as number)) {
-    modelValue.value = +value
+    modelValue.value = clamp(Math.floor(+value), 1, numberOfPages.value)
   }
 }
 
 function applyRowForm(): void {
   const { value } = currentRowInput
   if (!isNaN(value as number)) {
-    currentPageInput.value = Math.floor(+value / +props.perPage) + 1
+    currentPageInput.value = clamp(Math.floor(+value / +props.perPage) + 1, 1, numberOfPages.value)
     currentRowInput.value = +props.perPage * (+currentPageInput.value - 1) + 1
     modelValue.value = +currentPageInput.value
   }

lib/components/ResponsiveIframe/ResponsiveIframe.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { ref, onMounted, getCurrentInstance } from 'vue'
+import { ref, onMounted, onUnmounted, getCurrentInstance } from 'vue'
 import type { Parent } from 'pym.js'
 
 import { injectAssets } from '@/utils/assets'
@@ -41,6 +41,10 @@ onMounted(async (): Promise<void> => {
     props.options ?? {}
   )
 })
+
+onUnmounted(() => {
+  pymParent.value?.remove?.()
+})
 </script>
 
 <template>

lib/components/SharingOptions/SharingOptions.vue

@@ -91,7 +91,7 @@ const metaValues = computed((): MetaValuesMap => {
     title: defaultValueFor('sharing-options.title'),
     description: defaultValueFor(
       'sharing-options.description',
-      'meta[name="description]'
+      'meta[name="description"]'
     ),
     facebook_title: defaultValueFor(
       'sharing-options.facebook_title',
@@ -109,7 +109,7 @@ const metaValues = computed((): MetaValuesMap => {
 })
 
 function valuesFor(network: string): Record<string, string> {
-  const values = Object.assign(metaValues.value, props.values)
+  const values = Object.assign({}, metaValues.value, props.values)
   return reduce(
     props.valuesKeys,
     (res: Record<string, string>, key) => {

lib/components/SharingOptions/SharingOptionsLink.vue

@@ -72,7 +72,7 @@ export const networks: SharingPlatforms = {
 import querystring from 'querystring-es3'
 import reduce from 'lodash/reduce'
 import get from 'lodash/get'
-import { computed, reactive } from 'vue'
+import { computed, onUnmounted, reactive } from 'vue'
 
 import AppIcon from '@/components/App/AppIcon.vue'
 
@@ -229,6 +229,13 @@ function handleClick(event: Event): void {
   }
 }
 
+// Make sure the polling interval (and any open popup) is torn down when the
+// component unmounts, otherwise the setInterval started in openPopup() keeps
+// firing against a destroyed instance.
+onUnmounted(() => {
+  cleanExistingPopupInstance()
+})
+
 defineExpose({
   base,
   args,

lib/components/SlideUpDown/SlideUpDown.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { ref, computed, watch, onMounted, nextTick } from 'vue'
+import { ref, computed, watch, onMounted, onUnmounted, nextTick } from 'vue'
 import type { CSSProperties } from 'vue'
 
 import { SlideUpDownState } from '@/enums'
@@ -89,6 +89,10 @@ async function triggerSlide(): Promise<void> {
   state.value = SlideUpDownState.active
 }
 
+function onTransitionEnd(e: TransitionEvent) {
+  cleanLayout(e)
+}
+
 function cleanLayout(e: Event | null) {
   // This method can be triggered by animated child elements in
   // which case, we should do anything
@@ -111,9 +115,11 @@ onMounted(async () => {
   await deferredNextTick()
   mounted.value = true
   await cleanLayout(null)
-  container.value?.addEventListener('transitionend', (e: TransitionEvent) =>
-    cleanLayout(e)
-  )
+  container.value?.addEventListener('transitionend', onTransitionEnd)
+})
+
+onUnmounted(() => {
+  container.value?.removeEventListener('transitionend', onTransitionEnd)
 })
 
 defineExpose({

lib/composables/useChart.ts

@@ -155,7 +155,7 @@ export function useChart(
 
     await afterLoaded?.()
     isLoaded.value = true
-    emit('loaded')
+    emit('loaded', loadedData.value)
 
     if (onResized) {
       onResized()
@@ -217,7 +217,10 @@ export function useChart(
   })
 
   const dataHasHighlights = computed(() => {
-    const data = toValue(dataRef)
+    // Prefer loadedData (so URL-fetched data is inspected), but fall back to the
+    // raw prop so a synchronously-passed array reports highlights on first paint
+    // rather than flickering once the load watcher resolves.
+    const data = loadedData.value ?? toValue(dataRef)
     if (Array.isArray(data)) {
       return some(data, highlighted)
     }

lib/composables/useQueryObserver.ts

@@ -1,4 +1,4 @@
-import { computed, reactive, toRef, watch } from 'vue'
+import { computed, onScopeDispose, reactive, toRef, watch } from 'vue'
 import { first, get } from 'lodash'
 type ElementMap = Record<string, HTMLElement[]>
 
@@ -56,6 +56,12 @@ export function useQueryObserver(root = window.document, once = false) {
     return computed(() => get(elements, selector, []))
   }
 
+  onScopeDispose(() => {
+    for (const selector in observers) {
+      observers[selector].disconnect()
+    }
+  })
+
   return {
     querySelector,
     querySelectorAll

lib/composables/useResizeObserver.ts

@@ -1,7 +1,12 @@
 import { ref, reactive, onMounted, onBeforeUnmount } from 'vue'
 import { Ref } from 'vue'
+import debounce from 'lodash/debounce'
 import ResizeObserver from 'resize-observer-polyfill'
 
+// Coalesce resize bursts (e.g. a window/pane drag fires dozens per second) so
+// consumers only redraw once the dimensions settle, rather than every frame.
+const RESIZE_DEBOUNCE_MS = 100
+
 export function useResizeObserver(resizableRef?: Ref) {
   const resizeRef: Ref<HTMLElement> = resizableRef ?? ref()
   const resizeState = reactive({
@@ -10,25 +15,32 @@ export function useResizeObserver(resizableRef?: Ref) {
     narrowWidth: false
   })
 
-  const observer = new ResizeObserver((entries) => {
+  const onResize = debounce((entries: ResizeObserverEntry[]) => {
     entries.forEach((entry) => {
       resizeState.dimensions = entry.contentRect
       resizeState.offsetWidth = (entry.target as HTMLElement).offsetWidth
       resizeState.narrowWidth = (resizeState.offsetWidth ?? 540) < 540
     })
-  })
+  }, RESIZE_DEBOUNCE_MS)
+
+  const observer = new ResizeObserver(onResize)
 
   onMounted(async () => {
-    // set initial dimensions right before observing: Element.getBoundingClientRect()
-    resizeState.dimensions = resizeRef.value.getBoundingClientRect()
-    resizeState.offsetWidth = resizeRef.value.offsetWidth
-    resizeState.narrowWidth = false // TODO CD: old default is false but maybe this would work (resizeState.offsetWidth ?? 540) < 540 but would be often true
+    if (resizeRef.value) {
+      // set initial dimensions right before observing: Element.getBoundingClientRect()
+      resizeState.dimensions = resizeRef.value.getBoundingClientRect()
+      resizeState.offsetWidth = resizeRef.value.offsetWidth
+      resizeState.narrowWidth = false // TODO CD: old default is false but maybe this would work (resizeState.offsetWidth ?? 540) < 540 but would be often true
 
-    observer.observe(resizeRef.value)
+      observer.observe(resizeRef.value)
+    }
   })
 
   onBeforeUnmount(() => {
-    observer.unobserve(resizeRef.value)
+    // Drop any pending trailing call so it can't fire after teardown, then stop
+    // observing every element (disconnect covers unobserve).
+    onResize.cancel()
+    observer.disconnect()
   })
 
   return { resizeState, resizeRef }