Design comparison
Solution retrospective
Feedback appreciated :)
Community feedback
- @grace-snowPosted almost 3 years ago
Hello
Most of the problems with this solution are caused by the following code:
width: 25%; height: 86vh; position: relative; top: 50%; left: 50%; transform: translate(-50%, -50%);
This is not a good approach to take for layout or centering. Instead, use max-width in rem (or px for now) on the card, don't give it a height at all and use either css grid or flexbox to center it on the page.
Marked as helpful1 - @kmeganizPosted almost 3 years ago
Hi, Dan, good job for finishing your challenge, that quiet a step forward. I have a few things to take note:
- at your Readme file, appreciate if you change the word and delete unnecessaries.
- problem with your accessibility, but I am not sure how it look like at mobile, only can help you with this code source below.
this is ur code below which has problem:
<div class="attribution"> Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. Coded by <a href="#">Your Name Here</a>. </div>(explanation from google) Landmarks provide a way to organize the various types of content on a page for users of assistive technologies. When content is not contained in a landmark, it will be unreachable using landmark navigation, which is an important feature provided by assistive technologies such as screen readers. Read more here: https://fae.disability.illinois.edu/rulesets/LANDMARK_2/
The most important landmark roles are main and navigation, as nearly every page will include at least those regions.
You can put
<main> <div> </div> </main>If you find my feedback helpful please mark as helpful. Thank you. Continue do the hard work. You can do it! You are awesome!
Marked as helpful1 - @grace-snowPosted almost 3 years ago
Overall, you need to readdress how you're using padding and margin, consider accessibility and text size more and stop nesting css selectors. Here's what I just changed in browser. Go though bit by bit making the changes in dev tools and you should start to see why I'm making these changes:
.container { /* width: 100%; */ /* height: 100vh; */ min-height: 100vh; display: grid; place-content: center; padding: 1rem; } .container .card { /* width: 25%; */ /* height: 86vh; */ /* position: relative; */ /* top: 50%; */ /* left: 50%; */ /* transform: translate(-50%, -50%); */ max-width: 22rem; } .container .card .content { note: Don't nest css selectors! Use single class selectors to keep specificity low;; } .container .card .content p { /* margin-top: 1.5rem; */ /* padding-left: 2.5rem; */ margin-top: 1rem; } .container .card .content h1 { /* margin-top: 1.5rem; */ margin-top: 0; } .container .card .content .plan { /* margin: 1rem 2rem 1rem 2rem; */ /* padding: 1rem 2rem 1rem 2rem; */ /* grid-template-columns: repeat(4, 1fr); */ margin-top: 1rem; padding: 1rem; grid-template-columns: repeat(3, auto); gap: 1rem; } .btn { /* margin-bottom: 1rem; */ /* padding: 0.9rem 5.5rem 0.9rem 5.5rem; */ /* box-shadow: 0 14px 10px -2px rgba(70, 32, 204, 0.24); */ /* font-size: 0.7rem; */ margin-top: 1rem; padding: 1em; box-shadow: 0 0.75rem 0.5rem -0.375rem rgba(70, 32, 204, 0.24); font-size: 0.9375rem; note: should be an anchor tag; width: 100%; } .container .card .content .plan .box1 { /* padding-right: 1rem; */ } .container .card .content .plan .box3 { /* padding: 0.5rem 0 0 3rem; */ display: flex; align-items: center; justify-content: flex-end; } .container .card .content .plan .box2 li:nth-child(1) { /* position: absolute; */ /* padding-bottom: 0.5rem; */ } .container .card .content .plan .box2 li { /* padding-right: 1rem; */ } .container .card .content .plan .box2 li:nth-child(2) { /* padding-top: 1.5rem; */ } .container .card .content a { /* font-size: 0.7rem; */ font-size: 0.9375rem; display: inline-block; } body { overflow-x: hidden; } .hero { display: block; object-fit: cover; max-width: 100%; } .new-text-content-wrapper { padding: 2rem; note: smaller padding on mobile; } .box3 a { note: should be a button; } .box2 ul { note: should NOT be a ul; } .box2 { text-align: left; } .plan > * { display: flex; align-items: center; } .cancel-link { margin-top: 1rem; }
Marked as helpful0@codein-looneyPosted almost 3 years ago@grace-snow Thank you so much for taking the trouble to look and give me suggestions means a lot to me, appreciated 🙌
0 - @codein-looneyPosted almost 3 years ago
Thank you very much for your observations/suggestions. appreciated :)
0
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