Design comparison
Solution retrospective
Text decoration for crashing the old price
What challenges did you encounter, and how did you overcome them?Fontawsome somehow icons were not working, so I decided to use SVG pic on button so yeah that was my challenge.
What specific areas of your project would you like help with?Coders always tell me to code from phone device to computer device and I don't understand maybe someone from here we can use video call and assist my mistakes that would be so much help
Community feedback
- @grace-snowPosted 8 months ago
I'm afraid there are quite a few problems with this. It's overflowing my screen and I can see some significant accessibility and performance issues in the html. I'll try to list them all out but don't be discouraged — these are all very common things.
Before that though, have a read of this post about how to plan out the html. Don't just copy it– read and make sure you understand the process of planning out the right html for the content in the design: https://fedmentor.dev/posts/html-plan-product-preview/
Read that and refactor your HTML before anything else.
And make sure you're not repeating Google font preconnect links. You should be able to select the exact families and weights you need in one go from Google fonts and add the 3 lines it gives you instead of all of these.
1@grace-snowPosted 8 months agoIn css
- there is no element/selector called
font
so remove that from css. - get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
- never limit the height of elements that contain text, including the body. It must never have a max height. Perhaps you meant to use min-height? It's the max height that is breaking the solution completely on my phone.
- don't use position absolute to center the component on the screen. Use flexbox. Make the body a flex column and use properties to center the component.
- the max width on the component must be in rem not px. This allows it to scale correctly for all users even those who change their default text size. It must not have a width at all (unless 100%).
- the image must not have a max width in pixels. Its max width must be 100% but that is included in a css reset anyway.
- media queries must be defined in rem or em not px. You're also setting the media query at the wrong point. Read this and I hope it fixes your understanding: https://fedmentor.dev/posts/responsive-meaning/.
- make sure the component can't hit screen edges. Add some padding to the body.
- the image is distorting. Include an object fit property.
1 - there is no element/selector called
- @RyanFoersterPosted 8 months ago
Good job! I'll try to use your code as inspiration to better understand responsive, which looks pretty good on your site!
0@grace-snowPosted 8 months ago@RyanFoerster try viewing on a small phone emulator eg iPhone se size; try zooming in too and you'll see problems.
0@RyanFoersterPosted 8 months ago@grace-snow I'm using the element inspector on my browser but I'm having a bit of trouble with the dimensions, whether it's better to use units like px, % or rather vh, vw. I'll have to read some more documentation on this subject.
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