Conversation
This adds a new `useVisibleItems` composable and initially adopts it in the dependencies view of a package. In follow ups, we can adopt it in more pages to reduce duplicated logic.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughA new composable 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/Package/Dependencies.vue (1)
87-103: Consider extracting the repeated visible-item limit into a single constant.Using
10three times makes future tuning error-prone. A shared constant keeps the three sections aligned.Suggested refactor
+const DEPENDENCY_VISIBLE_LIMIT = 10 + const { visibleItems: visibleDeps, hasMore: hasMoreDeps, expand: expandDeps, -} = useVisibleItems(sortedDependencies, 10) +} = useVisibleItems(sortedDependencies, DEPENDENCY_VISIBLE_LIMIT) const { visibleItems: visiblePeerDeps, hasMore: hasMorePeerDeps, expand: expandPeerDeps, -} = useVisibleItems(sortedPeerDependencies, 10) +} = useVisibleItems(sortedPeerDependencies, DEPENDENCY_VISIBLE_LIMIT) const { visibleItems: visibleOptionalDeps, hasMore: hasMoreOptionalDeps, expand: expandOptionalDeps, -} = useVisibleItems(sortedOptionalDependencies, 10) +} = useVisibleItems(sortedOptionalDependencies, DEPENDENCY_VISIBLE_LIMIT)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cfe299c8-0c1a-4e32-aba0-8fd307d0639b
📒 Files selected for processing (2)
app/components/Package/Dependencies.vueapp/composables/useVisibleItems.ts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/unit/app/composables/use-visible-items.spec.ts (2)
6-40: Consider parameterising the initial-state tests withit.each.The four cases share the same arrange/assert pattern; a table-driven structure would reduce duplication and make future scenarios easier to add.
♻️ Example refactor
describe('initial state', () => { - it('returns all items when list is within limit', () => { - const { visibleItems, hasMore, hiddenCount } = useVisibleItems(['a', 'b', 'c'], 5) - expect(visibleItems.value).toEqual(['a', 'b', 'c']) - expect(hasMore.value).toBe(false) - expect(hiddenCount.value).toBe(0) - }) + it.each([ + { name: 'within limit', items: ['a', 'b', 'c'], limit: 5, visible: ['a', 'b', 'c'], hasMore: false, hidden: 0 }, + { name: 'exceeds limit', items: [1,2,3,4,5,6,7,8,9,10], limit: 5, visible: [1,2,3,4,5], hasMore: true, hidden: 5 }, + { name: 'equals limit', items: [1,2,3], limit: 3, visible: [1,2,3], hasMore: false, hidden: 0 }, + { name: 'empty list', items: [], limit: 5, visible: [], hasMore: false, hidden: 0 }, + ])('returns correct state when $name', ({ items, limit, visible, hasMore, hidden }) => { + const state = useVisibleItems(items, limit) + expect(state.visibleItems.value).toEqual(visible) + expect(state.hasMore.value).toBe(hasMore) + expect(state.hiddenCount.value).toBe(hidden) + }) })
103-146: Add explicit boundary tests for non-positive limits.Please add contract tests for
limit = 0and negative limits (either clamped behaviour or thrown error), so this edge behaviour is pinned down and protected from regressions.As per coding guidelines, "Write unit tests for core functionality using
vitest."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c97c3d9-f61e-4ff4-9e75-3dfc80a7bfdc
📒 Files selected for processing (2)
app/composables/useVisibleItems.tstest/unit/app/composables/use-visible-items.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/composables/useVisibleItems.ts
1 participant
🔗 Linked issue
N/A
🧭 Context
Reducing duplicated logic for determining which items are visible in a page.
📚 Description
This adds a new
useVisibleItemscomposable and initially adopts it inthe dependencies view of a package.
In follow ups, we can adopt it in more pages to reduce duplicated logic.