feat: code file tree switch file to active node#1977
feat: code file tree switch file to active node#1977btea wants to merge 8 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughAdds a template ref Suggested reviewers
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb857c39-22d7-4d7d-bdb1-c3dad6a02d27
📒 Files selected for processing (1)
app/components/Code/FileTree.vue
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/Code/FileTree.vue (1)
40-45: Consider using genericquerySelectorfor type safety.The type assertion
as HTMLElementworks but could be tightened. Using the generic form avoids a cast and aligns with the coding guideline for strictly type-safe code.Suggested improvement
const scrollIntoView = () => { - const el = treeRoot.value?.querySelector('[aria-current="true"]') as HTMLElement + const el = treeRoot.value?.querySelector<HTMLElement>('[aria-current="true"]') if (el) { el.scrollIntoView({ block: 'center' }) } }As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52934022-cdcd-4f8b-b5df-3d63f0824666
📒 Files selected for processing (1)
app/components/Code/FileTree.vue
ghostdevv
left a comment
There was a problem hiding this comment.
There is an interesting bug where if you load the page on a small screen when it scrolls the scroll isn't contained to the file tree
|
I tested it with the preview link, and when I clicked the icon in the lower right corner to expand the side tree structure, it correctly retrieved the node and scrolled the node into the view area. |
ghostdevv
left a comment
There was a problem hiding this comment.
I can't reproduce the scroll issue I had a couple weeks back, so LGTM!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/components/Code/FileTree.vue (1)
45-50:⚠️ Potential issue | 🟠 MajorRun the path watcher only from the root tree instance.
At Line 45, this watcher still executes in every recursive
CodeFileTreeinstance. That can queue repeatednextTick(scrollIntoView)calls (Line 50) and redundantautoExpandAncestorswork on each path change.Proposed fix
watch( () => props.currentPath, path => { - if (path) { + if (path && depth.value === 0) { autoExpandAncestors(path) nextTick(scrollIntoView) } }, { immediate: true }, )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95746bab-bc3a-40d5-ae1f-f6c1e2a49c9e
📒 Files selected for processing (1)
app/components/Code/FileTree.vue
ghostdevv
left a comment
There was a problem hiding this comment.
ah wait, it seems to still scroll even when the thing is already visible 🤔
|
In this case, the |
The smooth made it much more obvious but it happens without it 🫠 |
|
That's true. But I think the silky smooth scrolling effect looks good right now. 😄 |
🔗 Linked issue
Scroll the active file in the files list into view when it's clicked/page loaded
🧭 Context
📚 Description
When switching files in the code file tree area, the selected file will automatically jump to the center of the area.