@metascifiSubmitted about 2 months ago
Latest comments
- P@oridrPosted about 2 months ago
- It looks good, and the HTML/CSS code is easy to read.
- I would use more semantic HTML - for example the cards can be
<article>and<h2>for card titles. - The cards are taller than needed, because
.card-headinghas the wrongfont-size, and.card-quotehas the wrongline-height.
0 - @JuniorGodoyOrtegaSubmitted about 2 months agoP@oridrPosted about 2 months ago
Overall design looks good at desktop and mobile sizes. Great use of
grid-template-areaswithalign-selfto position the cards. Using your idea, I've simplified my own code for both desktop and tablet.Issues:
- You should handle the tablet state layout
- Cards background should be white
- You can remove attribution
Marked as helpful1 - @lulungarSubmitted about 2 months agoP@oridrPosted about 2 months ago
Desktop and mobile exact sizes look good. The problem is the sizes between them. To avoid the need to declare multiple media queries, you can the sizes more fluid by using
clamp()orwidth: min()with relative values (percentages for example) to make thewidthmore fluid, and static values (PX for example) to define the limits.You should also use more REMs than PX to respect the base font size set by the user, and allow elements to resize on zoom.
Marked as helpful1 - @Sajad-NajafiSubmitted about 2 months agoP@oridrPosted about 2 months ago
Looks good, and well structured overall.
Issues:
- Your HTML is mostly semantic. You can replace the sections'
<div>tags, with actual<section>tags. - The widths, margins, and border radius are off on mobile, tablet, and desktop.
0 - Your HTML is mostly semantic. You can replace the sections'
- P@davuplsSubmitted 2 months agoWhat are you most proud of, and what would you do differently next time?
How quickly I finished the webpage.
What specific areas of your project would you like help with?I think I need to find a system for naming tags.
P@oridrPosted 2 months agoLooks very good and responsive:
Issues:
- Set the body
min-heightto100vh(100dvhis better for mobile). - The card inline (left/right) paddings are lost in small size (at about 380px).
0 - Set the body
- P@davuplsSubmitted 2 months agoWhat are you most proud of, and what would you do differently next time?
Using variables and short time it took to make
P@oridrPosted 2 months agoOverall looks good.
Issues:
- Prefer classes to ids, especially when an item (card) might appear multiple times.
- Height is a bit off
- Prefer REM unit to PX because it would resize on zoom, or if user changes the browser texts defaults. The standard default is 16px. You can compute all other sizes from it.
0
