Submitted over 1 year ago
Order-summary using flex-box, CSS variable
@ismailakinkunmi
Design comparison
SolutionDesign
Solution retrospective
Am I following the best practice and where do I need to improve? I really appreciate any help you can provide.
Community feedback
- @fernandolapazPosted over 1 year ago
Hi ππ», in case you want to take a look:
HTML / ACCESSIBILITY:
- The main content of every page (the card in this case) should be wrapped with the
<main>
tag.
- Every page should have an
<h1>
. Maybe the 'heading-text'.
CSS:
- Think about using relative units like rem or em since they are better for scalable layouts. Something simple to start with would be to convert to rem (1 rem equals 16px by default). Consider this suggestion especially for the
font-size
.
- It might be good to get used to designing with the Mobile first approach, which means designing for mobile first and then for desktop or any other device, as it is widely considered best practice.
- You forgot to add the active states in the interactive elements.
I hope itβs useful : )
Regards,
Marked as helpful0 - The main content of every page (the card in this case) should be wrapped with the
- @hitmorecodePosted over 1 year ago
Congratulations well done. Here are a few things you can do to make it a little bit better.
- The background is not all the way across the width of the body, you can fix this by adding
background-size: contain;
- The shadow on the button is too strong, lower de opacity to maybe 25%.
- For the change, button and cancel, add a transition and make the colors change on hover.
- When hovering over the button, change and cancel, change the cursor to a pointer. This way the user knows you can click on it.
.btn-container .primary-btn { border-radius: 5px; background-color: var(--Bright-blue); color: var(--pale-blue); width: inherit; padding: 10px; border: none; box-shadow: 1px 5px 3px hsla(245, 75%, 52%, 0.25); ### lower opacity for the shadow }
Marked as helpful0@ismailakinkunmiPosted over 1 year agoThank you for your review, It was helpful @hitmorecode
0 - The background is not all the way across the width of the body, you can fix this by adding
- @ismailakinkunmiPosted over 1 year ago
@fernandolapaz thank you for the code review feedback so helpful
1
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