Design comparison
Solution retrospective
Consolidates the management of product-related assets into a single module(index.js), making the application more organized and easier to maintain.
What specific areas of your project would you like help with?Please kindly comment on my semantic Tailwind CSS setup.
Please also comment on my code, please help me to improve.
Many thx.
Community feedback
- @grace-snowPosted 2 months ago
Hi,
I'm curious as to why you've decided to build such a small project with React? Not a problem, just wondering what the motivation is there.
Here are a few things I noticed that you may want to revisit:
- the button icon is decorative so should have empty alt.
- the category is not a heading its just a paragraph. (And headings should always go in order.
- consider adding a unique ID to the product heading and an aria-labelledby on the article referencing that so everything in the card "belongs" to this one product programmatically.
- the old price should be wrapped in an
s(strikethrough text) element. Optionally, it could be good to include some sr-only text in a span before it to label it to assistive tech as the original/old price. - this would be very unlikely to be a component that serves the main title/heading for a page so it should not be using a h1. A h2 would be much more important. Remember headings should go in order once a component is used on a page.
- I recommend you reconsider how you're using the em unit in this. Do you really want the card padding to scale with the current font size? It's one case where pixels may work better.
- On a related note the max width on the component would be much better if in rem not px. When some end user has a larger text size (eg 32px/200%) you wouldn't want the component to have a max width of 600px. You'd want that in rem so the layout scales nicely.
Marked as helpful1@edpauPosted 2 months ago@grace-snow Thank you for your comment, it was very helpful last time and this time, I learned a lot from you. Greatly appreciated. I will refactor it before I start my next challenge.
I think using React for a static page isn't very beneficial; there's no advantage to using React over vanilla JavaScript and CSS for this project.
But I'm focused on improving my skills in vanilla JavaScript, vanilla CSS, React and Tailwind. To strengthen my fundamentals, I'm working on small projects that allow me to revisit the basics and take the time to thoroughly learn and understand each technology. I have done a few challenge with vanilla JavaScript and vanilla CSS. So for this similar challenge, I used React and Tailwind.
Fro example in background color, I am setting my background color in index.html, as I want the safe area in iphone X or above also has the background color. I don't think it is a good practice, I think I should handle it within the React component structure. Is there a better way?
0@edpauPosted 2 months ago@grace-snow I see, I will keep searching for the answer, thank you again for your help🙏
0
Please log in to post a comment
Log in with GitHub
Join 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