Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Responsive recipe card

P

@medic-code

Desktop design screenshot for the Recipe page coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


What are you most proud of, and what would you do differently next time?

Getting the design mostly accurate. i think i would've probably done less on getting the inbetween sizes working correctly and sticked to the desktop and mobile view only.

What challenges did you encounter, and how did you overcome them?

Whilst i was able to get the desktop and mobile versions to the design, inbetween sizes of height were incredibly difficult to figure out how to fix the overflowing top part of the card. It looked like the flexbox was actually causing this problem and was unable to find a solution to center the card without, despite thinking about floats or using auto margin.

Additionally getting the padding between the bullet point and the text in the list was really challenging, solutions made to seem this was relatively easy but nothing seemed to work, using ::marker, using ul or li padding/margin. Not quite sure why!

What specific areas of your project would you like help with?

Better responsiveness on intermediate desktop sizes, the card overflows on my own laptop for example.

A better way to add padding on the bullet point lists.

Community feedback

@induwara-thisarindu

Posted

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 😊🎉

0

P

@medic-code

Posted

@induwara-thisarindu

Thanks for the feedback!

For the first change I think i'm being a little lazy here as it feels a little shotgun to use normalize css for these small projects, but yes indeed i may just use * { } instead from here on. I'd have a proper CSS reset in a live project.

Thanks for the tip about height:100% I think I was using this to center the card with flexbox, as you need to define the parent container as having a height of 100% or 100vh. Your suggestion does get me closer to the overflowing problem it but will need to think of a different way to center.

I suspect i wont need the overflow setting on html,body if i remove height:100%.

Gets me a little closer, thanks!

0

@induwara-thisarindu

Posted

@medic-code do you just want to center the card?

0

@Jo-with-vision

Posted

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!

0

P

@medic-code

Posted

Thank you so much for the detailed feedback, it really means a lot that you spent this time doing it @Jo-with-vision in other lines of work i have done the same for others so it really is appreciated.

With regard to CSS think you suggest a good approach to see what the normal flow of the card does prior to flexbox usage, i do need to remember this generally.

I will take on board your HTML feedback, all seems sensible.

With regard to the repo url - makes sense to point the challenge url to the specific folder - not sure why i hadn't done that previously!

1

@ahhmedsafwat

Posted

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

0

P

@medic-code

Posted

Thanks for your feedback, indeed this was part of the feedback I gave myself - The figma design does not explicitly deal with mid to x large screens so whilst it works for 1440 x 1955px - I struggled with getting it to work with mid sized screens - any feedback would be most welcome@ahhmedsafwat

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join 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