1
\$\begingroup\$

This is for a client of mine: Enbridge Gas. I had to turn 2 .psd file into html and CSS. This client is my first real client by myself and I want to make sure my code is improved as much as possible. I would love feed back on my style, naming conventions and ect to improve the code.

If you'd like to see the full project files/folder and run it with the images and ect, here is the GitHub link to the project.

Note: IMAGES are of course, not part of the code snippet. This means the drop down list won't work. I commented one image out and put in a grey color. Also, one image at the bottom is not going to show.

For the snippet you have to scroll to the right to see the content:

body {
  padding: 0px;
  margin: 0px;
}
input[type=checkbox] {
  cursor: pointer;
}
.main_content {
  width: 480px;
  padding-left: 618px;
  padding-right: 620px;
}
.top_info {
  width: 477px;
  height: 105px;
  padding-bottom: 20px;
}
.top_info h2 {
  padding: 0px;
  margin: 0px;
  margin-top: 25px;
  margin-bottom: 13px;
  font: 700 13px / 18px Arial;
}
.top_info p {
  margin: 0 0 10px;
  font: 400 13px / 18px Arial;
}
.form {
  width: 479px;
  height: 329px;
  /*background: url(images/Layer-7.png);*/
  background: #bbb;
}
.form_header {
  background: url(images/Layer-11.png);
  background-repeat: no-repeat;
  height: 43px;
  width: 479px;
}
.form_header h2 {
  font: 700 17px / 19px Arial;
  color: #ffb81c;
  padding: 13px 35px 15px 35px;
}
.checkbox {
  font: 400 12px / 19px Arial;
  width: auto;
  color: #f7f7f7;
  padding-top: 11px;
  padding-left: 9px;
}
.left_content {
  float: left;
  width: 255px;
  font: 400 15px / 19px Arial;
  height: 175px;
  color: #f7f7f7;
}
.left_content ul {
  margin: 0px;
  padding-left: 28px;
}
.left_content li {
  padding-left: 3px;
}
.right_content {
  float: right;
  width: 189px;
  height: 175px;
  padding-left: 14px;
  padding-right: 19px;
}
.right_content input {
  width: 189px;
  height: 24px;
  margin-bottom: 6px;
}
.select {
  background: url(images/MergedLayers.png) no-repeat right #f7f7f7;
  overflow: hidden;
  width: 189px;
  height: 24px;
  margin-bottom: 6px;
  border: 0 none;
}
.select select {
  width: 189px;
  height: 24px;
  margin-bottom: 6px;
  background: transparent;
  border: 0 none;
  border-radius: 0;
  -webkit-appearance: none;
  -moz-appearance: none;
  cursor: pointer;
}
.right_content input[type=submit] {
  width: 189px;
  height: 36px;
  background: url(images/Layer-8-copy-3.png);
  border: 0 none;
  font: 700 16.35px / 14.43px Arial;
  padding-top: 11px;
  padding-bottom: 10px;
  padding-left: 7px;
  padding-right: 5px;
  color: #f7f7f7;
  cursor: pointer;
}
.right_content p {
  font: 400 12.95px / 16px Arial;
  color: #f7f7f7;
  padding: 0px;
  padding-top: 5px;
  margin: 0px;
  text-align: center;
}
.form_footer p {
  clear: both;
  font: 700 17px / 20px Arial;
  color: #f7f7f7;
  text-align: center;
  padding-left: 29px;
  padding-right: 31px;
  padding-bottom: 22px;
  padding-top: 10px;
}
.bottom_info h2 {
  margin: 0px;
  padding-top: 24px;
  padding-left: 9px;
  color: #02436b;
  font: 700 15px / 16px Arial;
}
.bottom_image {
  background: url(images/stock-photo-64105997-generic-hospital-building.png);
  height: 211px;
  width: 157px;
  float: left;
  margin: 0;
  padding: 0;
  margin-top: 19px;
  margin-left: 9px;
}
.bottom_right {
  width: 285px;
  float: right;
  height: 198px;
}
.bottom_info p {
  color: #252424;
  font: 700 13px / 16px Arial;
  padding: 0px;
  margin: 0px;
  padding-top: 19px;
}
.bottom_info ul {
  color: #252424;
  font: 400 13px / 16px Arial;
  padding: 0px;
  padding-left: 15px;
  padding-top: 5px;
  margin: 0px;
}
.bottom_info li {
  padding-bottom: 5PX;
  ;
}
<html>

<head>
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  <link rel="stylesheet" href="css/main.css" />
</head>

<body>
  <div class="main_content">

    <div class="top_info">
      <h2>Incentives for every building, every budget</h2>
      <p>Looking for energy saving solutions for your commercial building? You've come to the right place. Whether retrofitting or building new, we offer free services and financial incentives for buildings and budgets of every size. We'll guide the process
        from start to finish, making it easy to reduce energy consumption and improve your bottom line.</p>
    </div>
    <!-- end top_info -->

    <div class="form">

      <div class="form_header">
        <h2>Earn up to 50% of your project cost! Find out how.</h2>
      </div>

      <form action="#" method="get" class="checkbox">
        <input type="checkbox" name="communicate">Check here to allow us to communicate with you
      </form>

      <div class="left_content">
        <ul>
          <li>Energy Solutions Consultant (ESC) will speak with you directly and help you access your energy efficiency needs.</li>
          <li>Fixed and custom incentives to upgrade boilers, water and heating systems, make-up air and ventilation systems, building controls, and more</li>
        </ul>
      </div>

      <div class="right_content">
        <form action="#" method="get">
          <input type="text" placeholder="First and Last Name">
          <input type="text" placeholder="Email Address">
          <div class="select">
            <select name="sector" id="sector">
              <option value="sector">Sector</option>
            </select>
          </div>
          <input type="submit" name="submit" value="Download Case Study">
        </form>
        <p>Your information is safe with us.</p>
      </div>

      <div class="form_footer">
        <p>Contact your Energy Solutions Consultant today at 1-855-659-0549 or energyservices@enbridge.com</p>
      </div>

    </div>
    <!-- end form -->

    <div class="bottom_info">
      <h2>Need a reason to take part in the Commercial Energy Solutions program?</h2>

      <div class="bottom_image"></div>

      <div class="bottom_right">
        <p>Top gains include:</p>
        <ul>
          <li>Retrofit incentives for installing energy-efficient equipment and systems in older buildings</li>
          <li>Rebates for installing energy-efficient showerheads in multi-residential buildings</li>
          <li>Incentives for installing an ozone laundry systems on commercial washing machines</li>
          <li>Rebates for renting or purchasing high-efficiency and condensing boilers</li>
          <li>Free support and energy saving expertise from one of our Energy Solutions Consultants</li>
        </ul>
      </div>

    </div>
    <!-- end bottom_info -->

  </div>
  <!-- end main_content -->
</body>

</html>

\$\endgroup\$
  • \$\begingroup\$ What exactly are you wanting us to review? you only show html and css... \$\endgroup\$ – DelightedD0D Jul 2 '16 at 4:05
  • \$\begingroup\$ You are missing one point: add open graph meta tags. It has a major impact in social platforms. \$\endgroup\$ – vignesh warar Jul 2 '16 at 4:23
  • \$\begingroup\$ I'm pretty sure I stated that in my post :p just your thoughts on style, and general improvement on ANYTHING you may want to tell me about. \$\endgroup\$ – Darcey Mckelvey Jul 2 '16 at 4:25
4
\$\begingroup\$

If there is no specific reason to why you have designed the page to be 480px wide, I would move away from that. It makes it look like a flyer that you can print out. It's all very crammed up and feels kind of like a pop-up ad.

In your code you are doing this, which I assume is to center the main_content div:

.main_content {
  width: 480px;
  padding-left: 618px;
  padding-right: 620px;
}

While in reality it's not centered. You can do this to center it properly:

.main_content {
  width: 480px;
  margin: 0 auto; 
}

(Top and Bottom = 0 | Left and Right = auto)

When the div has an absolute width, you can use "auto" on both sides to give them equal spacing.

Also, when you are setting widths, paddings and margins with pixels, the page may look very different on a smaller or larger resolution screen.

The page on a 1280x600 resolution

Browse the web and look for some inspiration. Steal some ideas, and implement them with your own twist.

\$\endgroup\$
0
\$\begingroup\$
input[type=checkbox] {
  cursor: pointer;
}

is this needed?

background: url(images/Layer-11.png);
background-repeat: no-repeat;

and

  padding-top: 11px;
  padding-bottom: 10px;
  padding-left: 7px;
  padding-right: 5px;

plus others could be written shorthand.

You don't need to specify units when the quantity is 0 (0px)

It's in your PSD too so maybe out of scope but program is a widow. Widows should always be avoided in fixed width designs.

\$\endgroup\$
  • \$\begingroup\$ Greetings, you will get extra points if you actually write the short-hands out for the question poster. \$\endgroup\$ – konijn Jul 22 '16 at 13:40

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.