refactor: add missing IconClass type to icon props#2462
refactor: add missing IconClass type to icon props#2462C1ANCYSz wants to merge 6 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
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 (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis pull request tightens TypeScript typings for icon properties in two Vue components: Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ghostdevv
left a comment
There was a problem hiding this comment.
Good catch! The import path just needs updating - also in future could you create a branch to PR from instead of using your main branch, as it makes it easier for us 🙏
Co-authored-by: Willow (GHOST) <ghostdevbusiness@gmail.com>
Co-authored-by: Willow (GHOST) <ghostdevbusiness@gmail.com>
|
Thanks |
🔗 Linked issue
Partially resolves #1256
🧭 Context
The codebase already had an
IconClasstype defined in~/types/iconfor Iconify icon class strings (e.g.
i-lucide:home). However, somecomponent props that accept icon classes were still typed as plain
string,missing out on type safety.
📚 Description
Applied the existing
IconClasstype to props that accept Iconify iconclass strings in two components:
app/components/CollapsibleSection.vue:icon?: string→icon?: IconClassDiff/SidebarPanel.vue:icon: string→icon: IconClassinsidethe
sectionMetacomputed recordFull resolution of #1256 would require a dedicated icon component that
enforces
IconClassusage across the whole codebase, as noted by maintainers.