-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove this
from VGridSkeleton
props default
#3341
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, @Ashhar-24 !
I think the only thing that's preventing the linting step in the CI is the any
in the setup
call. I added a suggestion how to fix it. If the CI passes after that fix, I'll approved this PR. Otherwise I'll level another review :)
}, | ||
}, | ||
setup() { | ||
setup(props: { numElems: any; isForTab: string; }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setup(props: { numElems: any; isForTab: string; }) { | |
setup(props) { |
The types of props are defined above, and we don't need to add any type information here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the suggested changes.
But still the CI test is failing @obulat
You will need to run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went ahead and committed the suggested changes, @Ashhar-24. Now it's ready to go 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the contribution @Ashhar-24!
Thank you @obulat for your constant support. |
Fixes
Fixes #3268 by @Ashhar-24
Description
As mentioned in the issue #3268 that
VGridSkeleton
usesthis
in the props'default
function and this causes a build error in Vue 3.So I have moved default element count to the
setup
function and thenumElems
is replaced withelementCount
in thetemplate
.Note: if this PR is ready after #3322 is merged, this PR needs to remove the TODO from
packages/eslint-plugin/src/configs/vue.ts
:Testing Instructions
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin