Product Landing page using media queries

Community feedback

• 

  Yari Morcus•500

  @YariMorcusPosted about 1 year ago

  Hi SuperJulia2024,

  First of all, I think you did a great job coming up with a solution for this challenge. It looks exactly like the design and is also responsive.

  Here is my answer of your question about margin-tops and bottoms. I currently do not have time to also answer your second question about pixels and (r)em. I will come back to this solution to answer this question as well as soon as I got more time (and if I did not forget it 😳).

  1. I read somewhere (and I kind of share the same opinion) that you should always apply a margin bottom to elements. This has to do with a concept called 'margin collapsing'. If you start mixing margin tops and bottoms of elements and you do not understand this concept, then it can become quite hard to calculate the real distance between elements. This is because those margin tops and bottoms start to collide with each other. For example: if you apply a positive margin-bottom of 10px to element a, and a positive margin-top of 20px to element b, the real distance between element a and b will be 20px (not 10). This is ofcourse a simple example but in a real big project this can become quite complex (and frustrating if people start mixing them). I personally only use a margin-right and left on a complete container to push it away from other surrounding containers (a container that is completely independent from others (Containers like these should be marked with <article> (here you got a free tip haha))). Concerning your solution: I personally would have placed the margin-bottom: 1.8rem of .current_price on the container with .price.

  Other feedback I would like to give you.

  Feedback

  1. Create a hover state for your 'Add to Cart' button with enough contrast between the enabled state and hover state. This gives the user a distinct indication that they are hovering over an interactive element. You can achieve this with an overlay with a lower opacity than your current background color (to make sure people are not distracted).

  I hope you can do something with this, but if you still have questions, let me know. Don't see it as something you need to do, but rather something for later (you are always free to correct your solution).

  If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things :D.

  Marked as helpful

  0

  ---

  

  SuperJulia2024•220

  @SuperJulia2024Posted about 1 year ago

  Thanks @YariMorcus ! Much appreciated !

  Indeed, I was having some issues mixing margin-bottom and margin-top... I was not aware of that "margin collapsing" concept. I will google it

  Thanks for your help ! ;-)

  0
• 

  Nathan Ray•230

  @NathanRayMPosted about 1 year ago

  Hello,

  Great job! I have a suggestion that you may find helpful. Instead of using margin to space the paragraphs, you can set the div container to either display flex or grid, use column, and then use gap to create even spacing between them. I hope this helps.

  Example:

  div{
    display: flex;
    flex-direction: column;
    gap: 20px;
  }
  

  Marked as helpful

  0

  ---

  

  SuperJulia2024•220

  @SuperJulia2024Posted about 1 year ago

  Thanks @NathanRayM

  I will give it a try !

  0