Beyond the font size stuff here's some other feedback
card__infoandplan-cardshould not be section elements. This component is just one card, it's all one piece of related content, not multiple sections on a Web page.- Personally, I'd make annual plan a heading, but that's optional
- Not sure why cancel has been wrapped in a span(?) that shouldn't be necessary. Remember spans are for inline layout divs are for block... But this doesn't really need anything, a paragraph would be better as it contains text
- More questions for you than feedback - what would you expect to happen on click of each of the interactive elements Change, Proceed and Cancel? It would be good to understand why you've chosen the elements you have
- Theres no need to load a reset stylesheet separately. That's an extra server call for no reason, just include the reset in your one stesheet
- I recommend linking fonts in the document head instead of importing in css. Again, better for performance
- The card should not have a width, only max-width
- The card should definitely not have a height. That's really important to learn ASAP. Components containing text content should never have their height limited. Let the browser decide how tall it needs to be based off the content inside and available space (and font size chosen)
- Similar with the image. Give it a min height if you like but not a height. I'm surprised the reset you're using hasn't set img tags to display block max width 100% as that's a well established good base style. Maybe explore different modern resets. You may also want to read about the object fit property. It would be very useful here
- Card info and plan card must not have a height set either for the reasons outlined above. The only element that should have an explicit height in this challenge is the music icon
- Line height must never be in px. Usually this is unitless like 1.5. To calculate line height from px values divide the line height by font size
- Next time definitely work mobile first
Marked as helpful
@VitorMagnago
Posted
@grace-snow
Hello Grace. Wow, That's a lot of work to do.
Thank you very much for the tips, I will consider each one of them. I will study more and then I will redo these initial challenges.
I started doing these challenges without much knowledge because I was entering in a tutorial hell, and it seems that it was worth, I'm getting a lot of important feedback to direct my studies.
1- Can I use simple divs or is there a more semantic tag to use?
2- I used <p> because I thought the only header on the card was "Order Summary"
3- I can't remember why I used the span tag 😅
4- I believe these elements will take the user to other pages. Did I miss something here?
5- I thought about doing that, but figured it would mess up the file too much.
6- Ok
7- Thank you, I have a lot of doubts about the width and height of the elements, whether or not I should delimit.
8- Ok
9- I'll research more about reset/normalize and definitely fit property.
10- As I said before, I have a lot of questions about the size of elements.
11- I need to start working with relative units. About line height, I only specified that why I had this information in the Figma template. I believe that if I had ignored this property, it wouldn't have made that much of a difference in the final result.
12- I'll work on it.
