@lab1210Submitted 2 months ago
Prakhar Nagpal
@slightlybelowzenAll comments
- @slightlybelowzenPosted 2 months ago
- The font sizes are too small, they need to be updated
0.6rem
is about9.2px
, which is much smaller than the required font size. It is14px
for the base text. - Use semantic HTML, going from
<h1>
to<h5>
is not a great idea, making the section headingsh2
would make much more sense semantically.
0 - The font sizes are too small, they need to be updated
- @WindowsM16aSubmitted 2 months ago@slightlybelowzenPosted 2 months ago
- The font weight of the heading seems to be off, although I could just be missing something with that one.
- The font weight of the links is also off by 100, the design system specifies 700 (use inter bold)
1 - @MacRay-labSubmitted 2 months agoWhat are you most proud of, and what would you do differently next time?
I am proud of the improvement in my css for now but I look foward in learning more.
@slightlybelowzenPosted 2 months ago- Usually good to separate your constants in the style sheet as variables in the css, so you don't have to wonder what the value represents and go back to it again and again.
- Some of the values in the css are not taken from the design file, which is why the solution screenshot looks different, you can see the value of the spacing, line height, max-width etc. from the design spec. (Not sure if this project provides access to the figma design file without premium)
0 - @abbas-devlopSubmitted 2 months agoWhat are you most proud of, and what would you do differently next time?
i would use flexbox to center the div instead of the old way where we use position absolute
@slightlybelowzenPosted 2 months ago- The
<h2>
element hasn't been sized correctly. The design spec uses22px
to style it, and since no size has been specified in the css, it defaults to the browser default of1.5em
which is24px
assuming a base font size of16px
(also since the base font size hasn't been set in the css for thebody
element. - It's a good idea to apply a reset of
* { margin: 0; padding: 0; box-sizing: border-box; }
so you don't have default margins of elements messing with spacing and padding of elements.
Marked as helpful2 - The