Guitar Brothers: My Product Landing Page

Code Feedback
1

Hey,

Here is the link: https://codepen.io/aOrgun/pen/LYGPQJQ
I’m open to any criticism (even the rude ones xd) as they are really important for me to improve.

I don’t know why, but somehow I couldn’t center the nav-links on media queries.
Also I was gonna put prices on the hover state of pricing section, I couldn’t do it, but will try again later.

Thanks for sparing your time!

1 Like
2

Nice job on this page. It looks pretty good to me. My only suggestion is to increase the size of the title a bit more.

Re: your nav-bar, the issue is that you’ve set the width @ 33%. Increase it a bit or change it to 100% and play around with the padding until you get is spaced correctly.

Re: pricing, do you want it to pop up as a tooltip or as a modal window? This article might be useful if you’re aiming for the latter.

3

Thanks!
About the pricing: I wanted to hide the title of the price section and replace it with the price (for instance, BASIC was gonna be FREE or X$ on hover). So I tried changing the font-size to 0 on the hover state and then tried adding the price on the hover:after with content() but when I did that all of the text had disappeared so I thought I should try it later with less elements on a different page.

4

Some things to revisit;

  • This is HTML5, the frameborder attribute in the iframe element is obsolete. Use CSS
  • Check to see that the email input is properly filled out when clicking the submit button, throw an error if not. You learned to do this when creating the survey form.
5

I think I understand what you were trying to do.

Try this…

  1. Add a class or id to your headings in the price box.
  2. Update your HTML to add the price for each option (make sure this also has a class or id).
  3. Update your CSS code so the price is hidden by default
  4. Add a CSS rules to hide the title when you hover on the div.
  5. Add a CSS rules to show the price when you hover on the div.

Here is what I did for #price-1

  1. <h4 class="default">BASIC</h4>
  2. <h4 class="pricereveal-1">FREE</h4>
  3. .pricereveal-1 {display:none;}
  4. #price-1:hover .default {display: none;}
  5. #price-1:hover .pricereveal {display: block;}

If this wasn’t what you wanted let me know.

1 Like
6

I would do it like suggested, with element visibility switching and not using content and ::before. That will give you better control and more options.

But just for reference here is one way of doing it with content and ::before.

<div class="pricing" id="price-1">
  <h4 class="price" text="BASIC"></h4>
  <hr>
  <h5>Full access to all tabs</h5>
  <h5>Basic online tab player</h5>
</div>

.price::before {
  content: attr(text);
}

.pricing:hover .price::before {
  content: "FREE";
}
2 Likes
7

@JohnJohn @lasjorg I’m a little bit late to respond sorry and thanks for the tips!