Fix Sparkline empty render, stale tween attributes and one-value line (#3227)

core/js/src/sparkline.ts

@@ -262,7 +262,9 @@ class Sparkline extends BaseComponent {
     this._element.querySelector(`.${CLASS_NAME_LABEL}`)?.remove()
 
     if (this._config.values.length === 0) {
-      this._element.innerHTML = ''
+      cancelAnimationFrame(this._frame)
+      previous?.remove()
+      EventHandler.trigger(this._element, EVENT_RENDERED)
       return
     }
 
@@ -308,7 +310,8 @@ class Sparkline extends BaseComponent {
 
   dispose(): void {
     cancelAnimationFrame(this._frame)
-    this._element.innerHTML = ''
+    this._element.querySelector('svg')?.remove()
+    this._element.querySelector(`.${CLASS_NAME_LABEL}`)?.remove()
     super.dispose()
   }
 
@@ -365,6 +368,12 @@ class Sparkline extends BaseComponent {
     const pairs = Array.from(to.children).map((target, i) => {
       const source = from.children[i]
       const tweens: [string, string, string][] = []
+      for (const { name } of Array.from(source.attributes)) {
+        if (!target.hasAttribute(name)) {
+          source.removeAttribute(name)
+        }
+      }
+
       for (const { name, value } of Array.from(target.attributes)) {
         const start = source.getAttribute(name)
         if (start === null || start === value) {
@@ -431,8 +440,8 @@ class Sparkline extends BaseComponent {
 
     const { min, span } = rangeOf(cfg.values, cfg.min, cfg.max, ...(cfg.threshold === null ? [] : [cfg.threshold]))
     const yOf = (value: number): number => cfg.pad + (1 - (value - min) / span) * (cfg.height - cfg.pad * 2)
-    const ys = cfg.values.map(yOf)
-    const step = ys.length > 1 ? cfg.width / (ys.length - 1) : cfg.width
+    const ys = cfg.values.length === 1 ? [yOf(cfg.values[0]), yOf(cfg.values[0])] : cfg.values.map(yOf)
+    const step = cfg.width / (ys.length - 1)
 
     if (cfg.threshold !== null) {
       this._hairline(svg, yOf(cfg.threshold), 'threshold', true)
@@ -463,7 +472,7 @@ class Sparkline extends BaseComponent {
     if (spotIndex >= 0) {
       const spot = svgEl('circle')
       setAttr(spot, {
-        cx: spotIndex * step,
+        cx: cfg.values.length === 1 ? cfg.width / 2 : spotIndex * step,
         cy: ys[spotIndex],
         r: getCssNumber(this._element, 'spot-size', 2),
         fill: cssVar('spot'),
@@ -560,7 +569,7 @@ class Sparkline extends BaseComponent {
     const cfg = this._config
     const svg = this._createSvg()
 
-    const strokeWidth = getCssNumber(this._element, 'stroke-width', 3)
+    const strokeWidth = getCssNumber(this._element, 'stroke-width', 2)
     const radius = Math.max(0, Math.min(cfg.width, cfg.height) / 2 - strokeWidth / 2)
     const cx = cfg.width / 2
     const cy = cfg.height / 2

core/js/tests/unit/sparkline.spec.ts

@@ -156,9 +156,42 @@ describe('Sparkline', () => {
     it('should render nothing without values', () => {
       fixtureEl.innerHTML = '<span class="sparkline" data-bs-toggle="sparkline"></span>'
 
+      const spy = vi.fn()
+      el().addEventListener('rendered.bs.sparkline', spy)
       new Sparkline(el())
 
       expect(el().innerHTML).toBe('')
+      expect(spy).toHaveBeenCalledTimes(1)
+    })
+
+    it('should keep author content when rendering without values', () => {
+      fixtureEl.innerHTML = '<span class="sparkline"><b class="note">n/a</b></span>'
+
+      const sparkline = new Sparkline(el(), { values: [1, 2], label: 'x' })
+      sparkline.update([])
+
+      expect(el().querySelector('svg')).toBeNull()
+      expect(el().querySelector('.sparkline-label')).toBeNull()
+      expect(el().querySelector('.note')).not.toBeNull()
+    })
+
+    it('should draw a flat line for a single value', () => {
+      fixtureEl.innerHTML = '<span class="sparkline" data-bs-fill="auto" data-bs-spot="last" data-bs-values="5"></span>'
+
+      new Sparkline(el())
+
+      const y = el().querySelector('polyline')!.getAttribute('points')!.split(' ')[0].split(',')[1]
+      expect(el().querySelector('polyline')!.getAttribute('points')).toBe(`0,${y} 80,${y}`)
+      expect(el().querySelector('path')!.getAttribute('d')).toBe(`M 0 ${y} L 80 ${y} L 80 24 L 0 24 Z`)
+      expect(el().querySelector('circle')!.getAttribute('cx')).toBe('40')
+    })
+
+    it('should fall back to a stroke width of 2 without computed styles', () => {
+      const element = document.createElement('span')
+
+      new Sparkline(element, { type: 'circle', values: [50] })
+
+      expect(element.querySelector('circle')!.getAttribute('stroke-width')).toBe('2')
     })
 
     it('should accept values from the config object', () => {
@@ -209,6 +242,35 @@ describe('Sparkline', () => {
       expect(heights).toEqual(['24', '12'])
     })
 
+    it('should drop attributes the new element does not have', async () => {
+      fixtureEl.innerHTML = '<span class="sparkline" data-bs-type="bar" data-bs-threshold="1.5" data-bs-animation="50" data-bs-values="1,2"></span>'
+
+      const sparkline = new Sparkline(el())
+      const line = el().querySelector('line')!
+      expect(line.getAttribute('stroke-dasharray')).toBe('3 2')
+
+      el().removeAttribute('data-bs-threshold')
+      sparkline.update([1, -2])
+      await new Promise((resolve) => setTimeout(resolve, 200))
+
+      expect(el().querySelector('line')).toBe(line)
+      expect(line.hasAttribute('stroke-dasharray')).toBe(false)
+    })
+
+    it('should stop a running tween when the values are cleared', async () => {
+      fixtureEl.innerHTML = '<span class="sparkline"></span>'
+
+      const sparkline = new Sparkline(el(), { type: 'bar', values: [1, 2], animation: 1000 })
+      const rect = el().querySelector('rect')!
+      sparkline.update([2, 1])
+      sparkline.update([])
+      const height = rect.getAttribute('height')
+      await new Promise((resolve) => setTimeout(resolve, 100))
+
+      expect(rect.isConnected).toBe(false)
+      expect(rect.getAttribute('height')).toBe(height)
+    })
+
     it('should replace the svg when the shape changes or animation is off', () => {
       fixtureEl.innerHTML = '<span class="sparkline"></span>'
 
@@ -221,13 +283,13 @@ describe('Sparkline', () => {
   })
 
   describe('dispose', () => {
-    it('should empty the element and drop the instance', () => {
-      fixtureEl.innerHTML = '<span class="sparkline" data-bs-toggle="sparkline" data-bs-values="1,2"></span>'
+    it('should remove the chart and drop the instance', () => {
+      fixtureEl.innerHTML = '<span class="sparkline" data-bs-toggle="sparkline" data-bs-label="auto" data-bs-values="1,2"><b class="note">n/a</b></span>'
 
       const sparkline = new Sparkline(el())
       sparkline.dispose()
 
-      expect(el().innerHTML).toBe('')
+      expect(el().innerHTML).toBe('<b class="note">n/a</b>')
       expect(Sparkline.getInstance(el())).toBeNull()
     })
   })