-
-
Notifications
You must be signed in to change notification settings - Fork 321
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
Add booking card component #46
base: main
Are you sure you want to change the base?
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.
Hi shubhsk88 and thanks for your PR !
I've added a few comments, but you couldn't have known. I will add more documentation about adding components (rules to follow, good practice, etc ...). I will add linter rules too.
Don't forget to increment the counter of the component's category, here on file: /components/kit/components/commerce/index.tsx
Don't hesitate to contact me if you have any questions ;)
Thanks,
Charlie
const BookingCard: FC = () => { | ||
return ( | ||
<div className="lg:flex max-w-2xl mx-auto overflow-hidden rounded-lg m-24 shadow-lg"> | ||
<img |
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.
Images must be serve locally, please download this image on the project.
Then the image must be resized, compressed and optimised so that it is as light as possible (here around 30 ko).
You can do it with this tools : https://ezgif.com/resize for example.
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.
Can't you wrap that in the next image component if possible usually from version 10 I tend to use that in my projects
package.json
Outdated
@@ -33,5 +33,8 @@ | |||
"react-dom": "^17.0.1", | |||
"react-live": "^2.2.3", | |||
"tailwindcss": "^2.0.1" | |||
}, | |||
"volta": { |
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.
Volta is not useful here
|
||
const BookingCard: FC = () => { | ||
return ( | ||
<div className="lg:flex max-w-2xl mx-auto overflow-hidden rounded-lg m-24 shadow-lg"> |
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.
Can you implement the dark version of the component ? ( see : https://tailwindcss.com/docs/dark-mode)
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.
yes I will add Next/image for optimisation to the project and notify you when it's deploy ;)
|
||
const BookingCard: FC = () => { | ||
return ( | ||
<div className="lg:flex max-w-2xl mx-auto overflow-hidden rounded-lg m-24 shadow-lg"> |
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.
m-24
must to be added on container, not on component.
Component's code must ready to paste and not provide "margin" around it
I haven't heard your feedback @Charlie85270 . Please let me know |
Hi shubhsk88, Can you resolve conflicts please ? Thanks, Charlie |
It is only conflicting the new file import added I think this won't be a problem |
Is there any maintainer that is being helpful here? Or people just keep ignoring other person request |
This PR address the implementation of the booking card component in the shopping section