Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
da441a1 to
233afd6
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a reactive Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/Package/List.vue (1)
122-133:⚠️ Potential issue | 🟠 MajorHandle empty→non-empty results as a new-search reset.
At Line 129, the reset condition skips the
oldResults.length === 0 && newResults.length > 0transition. That leavesnewSearchBatchSizestale (oftenInfinityor0) and breaks the intended animation split for subsequent appended items.💡 Proposed fix
watch( () => props.results, (newResults, oldResults) => { // If this looks like a new search (different first item or much shorter), reset if ( !oldResults || + oldResults.length === 0 || newResults.length === 0 || - (oldResults.length > 0 && newResults[0]?.package.name !== oldResults[0]?.package.name) + newResults[0]?.package.name !== oldResults[0]?.package.name ) { hasScrolledToInitial.value = false newSearchBatchSize.value = newResults.length } }, )
🧹 Nitpick comments (1)
app/components/Package/List.vue (1)
182-190: Consider extracting the repeated animation binding into a helper.The same
:class/:styleanimation logic is duplicated in both card branches. A small helper/computed would reduce drift risk when tuning animation behaviour later.Also applies to: 241-249
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3beedd05-2a22-4856-a16f-47aed15ba65e
📒 Files selected for processing (3)
app/components/Package/List.vueapp/composables/useGlobalSearch.tsapp/pages/search.vue
|
Tested this locally and the search debounce and the layout shift fixes for the Claim button work really well. LGTM! |
|
I think it might be more effective to fix this with a different UI rather than using a complex debounce system. I can really feel the lag compared to our current setup, and since we prioritize speed, it might be worth approaching this from a different perspective. @alexdln shared the Figma file yesterday, and I noticed a different layout on most pages: maybe moving the package claim and the owner to the column is the right move here. What do you think?
|
|
If this works, it will be useful in any case, since on the sidebar in the future there may be 2 and more cards - f.e. user and organization (but sorry, I'm not sure I understand the solution and can't properly review it, but result lgtm) |
bee7094 to
5e80824
Compare
|
@MatteoGabriele Thanks for pointing it out. Actually, the major part of this fix is the conditional transition animation on the list, not the debounce. I added the debounce to make the deletion experience better — when users delete characters by pressing keys quickly, without debounce the list would change immediately and cause a flickering effect. If the lag feels too noticeable, I've reduced On the other hand, the new layout looks awesome! However, on small-screen devices, a one-column layout is the only option, which brings us back to the original problem of handling the flickering suggestions area. So I think it's still necessary to do something to improve it. @alexdln Thanks for reviewing. Could you point out which part of the code is confusing to you? It might be a good signal for me to improve the implementation or add some comments to explain it. |

🔗 Linked issue
Resolves #2112
🧭 Context
As the linked issue mentioned, the search page layout shifts too much during typing, which is not a good user experience.
📚 Description
Several UX improvements to the search page.
updateUrlQuery.showClaimPromptto make the behavior more intuitive.Screen.Recording.2026-03-17.at.14.24.18.mov