Order Summary Componet webpage
Design comparison
Solution retrospective
Hello Community, I would like if you give me suggestion regarding code improvement, mistakes I did in code, improvement, etc. I am also facing difficulty in responsive designing in this tasks, if you suggest the improvements and other things that I should work on. It would be really helpful, if you suggest improvements or mistakes.
Community feedback
- @MiculinoPosted almost 3 years ago
Hey Abhishek, I had a look through your code and final solution and I'd say you still have quite a few things to complete:
-
First of all, there isn't any responsive design implemented in your project. Consider using media queries to solve this issue
-
Make the corners of your card and button more round with the border-radius property
-
Your "Change" text should be inside an <a> element and same goes for "Cancel Order"
-
Your button should have a hover effect. Also, remove the default border by using "border: none". Change the background color and the font you use for the text inside the button and add a box shadow similar to the one in the original design.
-
The music icon is vertically stretched.
-
The elements inside the .content div aren't properly aligned. The margin properties you used there are causing a lot of the issues.
-
Your card isn't properly aligned vertically and horizontally. You used margin, margin-left, margin-top, and margin-right to position your element. Not only that, but you used % for it. Your element is positioned absolute so it'd make more sense to use the top, bottom, left, and right properties. But for a simpler solution, you could use display flex or you could also use the display grid property along with "place-content: center"
https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Aligning_Items_in_a_Flex_Container
https://developer.mozilla.org/en-US/docs/Web/CSS/place-content
Hope this helps. Keep it up and practice as much as you can. ;)
2 -
- @grace-snowPosted almost 3 years ago
Hi. All good feedback already I'll just add a bit more (and make sure you spend time refactoring this challenge before moving on, it will be really important)
- definitely don't position absolute this card or use large margins. Try flexbox properties and a min-height 100vh on the main
- you haven't closed your main element - it's important to Indent your code consistently ever time then it will be easy for you to spot errors like that. The browser is forgiving and will auto close tags for you, but often these are in the wrong place or introduce all sorts of annoying bugs that are hard to find
- Read about how and when to write image alt text and what it's for. These all need changing
- never style on IDS. That's not what they're for. Lots of tutorials out there may teach bad practices using IDS as css or javascript selectors but you should avoid it.
I hope this adds a bit more help on top of the great tips you've had already
1@abhishekdwaghmare2000Posted almost 3 years ago@grace-snow Thank you, I will refactactor this challenge and fix this issue.Thank you for your valuable suggestion.
0 - @abhishekdwaghmare2000Posted almost 3 years ago
@Remus432 Thank you for explaining each point where I am doing wrong and providing useful resources. This is going to be really helpful to work on these things and fix this issue. Thanks Again!
1 - @darryncodesPosted almost 3 years ago
Hi Abhishek,
Nice solution, solid job all round.
- you should add
overflow: hidden;
to.card
so the image doesn't overflow and cover up the rounded edges of your card
Keep coding!
1@abhishekdwaghmare2000Posted almost 3 years ago@darryncodes Thank you so much, I am struggling to fix that, but I am not able to fix that, now it is fixed. Thanks again for sharing your valuable feedback!
0 - you should add
- @Sam-GulikerPosted almost 3 years ago
Hi, Abhisek
Double check the paddings and fonts? They seem a bit off. Next to that try to use landmarks like so:
<main> {your content}</main>
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/main_roleA heading is also nice to have :) https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements
Happy coding
1@abhishekdwaghmare2000Posted almost 3 years ago@Sam-Guliker Thank you for your suggestion, it is helpful!
0 - @nikbosPosted almost 3 years ago
Rounded corners and the color of Proceed to Payment button.
0@abhishekdwaghmare2000Posted almost 3 years ago@nikbos Thank you for your valuable feedback. It would be really helpful for me to work on this issue. Thanks again!
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