-
-
Notifications
You must be signed in to change notification settings - Fork 421
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
feat(card): support custom renderImage functions for Card #730
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #730 +/- ##
==========================================
- Coverage 99.54% 99.53% -0.02%
==========================================
Files 163 166 +3
Lines 6621 6865 +244
Branches 401 420 +19
==========================================
+ Hits 6591 6833 +242
- Misses 30 32 +2
☔ View full report in Codecov by Sentry. |
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.
This is a first draft. Please have a look and let me know what you like and what not.
I will continue to work on this later. I will also update the docs. |
Can someone please review my PR? Would be so sad if all the work was in vain... |
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.
Looks great. I requested a few changes, and I would also love to listen to @tulup-conner's opinion on this one.
Once this has been accepted, I will rebase it all into one clean commit. |
@rluders Could you please resolve all conversations that have been resolved from your point of view? Will @tulup-conner come by any time soon to give their final review? |
@levino I think that this one is good, if you could just fix the test - it is good to be merged then. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I still need to update the "real" documentation with an example. Putting this to draft until I have time to fix that, probably next week. Enjoy the week end and thank you for coming back to this @rluders |
@levino I think that this is a problem that we are having right now with the CodePreview, we are trying to fix it. So, I would say... maybe doesn't matter right now? |
Any news on this? |
Sorry for dropping the ball. Will try to come back to this asap. Ping me please, if I forget. |
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.
Good to go imo.
Since in some cases, for example next.js apps, one needs to use a specific and optimized `Image` component, the `Card` component now accepts a `renderImage` property to render your own component. If `renderImage` is set, `imgSrc` and `imgAlt` are ignored. TypeScript will error if the user attempts to set them at the same time. fix themesberg#706
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.
@rluders I fixed the outstanding issues. If you think they are resolved, please resolve the conversations. Other than that, I would say, this can go out. Enjoy!
Amazing job, @levino. Thank you very much. |
…#730) feat: support custom renderImage functions for Card Since in some cases, for example next.js apps, one needs to use a specific and optimized `Image` component, the `Card` component now accepts a `renderImage` property to render your own component. If `renderImage` is set, `imgSrc` and `imgAlt` are ignored. TypeScript will error if the user attempts to set them at the same time. fix themesberg#706
Since in some cases, for example next.js apps, one needs to use a specific and optimized
Image
component, theCard
component now accepts arenderImage
property to render your own component. IfrenderImage
is set,imgSrc
andimgAlt
are ignored. TypeScript will error if the user attempts to set them at the same time.fix #706
To do:
imgSrc
-> should not render the an image tag