@vladmee
Posted
Hey Nathan, you have the most comprehensive solution using Next.js! Or at least the best I've checked so far :D
I used Next.js for this challenge too and I was looking for a similar tech stack for comparison. Well... I was amazed by your code: it's well-structured and easy to read, you make good use of tailwind and motion, and generally, I have a lot to learn from it.
To give back, I have a few suggestions for you as well:
I realized that most of your pages are straight-up Client components. Your application is not really taking advantage of the SSR capabilities of Next.js. My approach was to keep each page a Serve component with as much static content as possible then include the dynamic content as a Client component. In this way, each page will have its own metadata, plus the background, title, and data can load faster leaving for the browser to render the content.
You are using the Image component from Next.js but you're applying Tailwind classes right away, without using its built-in attributes that could make your images responsive by default. Actually, on the Crew page, on a laptop screen, the image overflows under the navbar. Not a big problem but with the addition of these two attributes you can make sure that the image always fits in its container:
<Image
fill={true}
style={{ objectFit: 'contain', objectPosition: 'center bottom' }}
/>
Hope this helps! As I already mentioned, I feel that your solution is already superior from many point of views, these are just some small details that could only make it even more perfect. Keep it up!
Marked as helpful
@NathLowe
Posted
Thank you so much for you report @vladmee. I can feel you really took time to study my code thoroughly.
As for the drawbacks you have, first, concerning all the page being client components, I really could have done better😂. I needed them to be on the client side in order to create the transition between the pages. For the SEO (title, tags, keywords...) particular to each page I could have created layout server component, but I didn't think about it.
For the images, I believe they are still responsive because I imported them statically, so NextJS got their sizes during compiling in order to proceed to responsiveness. The fill is usually when you pass a link to the Image Component and you don't know the width and the height of the image.
Thank you again for your feedback, it really helped me and I hope this answer may help you as well!
@vladmee
Posted
@NathLowe Thanks for replying!
Hey, the SEO part was just a suggestion for next time, I'm just getting to know Next.js as well and I find it amazing how easy it is to use its SSR capabilities. Wanted to share that with you!
About the images, you are absolutely right, they are responsive already! What I find a problem is how much they enlarge :D Please check how they look for me https://imgur.com/a/hjBqcBV This is my first view on a 16" macbook, no resizing or trying to "catch" a bad resolution. There are many solutions to solve this without adding fill={true} which, I agree with you, it's more of a fallback for when you don't know the size of your image.
Looking forward to seeing your next solutions!
Marked as helpful
@NathLowe
Posted
@vladmee Thank you so much for your really helpful comment man. I will make sure to fix everything as soon as possible.
Have you also noticed a glitch during page transition and planet change? 😂
@vladmee
Posted
@NathLowe lol flash, flash, flash 📸
I'm not criticizing these things so much, I see a lot of crazy motion here and there. Yours is acceptable at least!
