@andreasremdt
Posted
Hey @agnar-nomad,
Nice job on the challenge! It looks so close to the design, great work! ::before
and ::after
can be tricky to understand, keep going and these things will become easier. :-)
Some ideas to improve your code:
- Your title "Equilibrium #3429" uses an
h3
element, but since it's the first (and only) heading on the page it makes more sense to use anh1
instead. That's also the only remark on your accessibility report. Page headings should always start at the highest (h1
) and incrementally go down until the lowest (h6
). - You don't need to use both pseudo-classes, you can achieve the same result with just one (either
::before
or::after
, it doesn't matter).
.nft-container::before {
content: "";
position: absolute;
inset: 0;
background: var(--cyan) url(images/icon-view.svg) center no-repeat;
opacity: 0;
}
The above code achieves the same but is quite simplified:
- You can combine
background-color
,background-image
,background-position
, etc. into a single shorthand property calledbackground
. inset: 0
is a shorthand fortop: 0; left: 0; bottom: 0; right: 0
. It just saves time and lines of code :-)- Also, by using
inset: 0
, you don't needwidth
andheight
as the element will stretch all the way. content
should be an empty string, since you set the background image via thebackground
property.
Additionally, I wouldn't use an ul
element for the Ethereum price, since semantically it doesn't make the most sense. A simple div
or p
should do the job as well and is semantically more correct.
I hope my feedback helped you, let me know if you have any questions :-)
Marked as helpful
@agnar-nomad
Posted
@andreasremdt Hello friend,
no questions, you have made very good points.
Thank you for taking the time to write this and helping me reduce my code. Much appreciated. I will include changes shortly.