Responsive recipe card

Community feedback

• 

  induwara-thisarindu•230

  @induwara-thisarindu

  Posted about 1 year ago

  Hi good job on the submission first of all I want to say I am no expert so I will help as I can. In your css I noticed that you have written this,

  h1,h2,h3,p,div,img,section,article,main,ol,li,ul {
      margin: 0;
      padding: 0;
  }
  

  I believe it will be better if you do it like this,

  *{
      margin: 0;
      padding: 0;
  }
  

  I believe It is much more easier to write and time saving.

  Also, I think adding a height to body is what is causing the problem try removing that line and see I think the image will be visible in every screen

  html,body {
      height: 100%;
      margin: 0;
      padding: 0;
      overflow: auto;
  }
  

  The problem is that you are adding an overflow to both html and body maybe just add it to body like this

  body {
      overflow: auto; /* Ensure content is scrollable if necessary */
  }
  

  That's all I have to say Hope this helps 😊🎉

  02
• 

  Jo_WithVision•90

  @Jo-with-vision

  Posted about 1 year ago

  Hi, well done on the submission - there a few suggestions from quickly looking at your code:

  CSS

  • You don't need flex for any of the card contents.

  It should all flow in a stacked structure naturally because it's mainly block-level elements, the exception being images, which aren't block-level. But with a CSS reset that includes setting images to display: block; and max-width: 100%, they also will not be a problem.

  I suggest duplicating your entire project as a backup and then starting over with a new version by taking all the flex off of your rulesets (you can leave flex on the main container if you want to justify it centre). Remove all padding and margins for the content and just see what the content inside the recipe card is naturally doing by itself. Then you can decide, section by section if/where padding/margin etc is needed.

  Sometimes we can overcomplicate things by ADDING properties we don't need (like too much flex).

  • You can take a look at mine to see how I resolved the marker indent. I just used padding. But it's important to note that padding won't work on a li item if you have other rules overriding it (remember CSS specificity) or flex complicating things.
  • Don't use pixels, only rem/em for your max-width sizes.
  • Remove the body height of 100% (you should only need to use, if anything, min-height: 100vh) on the body/a container when using flex/grid to centre an element in the viewport. You don't need to set a width of 100% for your recipe card. Just a max-width in rem/em (remember the main .recipe-card should be an <article> not a <section>.
  • If you choose to simplify your project vs 2 and remove all the flex, you will need to revise your media query. The media query should be very minimal adjustments for this challenge (some people don't use one at all).

  html

  • Your html structure could do with revising. You have the entire card in a <section> and then an <article> inside that. Really, you should have a <main> element, and then the card would be best wrapped in an <article>. You can then use <section>'s to logically section the inner content of the card, instead of using <div> elements, which have no semantic meaning.
  • The <span>'s should be <strong> elements. They have semantic meaning to make the word more important (it's an important inline "heading" to the instruction) and come with a bold style by default so no further styling would be needed on them.
  • Try to give your class names more useful meaning; you have names like "text" but really a more helpful class name would be .instructions, .instructions-text etc. It helps making reading code easier for not only you but other people.
  • Don't forget in your <table> that calories, carbs etc are the headings and can be placed in a <th> cell to distinguish them from normal <td> data cells. You can then add the scope attribute to each of the <th> to tell screen readers if the heading is for the row or the column. (In this challenge they are for the row). It would look like this:

  <th scope="row">Calories</th>
  

  repo url

  • Your repo URL for this challenge is pointing to a repo containing all your challenges as sub-folders. It's ok that you decided to make one large repo - but it may get rather complicated the more challenges you do and people will have to hunt for the correct challenge to review in the list. Point the challenge URL to https://github.com/medic-code/frontendmentor/tree/main/recipe

  Good luck!

  01
• 

  ahmed safwat•230

  @ahhmedsafwat

  Posted about 1 year ago

  hello there, the photo isn't visible in mid to x-large screens

  01