feat(ScrollArea): add virtualizer - #2775
Open
yan-ad wants to merge 2 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughA new ChangesScrollAreaVirtualizer Feature
Sequence Diagram(s)sequenceDiagram
participant App
participant ScrollAreaViewport
participant ScrollAreaVirtualizer
participant useVirtualizer
App->>ScrollAreaViewport: mount scroll container
App->>ScrollAreaVirtualizer: pass options, estimateSize, horizontal
ScrollAreaVirtualizer->>ScrollAreaViewport: inject viewport context
ScrollAreaVirtualizer->>useVirtualizer: configure virtualization
useVirtualizer-->>ScrollAreaVirtualizer: virtual items
ScrollAreaVirtualizer->>App: invoke slot for visible items
ScrollAreaVirtualizer->>ScrollAreaVirtualizer: render positioned vnodes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)docs/content/docs/guides/migration.mdmarkdownlint-cli2 wrapper config was not available before execution 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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/meta/ScrollAreaVirtualizer.md (1)
1-70: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing
horizontalprop to the generated docs.docs/content/meta/ScrollAreaVirtualizer.mdis missinghorizontal, so the API table no longer matchespackages/core/src/ScrollArea/ScrollAreaVirtualizer.vue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/meta/ScrollAreaVirtualizer.md` around lines 1 - 70, The generated docs for ScrollAreaVirtualizer are missing the horizontal prop, so update the Props/Slots content in ScrollAreaVirtualizer.md to include horizontal alongside estimateSize, options, and overscan. Use the existing ScrollAreaVirtualizer docs structure and ensure the generated API table matches the ScrollAreaVirtualizer.vue component’s props exactly.Source: Coding guidelines
🧹 Nitpick comments (1)
docs/content/docs/components/scroll-area.md (1)
37-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnatomy import includes
ScrollAreaVirtualizerbut template doesn't demonstrate its placement.The Anatomy section (lines 37-54) lists
ScrollAreaVirtualizerin the import but the<template>only shows the non-virtualized structure. This is inconsistent with other parts that demonstrate all imported components. Consider adding a comment or second example showing whereScrollAreaVirtualizerfits, or remove it from the Anatomy import to avoid confusion. However, since the Virtualized list example (lines 119-150) already covers usage, this is minor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/components/scroll-area.md` around lines 37 - 54, The Anatomy example imports ScrollAreaVirtualizer but does not show it in the template, so the snippet is misleading. Update the ScrollAreaRoot example to either remove ScrollAreaVirtualizer from the import or add a brief note/placement hint in the template that shows where ScrollAreaVirtualizer belongs, using the existing ScrollAreaRoot, ScrollAreaViewport, and ScrollAreaScrollbar structure as the reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/docs/guides/migration.md`:
- Around line 146-166: The migration example in ScrollAreaVirtualizer is using
undefined item data and an invalid slot shape, so update it to match the actual
items source used by the docs (or clearly define a matching object array) and
render properties that exist on the slot value. Remove the manual key from the
virtualized slot content because ScrollAreaVirtualizer already assigns stable
keys internally via its vnode cloning behavior. If you keep this example, also
include ScrollAreaScrollbar alongside ScrollAreaRoot and ScrollAreaViewport so
the virtualized scroll area demonstrates usable scrolling UI.
In `@packages/core/src/ScrollArea/ScrollArea.test.ts`:
- Around line 15-77: The virtualized ScrollArea tests currently miss the
accessibility coverage used by the other ScrollArea cases. Update the
virtualized suite in ScrollArea.test.ts to include an axe assertion for the
mounted ScrollAreaRoot/ScrollAreaViewport/ScrollAreaVirtualizer setup, using the
existing test utilities or axe helper already used elsewhere in this file. Keep
the new assertion alongside the existing render and horizontal virtualization
tests so the virtualized variant is checked for a11y too.
In `@packages/core/src/ScrollArea/ScrollAreaVirtualizer.vue`:
- Around line 60-62: The cloned VNode and the template loop are both assigning a
key, which creates duplicate/conflicting identity handling. In
ScrollAreaVirtualizer.vue, update the render logic around cloneVNode and the
<component> loop so there is only one source of truth for the item key: remove
the manual key passed into cloneVNode and keep the template :key="item.key" (or
otherwise ensure both use the same stable value without duplicating assignment).
- Around line 47-56: The virtualized slot rendering in ScrollAreaVirtualizer
should be made defensive because slots.default is assumed to exist and Fragment
unwrapping can leave targetNode undefined. Update the computed virtualizedItems
logic to safely handle a missing default slot, then make the Fragment handling
in the slots.default result more robust by checking children safely and avoiding
the fragile symbol-based find/cast path. Ensure targetNode always has a valid
fallback before cloneVNode is used, and add a warning or guard when no usable
vnode can be extracted.
- Around line 14-19: ScrollAreaVirtualizer is missing ref forwarding, so parent
components cannot access the underlying container/virtualizer state. Update the
ScrollAreaVirtualizer.vue component to use the useForwardExpose composable
alongside its existing setup logic, and bind the forwarded ref to the root div
so consumers can interact with the virtualizer container (for example, to scroll
to items). Keep the change aligned with the component’s existing useVirtualizer,
injectScrollAreaRootContext, and root element wiring.
---
Outside diff comments:
In `@docs/content/meta/ScrollAreaVirtualizer.md`:
- Around line 1-70: The generated docs for ScrollAreaVirtualizer are missing the
horizontal prop, so update the Props/Slots content in ScrollAreaVirtualizer.md
to include horizontal alongside estimateSize, options, and overscan. Use the
existing ScrollAreaVirtualizer docs structure and ensure the generated API table
matches the ScrollAreaVirtualizer.vue component’s props exactly.
---
Nitpick comments:
In `@docs/content/docs/components/scroll-area.md`:
- Around line 37-54: The Anatomy example imports ScrollAreaVirtualizer but does
not show it in the template, so the snippet is misleading. Update the
ScrollAreaRoot example to either remove ScrollAreaVirtualizer from the import or
add a brief note/placement hint in the template that shows where
ScrollAreaVirtualizer belongs, using the existing ScrollAreaRoot,
ScrollAreaViewport, and ScrollAreaScrollbar structure as the reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7729bf97-df0b-486a-91a1-fc225f3c4edf
📒 Files selected for processing (9)
docs/content/docs/components/scroll-area.mddocs/content/docs/guides/migration.mddocs/content/meta/ScrollAreaVirtualizer.mdpackages/core/constant/components.tspackages/core/src/DropdownMenu/DropdownMenu.test.tspackages/core/src/ScrollArea/ScrollArea.test.tspackages/core/src/ScrollArea/ScrollAreaVirtualizer.vuepackages/core/src/ScrollArea/index.tspackages/core/src/ScrollArea/story/ScrollAreaBasic.story.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2491
Description
Verification
Summary by CodeRabbit
New Features
Documentation
Tests