Week 12 – Design Handover – Trista & Nathalie - #57
Conversation
kathinka
left a comment
kathinka
left a comment
There was a problem hiding this comment.
Nathalie and Trista! such a good effort! 👏🏼 the designer must be super happy with this result. I found some small suggestions for improvements, but I guess you would be happy to ignore them all except maybe two of them. Really well done. very clean setup too🤩
| <img src={image} alt={alt} className='w-[342px] h-[289px] rounded-[10px]'/> | ||
| <div className="flex flex-col gap-y-2"> | ||
| <h5 className="text-xl">{courseName}</h5> | ||
| <p className="text-5xl">{courseInfo}</p> |
There was a problem hiding this comment.
I had a similar problem as you still have in safari for text - it overflows the container- use "white-space:normal" in order to keep it within the container.
There was a problem hiding this comment.
Thank you for the tip. We really need to keep an eye on other browser.
| <img src={image} alt={alt} className={imageStyle} /> | ||
| <div className="flex flex-col gap-y-[8px] md:gap-y-[15px] lg:gap-y-[20px]"> | ||
| <h3 className=" text-lg md:text-xl ">{instructorName}</h3> | ||
| <p className=" lg:text-7xl text-base leading-tight tracking-wide"> |
There was a problem hiding this comment.
same issue in safari as the class description container- use "white-space:normal" in order to keep it within the container.
There was a problem hiding this comment.
maybe store the info and images to this section in an object?
There was a problem hiding this comment.
maybe store instructor info in an object, so you could just loop over it and keep it more DRY?
There was a problem hiding this comment.
there is a lot if images who are huge (several megabytes), the site would preform much faster if the images was compressed and/or also had webp or avif formats.
There was a problem hiding this comment.
you could potentially save 20 mb+ if the images where optimized for web.
Using image CDNs
Compressing images
Replacing animated GIFs with video
Lazy loading images
Serving responsive images
Serving images with correct dimensions
Using WebP images
There was a problem hiding this comment.
Yeah good point. I actually thought about lazy loading the images that are not in the viewport, but were already late with the hand in. Ideally, we would use WebP and lazy loading. Thank you for this input!
| return ( | ||
| <div className="flex flex-row items-center gap-[11px] self-end md:justify-end md:gap-[20px] md:self-start "> | ||
| <p className="text-8xl md:text-9xl text-yellow lg:text-lg"> | ||
| Read more about it{" "} |
There was a problem hiding this comment.
We discussed it with the designer. It's not clickable. At least not for now. Therefore, we didn't put a link behind it.
| return ( | ||
| <div className="flex flex-row items-center gap-[11px] self-end md:justify-end md:gap-[20px] "> | ||
| <p className="text-8xl md:text-9xl text-yellow lg:text-lg"> | ||
| Read more about it{" "} |
There was a problem hiding this comment.
same as middlereadmore - i miss some feedback in order to see that this is clickable.
| import smallTraining from "/valueImg/smallTraining.png" | ||
|
|
||
| export const ValueCards = () => { | ||
| return ( |
There was a problem hiding this comment.
could it be smart to store the data needed in a separate object, and then just loop through it to render the content to keep it a bit more DRY?
| The Zumba Hall | ||
| </h1> | ||
| <p className="mx-[24px] mb-3 mt-[8px] leading-tight md:mb-6 md:text-base lg:mb-7 lg:mt-1 lg:text-3xl"> | ||
| Join our training centre and get fit through{" "} |
There was a problem hiding this comment.
no it's just for the gap between "through" and "Dance"
| @@ -0,0 +1,37 @@ | |||
| import { useState } from "react"; | |||
|
|
|||
| export const FindUsButton = () => { | |||
There was a problem hiding this comment.
These buttons seems the same except for the copy - why make two different versions?
There was a problem hiding this comment.
They are almost the same. But yeah we could have just pass the content into the button element, but you know. We went the easy route 😉
| The Zumba Hall | ||
| </h1> | ||
| <p className="mx-[24px] mb-3 mt-[8px] leading-tight md:mb-6 md:text-base lg:mb-7 lg:mt-1 lg:text-3xl"> | ||
| Join our training centre and get fit through{" "} |
There was a problem hiding this comment.
no it's just for the gap between "through" and "Dance"
NellaMori
left a comment
NellaMori
left a comment
There was a problem hiding this comment.
Hello Nathalie and Trista, congratulations for completing your week 12 project. It looks ✨ awesome ✨ The site looks very close to the design and followed our requirements. How did you like Tailwind? Seems like you tackled it in the right way and implemented a great result! Would have been nice to see the carousel used rather than the scrollable section, maybe it's something you can improve on a second iteration of your implementation.
Good job nailing accessibility and responsiveness, kudos! 👏
| "react": "^18.2.0", | ||
| "react-dom": "^18.2.0" | ||
| "react-dom": "^18.2.0", | ||
| "react-responsive-carousel": "^3.2.23", |
There was a problem hiding this comment.
please remove the installed packages if unused 🧹
There was a problem hiding this comment.
We didn't have the time to make a proper carousel. Therefore, the fastest way was the horizontal scroll. We talked with the designer about it, and it was okay for her.
We might consider adding a carousel when we find the time to change it.
Tailwind itself was fun. It made sense to us to use Tailwind. It's nice to add something new to our skill set.
4 participants
Netlify link
https://the-zumba-hall.netlify.app/
Collaborators
[trista1987]