I need review on my projects please

Hi all, I’m new here and I need some feedback on my projects and advice on how I can improve my code please! https://codepen.io/miriam-jinx-olivier/pen/ExxYXrZ - my product landing page
https://codepen.io/miriam-jinx-olivier/pen/MWgdZeQ - My Tribute page
https://codepen.io/miriam-jinx-olivier/pen/eYYOGKb - Survey Form

1 Like

Hi I had a quick look. Feedback

  1. The image of the person crosslegged is stretching when I stretch the window and losing aspect rati. It would be better added in the CSS - look here https://css-tricks.com/aspect-ratio-boxes/
  2. Italics are generally a bad idea as they are hard to read
  3. You need to use media queries to remove the hamburger menu and have it appear and remove the menu when the screen is small

This helpful feed back thank you.

hello @miriam098, here’s my feedback:
product landing page: I think the navigation bar is too tall. i get that it’s because of the image on the
right but you could make it smaller. Or align the text vertically so it’s in the midpoint of the bar.
tribute page: The tribute-info could you use some centering. I’m view the page on a laptop and the text stretches from left end to right end making it hard to read. Moreover, how about a
sans-serif font?
survey form: I I think your fieldset elements need some margins. You could also align the input elements so they seem as if they are in their own column, likewise with the labels.https://css-tricks.com/label-placement-on-forms/

1 Like

Hi @miriam098 !
Congratulations on your work!:+1:

My review is about your tribute page. I think it’s a good project, although I have a couple of suggestions that may be useful :wink::

About HTML:

1- <h1> and <h2> are out of <body>. They should be inside.

2- In this case, id="main" applied to <main> would make more semantic sense than applied to <body>.

3- You could (in this case, maybe “you should”) use <figure> instead of <div> to contain <img> and <figcaption>.

4- Instead of adding a <br> at the end of each <li>, you can adjust the style of all <li> by padding (e.g., padding: 0.5em;) , margin (e.g., margin-bottom: 1em;) or line-height (e.g., line-height: 2em;).

About CSS:

1- The image is not responsive. One possible solution is to make a couple of changes in the styles of #img-div and img:

 text-align: center;
 background-color: #fcfcfc;
 max-width: 450px;  /* instead of width: 450px; */
 max-height: 450px; /* instead of width: 450px; */
 border: 1px solid #c3c3c3;
 margin: auto;
 padding: 1em;

 position: relative;
 margin: auto;
 padding-top: 1em;
 max-width: 100%; /* add this */

2- body {display: block;} is unnecessary, display: block is the default value of <body>.

3- div {align-content: center;} does not apply, because there is no div that is a flex container.

4- main {width: 100%; } It is unnecessary, as its display is “block” its default behavior is to occupy the full width of its parent.

5- The last <p> and <footer> are inside the <ul>. They should be out.

1 Like

Wow this in depth review is really what I needed, thank you so much. It really helped alot. I will work on it right away. Thank you so much!!

1 Like

Thank you very much, I will see to it right away.

I can definitely see that I wasnt paying attention to the way I write my code here lol…seeing alota embarrasing mistakes:see_no_evil:

1 Like

You are welcome @miriam098!:wink:

Congratulations @miriam098 , I see that you have made many changes to your tribute page!:muscle:
I think it has improved a lot.:ok_hand:

But if you still want to polish a couple more details :grin:, you could also:

(HTML) 1- Correct the position of </footer>. It should go before </main> and </section>.

(CSS) 2- Achieve the spacing you want between <li> with only one of the properties you used (padding, margin or line-height).
Of course, using the 3 you also achieve the desired result, but why use 3 if in this case you can achieve it with only 1?:wink:

And if you wish you can also give me your feedback about my tribute page here :blush:

Can you check if the image is still stretching? Im not sure how to remover the hamburger with media query.

I’m not sure how to align the text with the image, been trying to do that.

it is still stretching. check out some css properties for background images, I don’t know one that can stop the stretching but check this https://www.freecodecamp.org/learn/responsive-web-design/responsive-web-design-principles/make-an-image-responsive

try vertical-align: middle;