Will Cooley
@CooleyWCAll comments
- @medbdeyP@CooleyWC
Nice Work! In particular the desktop design looks like a strong match!
Items to check out: • There is a horizontal scroll when the screen gets smaller towards mobile size. I believe the hard-coded width and height will do this on your container - consider min-width/max-width instead. • See if you can find a way for the share "modal" to cover the name/avatar on mobile - just like the figma design. Playing around with absolute positioning and z-index might help.
- P@joeabrahamianWhat are you most proud of, and what would you do differently next time?
Yes
What challenges did you encounter, and how did you overcome them?Forgot to add media queries for mobile screen
What specific areas of your project would you like help with?media queries
P@CooleyWCNice work!
It's difficult to check the code as your code link goes to a 404. However, I do notice that the content doesn't scale down when the width shrinks - so I imagine you might have some hard-coded widths that are currently being inflexible. You might start with a mobile-first approach by getting everything to look as close as possible to the figma file at 374px, then build out the other sizes/media queries from there.
Hope that helps! - P@nmoon8019What specific areas of your project would you like help with?
HTML-- Is my use of div's acceptable? CSS-- am I using too much code to accomplish this?
P@CooleyWCYou could use an article tag (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article) instead of a div for the card. I would also use section tags instead of divs for the 'info' and 'authorbox'.
CSS: I would avoid using pixels for font-sizes - convert to rems or use clamps for better responsiveness. I usually copy clamps from this site https://utopia.fyi/type/calculator?c=320,18,1.2,1240,20,1.25,5,2,&s=0.75%7C0.5%7C0.25,1.5%7C2%7C3%7C4%7C6,s-l&g=s,l,xl,12
Also avoid setting hard widths and heights (on the container) - usually its better to use min-widths or min-widths to control the sizing.
Hope that helps!
Marked as helpful - P@JairRaidWhat are you most proud of, and what would you do differently next time?
I learned to make this quote mark position absolute to his parent.
- @AceThunder546P@CooleyWC
Looks Great! One thing to consider is using a different unit type for font-sizes instead of pixels(rems are what I use most). This helps both with responsiveness and accessibility. And I also often use the typography clamps that I get from this site: https://utopia.fyi/ - they smoothly scale your font size and they make sure the size doesn't get too big or too small. Also consider updating your README (you can use the template they give you in the project files)
Marked as helpful - @adetoadetoP@CooleyWC
Looks great! Perhaps adjust the breakpoint so it doesn't get quite as wide before switching to your desktop layout (when the image is next to the description). Maybe somewhere in the 350-500px range.
Marked as helpful - @masaudahmodP@CooleyWC
Looks great!! Perhaps consider using a separate css file for styling. A few items to compare with the original design:
Mobile:
- image should have no margin/padding.
- container should extend to full width and height.
- check colors of main heading text and the preparation time card.
- spacing between li and their respective markers should be larger.
- no bottom border after the last nutrition table row. can try not(:last-child) selector for this.
Desktop: -larger top/bottom margin
Marked as helpful - @arachoxsP@CooleyWC
Looks great! Check responsiveness on small screens
- @knurikWhat are you most proud of, and what would you do differently next time?
I would start with writing down full HTML page, and only after write CSS for it
What challenges did you encounter, and how did you overcome them?Main challenge was center div on the middle of the page. I overcome it using display: grid
P@CooleyWCLooks great!
It appears your solution has some box shadow on the top and left, whereas it should just be right and bottom. (your repo link is down so I can't look into the CSS).
- @BestOlumeseP@CooleyWC
Looks great! Perhaps check the padding (increase) for the text to make it look like the source example.