Skip to content

fix: use varying widths for compare bar chart skeleton loaders #2209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thealxlabs wants to merge 3 commits into
base: main
Choose a base branch
from
+83 −10
Open

fix: use varying widths for compare bar chart skeleton loaders #2209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c805173
fix: improve compare bar chart skeleton loaders with varying widths (…
thealxlabs Mar 22, 2026
b1ddc84
test: add FacetBarChart skeleton varying widths regression test
thealxlabs Mar 22, 2026
06909d3
[autofix.ci] apply automated fixes
autofix-ci[bot] Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions app/components/Compare/FacetBarChart.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,23 +327,31 @@ const config = computed<VueUiHorizontalBarConfig>(() => {
</VueUiHorizontalBar>

<template #fallback>
<div class="flex flex-col gap-2 justify-center items-center mb-2">
<SkeletonInline class="h-4 w-16" />
<SkeletonInline class="h-4 w-28" />
<div class="flex flex-col gap-2 justify-center items-center mb-4" aria-hidden="true">
<SkeletonInline class="h-4 w-20 rounded" />
<SkeletonInline class="h-3 w-36 rounded" />
</div>
<div class="flex flex-col gap-1">
<SkeletonInline class="h-7 w-full" v-for="pkg in packages" :key="pkg" />
<div class="flex flex-col gap-2 px-2" aria-hidden="true">
<div v-for="(pkg, i) in packages" :key="pkg" class="flex items-center gap-3">
<SkeletonInline class="h-3 shrink-0" :style="{ width: `${40 + ((i * 17) % 40)}%` }" />
<SkeletonInline class="h-7 flex-1 rounded" />
</div>
</div>
</template>
</ClientOnly>

<template v-else>
<div class="flex flex-col gap-2 justify-center items-center mb-2">
<SkeletonInline class="h-4 w-16" />
<SkeletonInline class="h-4 w-28" />
<!-- Title skeleton -->
<div class="flex flex-col gap-2 justify-center items-center mb-4" aria-hidden="true">
<SkeletonInline class="h-4 w-20 rounded" />
<SkeletonInline class="h-3 w-36 rounded" />
</div>
<div class="flex flex-col gap-1">
<SkeletonInline class="h-7 w-full" v-for="pkg in packages" :key="pkg" />
<!-- Bar skeletons with varying widths for visual realism -->
<div class="flex flex-col gap-2 px-2" aria-hidden="true">
<div v-for="(pkg, i) in packages" :key="pkg" class="flex items-center gap-3">
<SkeletonInline class="h-3 shrink-0" :style="{ width: `${40 + ((i * 17) % 40)}%` }" />
<SkeletonInline class="h-7 flex-1 rounded" />
</div>
</div>
</template>
</div>
Expand Down
65 changes: 65 additions & 0 deletions test/nuxt/components/compare/FacetBarChart.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { describe, expect, it, vi } from 'vitest'
import { mountSuspended } from '@nuxt/test-utils/runtime'
import FacetBarChart from '~/components/Compare/FacetBarChart.vue'

// Stub the heavy chart library
vi.mock('vue-data-ui/vue-ui-horizontal-bar', () => ({
VueUiHorizontalBar: { template: '<div class="chart-stub"><slot name="fallback" /></div>' },
}))
vi.mock('vue-data-ui/style.css', () => ({}))

describe('FacetBarChart skeleton loaders', () => {
const baseProps = {
values: [null, null, null],
packages: ['react', 'vue', 'svelte'],
label: 'Weekly Downloads',
description: 'Downloads per week',
facetLoading: true,
}

it('renders skeleton loaders when facetLoading is true', async () => {
const wrapper = await mountSuspended(FacetBarChart, {
props: baseProps,
})

const html = wrapper.html()
// Should show skeleton elements
expect(html).toContain('h-7')
})

it('skeleton bar label widths vary across packages (not all the same)', () => {
// The formula used is: width = `${40 + (i * 17) % 40}%`
// For 3 packages: i=0 -> 40%, i=1 -> 57%, i=2 -> 74%
const packages = ['react', 'vue', 'svelte']
const widths = packages.map((_, i) => `${40 + ((i * 17) % 40)}%`)

// Verify they are not all the same (the bug was all bars had the same width)
const uniqueWidths = new Set(widths)
expect(uniqueWidths.size).toBeGreaterThan(1)
})
Comment on lines +30 to +39
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Regression test is detached from the component output.

Line 30 to Line 48 currently re-tests arithmetic in the spec itself, so it can still pass if the template no longer applies those widths. Please assert widths from rendered skeleton DOM/styles instead.

✅ Proposed test rewrite (DOM-coupled) 
-  it('skeleton bar label widths vary across packages (not all the same)', () => {
-    // The formula used is: width = `${40 + (i * 17) % 40}%`
-    // For 3 packages: i=0 -> 40%, i=1 -> 57%, i=2 -> 74%
-    const packages = ['react', 'vue', 'svelte']
-    const widths = packages.map((_, i) => `${40 + ((i * 17) % 40)}%`)
-
-    // Verify they are not all the same (the bug was all bars had the same width)
-    const uniqueWidths = new Set(widths)
-    expect(uniqueWidths.size).toBeGreaterThan(1)
-  })
-
-  it('skeleton width formula produces values within 40-80% range', () => {
-    const indices = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
-    for (const i of indices) {
-      const width = 40 + ((i * 17) % 40)
-      expect(width).toBeGreaterThanOrEqual(40)
-      expect(width).toBeLessThan(80)
-    }
-  })
+  it('renders varying skeleton label widths within 40–79% for package rows', async () => {
+    const wrapper = await mountSuspended(FacetBarChart, { props: baseProps })
+    const rows = wrapper.findAll('div.flex.items-center.gap-3')
+    const widths = rows.map((row) => {
+      const style = row.find('span.h-3.shrink-0').attributes('style') || ''
+      const match = style.match(/width:\s*(\d+(?:\.\d+)?)%/)
+      return Number(match?.[1])
+    })
+
+    expect(new Set(widths).size).toBeGreaterThan(1)
+    for (const width of widths) {
+      expect(Number.isFinite(width)).toBe(true)
+      expect(width).toBeGreaterThanOrEqual(40)
+      expect(width).toBeLessThan(80)
+    }
+  })

As per coding guidelines, "Write unit tests for core functionality using vitest."

Also applies to: 41-48


it('skeleton width formula produces values within 40-80% range', () => {
const indices = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
for (const i of indices) {
const width = 40 + ((i * 17) % 40)
expect(width).toBeGreaterThanOrEqual(40)
expect(width).toBeLessThan(80)
}
})

it('renders one row per package in the skeleton', async () => {
const wrapper = await mountSuspended(FacetBarChart, {
props: {
...baseProps,
packages: ['pkg-a', 'pkg-b', 'pkg-c'],
facetLoading: true,
},
})

// The skeleton renders one flex row per package
const html = wrapper.html()
// flex items-center gap-3 = the skeleton row class
const rowMatches = html.match(/flex items-center gap-3/g)
expect(rowMatches).not.toBeNull()
})
Comment on lines +62 to +64
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Assert exact row count, not only presence.

Line 63 only checks that at least one match exists. To verify “one row per package”, assert the exact number of rendered rows.

🎯 Tightened assertion 
-    const rowMatches = html.match(/flex items-center gap-3/g)
-    expect(rowMatches).not.toBeNull()
+    const rowMatches = html.match(/flex items-center gap-3/g) ?? []
+    expect(rowMatches).toHaveLength(3)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const rowMatches = html.match(/flex items-center gap-3/g)
expect(rowMatches).not.toBeNull()
})
const rowMatches = html.match(/flex items-center gap-3/g) ?? []
expect(rowMatches).toHaveLength(3)
})

})
Loading