Recipe Page

Community feedback

• 

  haquanq ®️•1,585

  @haquanqPosted 4 months ago

  Hey @hcolmenares👋👋

  Congrats on completing the challenge!!!

  Here is my feedback on responsiveness:

  • In the body element, don't use fixed height (i'll explain later), use min-height: 100vh instead to make sure that all child elements is scrollable when overflowed (you notice when screen size is around 400px the image is hidden and not scrollable).
  • Let's call these elements such as the body container element. When you work with container elements you should avoid center it's content vertically unless it is supposed to be it, most page structure tend to be horizontally centered and for the vertical gap you should use padding or margin to achieve it (most of the time users with a wide screen want it's content to be centered). Also, when you only centering a single element, this old trick is cleaner:

  .recipe {
       margin: 0 auto:
       /* 
           equal to (both side will have the same margin === the element is centered)
       */
       margin-left: auto;
       margin-right: auto;
       margin-top: 0;
       margin-bottom: 0;
       /* now is time to get rid of the flexbox on the container */
  }
  

  • You should decide what type of page are you building, is it desktop or mobile-first? Then try to use the media query from one direction only (min-width for mobile-first - getting larger / max-width for desktop - getting smaller) to avoid mixing both type unnecessarily.
  • It is so important to avoid using fixed height on any element since it will make your element less responsive and adaptive to screen size changes. Instead you can use max-width / min-width / max-height / min-height to make sure the element stay in the width or height you wanted.

  I want to talk more about HTML semantic in your solution:

  • Each page must have one main landmarks and you are misusing the main element here (read more about landmarks) which means wrap your most important content of the page inside it (in this case the main will play the role of container instead of body).
  • Avoid div wrapping when ever possible because div has zero semantic meaning and only when you want the grouped elements to be styled differently than default behavior (changing the layout structure maybe vertical or horizontal). For example, the img don't need div and you can leave it alone and the structure stay the same. Just keep your HTML simple and clean.
  • The nutrition table should be using a table since it's is tabular data as the design has shown. Entire card can be wrapped inside article to represent an article about food to provide better semantic (as i said, avoid using div whenever possible).

  If you want, take a look at my solution for this challenge for more references.

  Hope these above help you in your path, have a nice day and happy coding 😁😁😁

  Marked as helpful

  1

  ---

  

  hcolmenares•80

  @hcolmenaresPosted 3 months ago

  @haquanq Thank you! I fixed my code following your suggestions! :D

  0