Order Summary Component with React and vanilla CSS (updated)
Design comparison
Solution retrospective
Early in App.css, I have the following code:
.cardContent > p { color: white; font-size: 16px; }
when I move this code anywhere else, or just remove it, all of the background styling goes away. I would love to know why that happens!
Community feedback
- @pikapikamartPosted almost 3 years ago
Hey, really nice work on this one. The overall layout on this looks really great. Also, congrats on your first challenge here at FEM! Hope that you found this one exciting to create.
Here are some suggestions for the site:
- It would be really nice to include:
*, *::before, *::after { box-sizing: inherit
On your css reset so that each element's sizing will be much easier to handle.
- Always have a
main
element to wrap the main content of the site. For this usemain
tag on the.card
selector. - The card image on this one, if you look at it, it doesn't really give much information about it since it is only acting as a decoration on this. Decorative images should be hidden for screen-reader at all times by using
alt=""
andaria-hidden="true"
to theimg
tag or onlyaria-hidden="true"
if you are usingsvg
instead ofimg
tag. - Also, when using
img
tag, you don't need to add words that relates to "graphic" such as "logo" and others, sinceimg
is already an image so no need to describe it as one. - For this one, since you didn't use
h1
for theorder summary
text, you should use theh1
as a screen-reader only heading tag. Have a look at Vanza's solution on this same challenge. Inspect the usage of theh1
on this as well the stylings applied. - The music-icon
svg
is only acting as decorative as well, so adding an extraaria-hidden="true"
attribute on thesvg
would be nice so that it will be skipped by screen-reader. - Use
h2
instead ofh4
for theannual plan
text. When using a heading tag, make sure you are not skipping a level. If you useh4
then make sure thath1, h2, h3
are all present. - On mobile state, at around 350px, the
padding
on the.cardContent
makes the layout's size not respond on the screen-size changes. Adjusting it would be nice.
Aside from those, great job again on this one.
Marked as helpful1@lenniecottrellPosted almost 3 years ago@pikapikamart wow thank you! This feedback is so thorough! Feedback like this definitely motivates me to do more :)
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