3
\$\begingroup\$

This is my very first attempt to write HTML and CSS. I designed this website for a friend. I'm not sure if I used the best practices or maybe I did something wrong but I just started my learning process so I would love a review to point bad practices and things I could have done better or I did wrong.

This is meant to be the first section (above the fold) of a vertical single-page website.

* {
  font-family: Open Sans;
  margin: 0px
}

#PageHeader {
  background-color: rgb(236, 100, 62);
  height: 615px;
}
/* logo and nav look aligned on the original file :c */
#logo {
  position: relative;
  margin-left: 40px;
  top: 30px;
}

a.nav:link {
  text-decoration: none;
  font-size: 12px;
  font-weight: bold;
  color: white;
  text-transform: uppercase;
  margin-right: 40px;
  float: right;
}

a.nav:hover{
  border-width: 0px 0px 3px 0px;
  border-style: solid;
}

#heading{
  color: white;
  text-align: center;
  line-height: 1.2;
  margin-top: 150px;
}

h1{
  font-size: 60px;
  margin-bottom: 30px;
}
<html lang="pt-br">
  <head>
    <title>Empreendi na Rede</title>
    <meta charset="utf-8">
    <meta name="author" content="Luciano Infanti">

    <link href="https://fonts.googleapis.com/css?family=Open+Sans:400,400i,800" rel="stylesheet">
  </head>

  <body>
    <header id="PageHeader">
      <div id="logo">
        <img src="https://s31.postimg.org/4esz43xaj/empreendipngwhite.png" height="35">
 
      <nav>
        <a class="nav" href="#">Solucoes</a>
        <a class="nav" href="#">Metodologia</a>
        <a class="nav"  href="#">Sobre nos</a>
        <a class="nav"  href="#">Depoimentos</a>
        <a class="nav"  href="#">Clientes</a>
        <a class="nav"  href="#">Contato</a>
      </nav>

      <div id="heading">
        <h1>A consultoria empresarial<br> acaba de ser redefinida</h1>
        <p id="kicker">Aliquam et placerat lacus. Donec pharetra nibh a placerat tincidunt.<br>
  Integer viverra nisl dolor, in viverra enim sollicitudin nec.</p>
      </div>
    </header>
  </body>
</html>

|improve this question
\$\endgroup\$
3
\$\begingroup\$

HTML validation

Always include a doctype declaration. These days, HTML 5 is recommended, so the document should start with <!DOCTYPE html>.

The <div id="logo"> is unclosed. That would likely explain your vertical alignment issues.

The <meta charset="utf-8"> declaration should occur as early as possible, preferably immediately after the opening <head> tag, because the charset affects how all text in the document is interpreted. It would be better not to make the browser re-parse the <title>.

The <img> tag must have an alt attribute. The omission of the alt attribute is unacceptable here, especially since the image contains text. It is odd that the logo has a height attribute but no width.

HTML semantics

The <br> inside the <h1> is not appropriate, since it is there for styling. I would go with a width rule in CSS, using em as the measurement unit so that it scales with the font size.

Writing class="nav" in every navigation link, though harmless, seems excessive. These elements could be selected in CSS using nav > a instead.

It's unclear which of the <nav> links, if any, takes you back to the "home" page.

CSS

The * selector is excessive. If you want to set a font for the page, a body rule should suffice.

In #PageHeader, you set a background-color. It would be a good idea to also set the color at the same time. A comment saying /* white text on orange */ would be nice. You should specify color: white on the links too, in such a way that applies even to visited links.

When you float each nav link to the right, the visual order ends up backwards relative to the DOM order. I would rather float the entire <nav>. It would be a good idea to specify a margin-left on the nav so that it will not overlap the logo if the page is too narrow.

body {
  font-family: Open Sans;
  margin: 0px
}

#PageHeader {
  background-color: rgb(236, 100, 62);
  color: white;
  height: 615px;
}
/* logo and nav look aligned on the original file :c */
#logo {
  position: relative;
  margin-left: 40px;
  top: 30px;
}

nav {
  float: right;
  margin-left: 200px;
}

nav > a {
  text-decoration: none;
  font-size: 12px;
  font-weight: bold;
  text-transform: uppercase;
  margin-right: 40px;
  color: white;
}

nav > a:hover {
  border-width: 0px 0px 3px 0px;
  border-style: solid;
}

#heading {
  color: white;
  text-align: center;
  line-height: 1.2;
  margin-top: 150px;
}

#heading h1 {
  font-size: 60px;
  margin-bottom: 30px;
  margin-left: auto;
  margin-right: auto;
  width: 15em;
}
<html lang="pt-br">
  <head>
    <title>Empreendi na Rede</title>
    <meta charset="utf-8">
    <meta name="author" content="Luciano Infanti">

    <link href="https://fonts.googleapis.com/css?family=Open+Sans:400,400i,800" rel="stylesheet">
  </head>

  <body>
    <header id="PageHeader">
      <div id="logo">
        <img src="https://s31.postimg.org/4esz43xaj/empreendipngwhite.png" height="35">
      </div>
      <nav>
        <a href="#">Solucoes</a>
        <a href="#">Metodologia</a>
        <a href="#">Sobre nos</a>
        <a href="#">Depoimentos</a>
        <a href="#">Clientes</a>
        <a href="#">Contato</a>
      </nav>

      <div id="heading">
        <h1>A consultoria empresarial acaba de ser redefinida</h1>
        <p id="kicker">Aliquam et placerat lacus. Donec pharetra nibh a placerat tincidunt.<br>
  Integer viverra nisl dolor, in viverra enim sollicitudin nec.</p>
      </div>
    </header>
  </body>
</html>

|improve this answer
\$\endgroup\$
1
\$\begingroup\$

Just passing by, most of the modifications have been highlighted by other reviewers but there are few things I will like to mention here -

  1. You can improve on the css for the h1 tag. When specifying the font-size use em rather pixels.

    “Em” is a scalable unit that is used in web document media. An em is equal to the current font-size, for instance, if the font-size of the document is 12pt, 1em is equal to 12pt. The advantage of using em's in web documents are due to scalability and their mobile-device-friendly nature.

    Pixels are fixed-size units that are used in screen media (i.e. to be read on the computer screen). Although, it is popularly used because it produces a pixel-perfect representation of most websites as it is rendered in the browser. the disadvantage of this is that pixel unit is that it does not scale upward for visually-impaired readers or downward to fit mobile devices.

This are things you want to take into consideration

PS: this nice explanation was gotten from CSS Font Size

I have copied the default css for h1 tag from w3schools H1 element is

h1 {
    display: block;
    font-size: 2em;
    margin-top: 0.67em;
    margin-bottom: 0.67em;
    margin-left: 0;
    margin-right: 0;
    font-weight: bold;
}

can be extended to 

    h1 {
            font-size: 3.75em;           
            margin-bottom: 1.875em;        
        }

based on the conversion 1em gives 16px , so 60px is 3.75 em.

  1. Using the asterisk for specifying a font-family for the document is not the best way to go champ!! be selective
p {
  font-family: Open Sans;
  margin: 0px
}

except you want all the fonts in your webpage to look like this.

  1. When using a Google font, best practice is to always have a fallback font in case your browser can not interpret it . For instance 
p{font-family:"Open Sans", Times, serif;}
  1. Please refrain from using names like "kicker" for an id. In general it's best to have a more meaningful name
  2. Media Queries : It's sad that your beautiful website lacks media queries. The catch is when the dimension reduces your header is messed up. It will be nice to have them in your code.
|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Thank you for your help! I just barley started learning HTML/CSS but I promise I'll do my best to apply what you guys are teaching me! If you don't mind, I have a few questions: 1. I heard about em but I wasn't really sure of how to use it. I'll read your link and learn about it 'cause it seems to be super awesome! 2. I do want all my fonts to be Open Sans. I thought using * would be a smart move 'cause that would require less coding. 3. Thanks for the advice! 4. What if the meaningful name is a big one? Would that be a problem? 5. I still have to learn Media Queries. \$\endgroup\$ – Mr. Uselessrobot Jul 21 '16 at 14:29
0
\$\begingroup\$

Few best practices

  • Don't use css ID to define style-sheets rather have classes. ID's are used to locate elements
  • use css pre-parsers like Sass/Less that can give you control in writing css in efficient manner

  • organise your code / css into multiple files for e.g have a global.css & define all your global classes in that file & have module specific css in respective file

  • Better go for Responsive web design using mobile-first approach. Adapt any framework like bootstrap/foundation to accomplish.

  • Use HTML5 semantics which you seems to be followed

  • provide alternative ( alt ) tag for images. see if you can use it as a background image by defining a class.

  • For navigation , use list structure ( ul & li )

Hope this helps to improve your code upto some extent.

|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer. If you don't mind I would like to ask a few questions about what you just said. * You said I should use classes instead of ID's, right? But shouldn't I be using ID's to apply very specific style-sheets? Or I got that part wrong? * I just started learning HTML/CSS. Should I start learning Sass/Less? Or do you think it would be better to master those two first? * I tried to do so but I failed. I do not have a Mac so I can't use Jekyll to do that and I have no idea how to do that on Windows (specially 'cause I barley know HTML/CSS) \$\endgroup\$ – Mr. Uselessrobot Jul 21 '16 at 4:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.