Design comparison
Solution retrospective
Decided to fully get into web development last month so this is my first challenge. Any tips and advice are much appreciated.
Community feedback
- Account deleted
Hello there! 👋
Congratulations on finishing your challenge! 🎉
I have some feedback on this solution:
- Always Use Semantic HTML instead of
div
like<main>
<header>
<footer>
, etc for more info
i hope this is helpful and goodluck!
Marked as helpful2@dorianarriPosted almost 3 years ago@Old1337 Thank you! I'll keep this in mind from now on. 👍
1 - Always Use Semantic HTML instead of
- @GitHub-dev12345Posted almost 3 years ago
If you want to reduce your accessibility, Change this code. 😊
<div class="container"> to <main> ( Used main tag for main card design ) <div class="attribution"> to <footer> ( Used footer tag for footer design) I hope you find this helpful & Nice Work Keep it up 👍👍Marked as helpful1@dorianarriPosted almost 3 years ago@GitHub-dev12345 Thanks! I'll start using semantic tags more. I appreciate the input. 😄
0 - @HolllyyyyPosted almost 3 years ago
Great job @dorianarri :3 You can add box-shadow if u would like it.
1 - @PhoenixDev22Posted almost 3 years ago
Greeting @dorianarri ,
I have few suggestions regarding your solution:
- To tackle the accessibility issues, you can add a
<h1>
withclass="sr-only"
(Hidden visually, but present for assistive tech).
.sr-only { border: 0 !important; clip: rect(1px, 1px, 1px, 1px) !important; -webkit-clip-path: inset(50%) !important; clip-path: inset(50%) !important; height: 1px !important; margin: -1px !important; overflow: hidden !important; padding: 0 !important; position: absolute !important; width: 1px !important; white-space: nowrap !important; }
-
You should use
em
andrem
units .Bothem
andrem
are flexible, Usingpx
won't allow the user to control the font size based on their needs. -
` height: 490px;It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.
Overall , your solution is good.Hopefully this feedback helps.
1@dorianarriPosted almost 3 years ago@PhoenixDev22 Interesting, I'll have to look into 'sr-only' and try to understand 'em' more. Thank you.
0 - To tackle the accessibility issues, you can add a
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord