@moniquevnnSubmitted 4 months ago
NG
@nguernseAll comments
- @nguernsePosted 4 months ago
Good job! Visually looks good. Some suggestions:
- You have multiple uses of the
h1
tag. It's generally advised to keep it to oneh1
per page. See here. Also, in the context of this project, using theh1
for the.TAG
and.post-copy
doesn't quite make sense here. Might opt for<span>
or<div>
for the.TAG
and<p>
for the.post-copy
- The
.container-img
class could be refactored by moving thepadding
into the.container
class. Then you could remove thediv
which creates an unnecessary nesting.
0 - You have multiple uses of the
- @ahmedraza032Submitted 4 months ago@nguernsePosted 4 months ago
Looks good as is. Great job!
For future consideration, you might refactor the
style.css
file to utilize css variables. This would benefit in the future, if the design system changes or you add more classes with similar attributes, allowing you to edit a single variable versus everywhere the same value is being used.For example, working with the design system
colors
::root { --color-white: hsl(0, 0%, 100%); --color-slate-300: hsl(212, 45%, 89%); --color-slate-500: hsl(216, 15%, 48%); --color-slate-900: hsl(218, 44%, 22%); }
So then whenever you use the dark grey color you can do:
p { color: var(--color-slate-900); } .text-content { color: var(--color-slate-900); }
Marked as helpful0