Product Landing Page( Gym ) - Feedback

Project Feedback
#1

Hello everyone,
I have completed my Product-Landing Page project. Please provide your feedback and criticism also
please check my work on flexbox in the project (i was practicing flexbox concepts).

Thank You,

Project Link: https://codepen.io/adb2396/full/WmWoEv

#2

Nice job @adb2396

A couple of things I noticed.

  • The navbar is not fixed. As soon as I start scrolling I no longer have the navbar to navigate the rest of your page.
  • I like the color change when hovering over the buttons, I’d also suggest changing the cursor to a pointer.
#3

Thank You…!! :slightly_smiling_face:
Fixed suggested problems.

#4

Nice. Looking better but when the screen size is below 480px the navbar changes and is no longer fixed. (It’s only fixed for larger screens)

#5

Changed my navigation look below 480px. But not able to fix that header element.
As I have written in code: #header{ position: fixed } header is stuck with about section leaving some white space on top. Need the advice to solve that problem.

#6

Adding top: 0; to #header should fix that. I am not sure if it’s best practice, but it worked for me.

#7

Hey there.

One thing that catch my eyes was the Work Out section with your unordered list that is spaced far out from the right content.

abdworkoutspaced
abdworkoutspaced1525Ă—482 162 KB

I made your ul stick right in the left container but it looks super strange now

abdworkout2
abdworkout21295Ă—380 80.9 KB

What I would do is put the 2 containers on top of each other and the li elements an inline-block.

Would make the list look like that under the text:

workoutulnew

EDIT:

Forgot to say you have a few semantic problems.

The italic text in your workout section is wrapped in a “em” element only. The “em” element is used to emphasize a text and it default make the text italic but it should be used to tell the browser that this text is important in your page. This shouldn’t be used only to style the text.

A correct way of making your text in italic would be to wrap it in a “p” element and add a class for it like this:

<p class="italic">text text<p>

and css

.italic {
font-style: italic;
}

The same goes for the “strong” tag, you have many elements wrapped only in strong inside a div (example: the 100$ in the pricing part).

#8

Hi,
I checked out your solution but there still problem is there is no margin between header and about section. how to work on that ?

#9

Thank You for all your feedback…!!:+1:
I cleared out all the bugs and taken all your suggestions for improvement. If you can please point out any additional bugs or points for me to work on.

#10

You could fix this by setting: #about { margin-top: 100px; } then I noticed this messes up your background-color so you can set this on the html rather than on your #wrap class like so: html { background-color: #F5F5F5; }

Hopefully this helps!

1 Like
#11