2
\$\begingroup\$

Since I'm new to HTML and CSS, I was wondering whether my HTML layout is correct? Before I used div id for everything and everything seemed to work fine, however I recently heard about the use of span and div classes, so I've attempted to make use of them on a page from my website.

The issue is I have no idea if I've used div class and span in the right way on my site. I'm really quite confused. I've tried to ask questions about it before, but I can't understand the explanation. I've also tried reading up on them, but I'm still not sure on the difference. I don't understand what in-line or block-line in context to span and id means. I really just want to know if my code structure is technically correct and efficient. I mean it works, but I want to ensure I'm using proper practices.

Here is an portion of code from my HTML:

<!--GLOBAL HTML-->

<!--Code marked with GREEN is unused code that may be used later on-->

<!DOCTYPE html>

<html>

<!--CSS LINKS-->

<link href="css/styles.css" rel="stylesheet" type="text/css" media="screen">
<link href="css/index.css" rel="stylesheet" type="text/css" media="screen">

<!--TAB TITLE-->

<head>
    <meta name="author" content="GR412">
    <title>CRAFT412 - Home</title>
</head>

<!--BODY-->

<body>

<!--TOP BANNER-->

<div class="banner">

                  <div id="banner-image">
                                       <img src="images/SiteBackground.png" width="100%" height="143.5" alt="SiteBackground">
                                   </div> 

                  <div id="logo"> 
                               <img src="images/CRAFT412 - Logo.png" width="500" height="281" alt="CRAFT412">
                           </div>

                  <div id="ip-box">
                                 <span id="ip-text">SERVER IP 
                                  craft412.serveminecraft.net</span>
                            </div>

                  <div id="teamspeak-box">
                                        <a href="ts3server://craft412.serveminecraft.net:9987">
                                        <span id="teamspeak-box_2"></span>
                                        <span id="teamspeak-text">TEAMSPEAK</span>
                                        <img  id="teamspeak-image" src="images/CRAFT412 - Box - Teamspeak.png" alt="TEAMSPEAK">
                                        </a>
                                    </div>

                  <div id="red-bar"></div>

</div>

<!--WRAPPER-->

<div class="wrapper">

                   <!--TOP NAV BAR-->

                   <div class="nav-bar">
                                      <ul>
                                        <li><a href="index.html" class="active-page">Home</a>
                                        </li
                                        ><li><a href="status.html">Status</a>
                                        </li
                                        ><li><a href="info.html">Info</a>
                                        </li
                                        ><li><a href="">Gamemodes</a> 
                                      <ul>
                                        <li><a href="survival.html">Survival</a>
                                        </li
                                        ><li><a href="pure-pvp.html">Pure-PVP</a>
                                        </li
                                        ><li><a href="gamesworld.html">Gamesworld</a>
                                        </li>
                                     </ul>
                                        </li
                                        ><li><a href="rules.html">Rules</a>
                                        </li
                                        ><li><a href="vote.html">Vote</a>
                                        </li
                                        ><li><a href="contact.html">Contact</a>
                                        </li>
                                     </ul>
                                 </div>

                   <!--INDEX CONTENT-->

                   <div id="index-banner">
                                        <img class="bottom" src="images/SPAWN IMAGE.png" height="360" alt="INDEX BANNER">
                                        <img class="top" src="images/SURVIVAL IMAGE - GAMEMODES.png" height="360" alt="INDEX BANNER 2">
                                    </div>

                   <div id="welcome-text">                                        
                                         <h3>Welcome to CRAFT412</h3>
                                         <h3>We are currently running version 1.8.1</h3>
                                         <h3>Survival / PurePVP / GamesWorld</h3>                                       
                                     </div>

                   <div id="trailer-title">                                   
                                         <h4><br>SERVER TRAILER</h4>
                                     </div>

                   <div id="trailer-video">
                                         <iframe width="832" height="468" frameborder="0" src="http://www.youtube.com/embed/dfbWw757Iow"></iframe>
                                     </div>
                                 </div>                                                                                                                            

                   <!--GLOBAL HTML-->

                   <!--BOTTOM FOOTER SECTION-->

                   <div class="bottom-footer">                    
                                          <span id="created-by">
                                                             <p>© 2015 GR412</p>
                                                         </span>

                                          <span id="social-media-youtube"> 
                                                                       <a href="https://www.youtube.com/user/GR412" target="_blank">
                                                                       <img src="images/CRAFT412 - YouTube Button.png" width="40" height="40" alt="YOUTUBE"></a>
                                                                   </span>

                                          <span id="social-media-twitch"> 
                                                                      <a href="http://www.twitch.tv/gr412" target="_blank">
                                                                      <img src="images/CRAFT412 - Twitch Button.png" width="35" height="35" alt="TWITCH"></a>
                                                                  </span>

                                          <span id="social-media-twitter"> 
                                                                       <a href="https://twitter.com/GR412" target="_blank">
                                                                       <img src="images/CRAFT412 - Twitter Button.png" width="55" height="55" alt="TWITTER"></a>
                                                                   </span>

                                          <span id="social-media-facebook"> 
                                                                        <a href="https://www.facebook.com/GR412" target="_blank">
                                                                        <img src="images/CRAFT412 - Facebook Button.png" width="45" height="40" alt="FACEBOOOK"></a>
                                                                   </span>


</div>

</body>

</html>
\$\endgroup\$
  • 8
    \$\begingroup\$ You've learned literally nothing from the last review you posted. Here you are with almost all of the same mistakes you made in the original version of this code. \$\endgroup\$ – cimmanon Aug 14 '15 at 23:07
  • 1
    \$\begingroup\$ This question is being discussed on Meta. \$\endgroup\$ – 200_success Aug 15 '15 at 5:39
  • 1
    \$\begingroup\$ @cimmanon I've updated the code as I've been reading a bit about it. Is this any better? It seems far more efficient in my opinion I just don't know if it's still properly implemented. \$\endgroup\$ – user56811 Aug 18 '15 at 14:12
17
\$\begingroup\$

Your root question is:

The issue is I have no idea if I've used div class and span in the right way on my site.

Technically, it's not very hard to use them wrong, though there are a few principles to follow:

  1. div tags are expected to be block-level elements. That is, they take up 100% of the width of their parent, by default. (Or whatever the browser is configured for.)
  2. span tags are the opposite: they are inline elements. That is, they only take up enough space to fit their content, by default. They should never include other block-level elements within them.

There are certain, "acceptable" use differences between the two. For example, span elements should never contain any block-level elements. It's considered good practice to use span tags around bits of text that need special styles. It's considered bad practice to use them as block-level containers. You should also only use div elements when you need a true container, as that is what div elements are meant for.

A div is a "division" in the page. That is, this next bit if content should be significantly different from the previous content. A span is merely intended to allow for the grouping of bits of text.

Inline elements are similar to the inline code-blocks you see above. The style is only applied to fit whatever content is within that element. So, with a <span> tag, it will resize itself to fit the content within it.

Block-level elements are similar to the quote you see above. They will stretch to fit the entire width of the parent container, unless otherwise specified. Think of them as a rectangle. They will always be a rectangle-shaped element, whereas inline elements will wrap to their content.


On to the Review

Your HTML is not at all valid. If this is a snippet, then that is expected, but I would recommend to take use of the tools out there to get automated reviews of whether or not the HTML is legal.

The W3C Validator does a great job at that.

Some points to take from that validator:

  1. Your <head> tag opens in the wrong spot. You have two <link> tags that appear before it, which is very illegal to the spec. You can fix it as so:

    <head>
        <!--CSS LINKS-->
    
        <link href="css/styles.css" rel="stylesheet" type="text/css" media="screen">
        <link href="css/index.css" rel="stylesheet" type="text/css" media="screen">
    
        <!--TAB TITLE-->
    
        <meta name="author" content="GR412">
        <title>CRAFT412 - Home</title>
    </head>
    
  2. Don't use percentages or decimals or negatives for img width or height attributes. Those are considered illegal. If the image needs to be 100% width, then you should apply an appropriate class to it. The width and height attributes on img tags are designed so that browsers can determine how an image will affect page-rendering, before it has been loaded.

  3. For img and a tag href attributes, you should encode the URL when necessary. I.e. use %20 for spaces, especially.

  4. frameborder attributes on iFrame tags are obsolete - CSS can replace this. Of course, iFrame tags alone can be considered bad-practice, avoid them if you can.

  5. Back to div vs. span: the following code is illegal by the specification:

    <span id="created-by">
        <p>© 2015 GR412</p>
    </span>
    

    Instead, the span should be a div, or the <p> and </p> should be removed. Acceptable tags within a span include a, img, other span tags, and several others. (A web-search can get you a more complete list.)

Those are the biggest kickers I see, though you should certainly fix your whitespace, and minify it where you can. Since HTML is sent as-is to the browser (unless the web-server does the minification/compression) it's important not to bloat it.


You should also look into the <section> tag, and the <nav> tag. Your <!DOCTYPE html> line indicates that it is to be HTML5 compliant, so it is acceptable to use those tags here.


As far as id and class go, you should never repeat an id value more than once on any page. If you need to include multiple elements that use the same CSS rules, always apply those rules to the class, and always apply that class to your <div>, <span>, <p>, <a>, <img>, <section>, <nav>, or whatever other tag you have. You can attach an id or class to practically any tag. So don't let those restrict you to only using div or span.

\$\endgroup\$
  • 2
    \$\begingroup\$ Well I'm actually so happy I've finally got a question that hasn't been flagged down. However it's clear to me I'm not smart enough to understand this. I thought I was good at it but evidently I haven't got a clue. I appreciate your answer and will give you the credit you deserve. \$\endgroup\$ – user56811 Aug 14 '15 at 21:28
  • \$\begingroup\$ @GR412 I assume you are self-taught, which is very common in the web world, so don't feel bad that you are unaware of some of these concepts. Most of this is something you pick up after being in the field for an extended time. I would recommend you study up on HTML and CSS, and bring that which you would like us to review (as I have shown) to this site. Do remember, though, Code Review is only for working code that you want a review on. Reviewers are permitted to critique each and every aspect of it. :) \$\endgroup\$ – 410_Gone Aug 14 '15 at 22:04
  • \$\begingroup\$ Yeah I need to do some research are there any sources you recommend for learning this sort of stuff? \$\endgroup\$ – user56811 Aug 14 '15 at 22:31
  • 4
    \$\begingroup\$ @GR412 Unfortunately, such requests are off-topic for Code Review. If you have made significant changes, you can post a new question as a follow-up. Code Review is not the place to ask questions about issues that "don't work" or "need fixing", or to get guided help, but instead for review of code that is actually working as desired. If you want help changing the output/rendering of the HTML to meet certain requirements, you should post those on Stack Overflow. \$\endgroup\$ – 410_Gone Aug 18 '15 at 14:57
  • 1
    \$\begingroup\$ @GR412 You should read the following Meta post for that information: meta.codereview.stackexchange.com/questions/1065/… \$\endgroup\$ – 410_Gone Aug 18 '15 at 21:20
11
\$\begingroup\$
<span id="social-media-facebook"> 
    <a href="https://www.facebook.com/GR412" target="_blank">
        <img src="images/CRAFT412 - Facebook Button.png" width="45" height="40" alt="FACEBOOOK"></a>
</span>

including the width and height in HTML tags not good, HTML is about content, while CSS makes it look nice and pleasing. Mixing the two leads to difficulties down the line.

As a small note, if you want an image to be a certain size, resize it yourself, and upload the resized picture. This means you don't have to adjust it with code. It also means that every time the page is opened, the image doesn't have to be resized. Its a win-win for both you and the users.


It is up to you, but if I was designing the social media footer, I would use one nav tag, and just include all the link and image tags inside an unordered list, like the nav-bar. It means you can style all the buttons the same, without changing multiple lines of code in your CSS file.


© = &copy;
A space = %20, taking all the spaces out of links, including to images is a good thing

and are used without or , if you are using them for the size of the text, this is easily fixed with CSS.

There is also a break-line tag inside the for some reason? Again, style with CSS, not with whitespace.


Presumably, from the many links you have many pages in the site, changing the header and footer on each page, every time there is a small change gets tedious, and a waste of time. A server side script like PHP would be able to fix this problem, or if you really want, you could encode the headers and footers in javascript and have to wade through that code for ever small change.

I think doing it server side would be much smarter in the long run, and you can then learn how to make other features like a login, or a contact us page that sends emails

\$\endgroup\$
  • \$\begingroup\$ "including the width and height in HTML tags not good" <- When it comes to images, you're dead wrong (see: stackoverflow.com/questions/1247685/…). \$\endgroup\$ – cimmanon Aug 14 '15 at 22:47
  • \$\begingroup\$ the page you linked states clearly that it should be included in either HTML or CSS, and I definitely favour it in the CSS. If I said remove the width and height completely from all the code, I would agree with you. \$\endgroup\$ – spyr03 Aug 14 '15 at 23:22
  • \$\begingroup\$ And what happens when the loading of the CSS is delayed or disabled? You think reflow doesn't happen in a document with no styling? If the images are resized, you have to modify the code either way: whether it is in the markup or the CSS makes no difference. \$\endgroup\$ – cimmanon Aug 15 '15 at 12:06
  • \$\begingroup\$ I can't think of something that would cause reflow in a document with no styling or javascript, when would it happen? If it makes no difference, I'd definitely prefer it in the CSS, as then it is with other similar commands, as opposed to on its own in HTML \$\endgroup\$ – spyr03 Aug 15 '15 at 14:07
10
\$\begingroup\$

First thing that you need to do is take care of your indentation so that it is easier to read.

<!--GLOBAL HTML-->

<!--Code marked with GREEN is unused code that may be used later on-->

<!DOCTYPE html>

<html>

    <!--CSS LINKS-->

    <link href="css/styles.css" rel="stylesheet" type="text/css" media="screen">
    <link href="css/index.css" rel="stylesheet" type="text/css" media="screen">

    <!--TAB TITLE-->

    <head>
        <meta name="author" content="GR412">
        <title>CRAFT412 - Home</title>
    </head>

    <!--BODY-->

    <body>

        <!--TOP BANNER-->

        <div class="banner">
            <div id="banner-image">
                <img src="images/SiteBackground.png" width="100%" height="143.5" alt="SiteBackground">
            </div> 

            <div id="logo"> 
                <img src="images/CRAFT412 - Logo.png" width="500" height="281" alt="CRAFT412">
            </div>

            <div id="ip-box">
                <span id="ip-text">SERVER IP 
                craft412.serveminecraft.net</span>
            </div>

            <div id="teamspeak-box">
                <a href="ts3server://craft412.serveminecraft.net:9987">
                    <span id="teamspeak-box_2"></span>
                    <span id="teamspeak-text">TEAMSPEAK</span>
                    <img  id="teamspeak-image" src="images/CRAFT412 - Box - Teamspeak.png" alt="TEAMSPEAK">
                </a>
            </div>

            <div id="red-bar"></div>
        </div>

        <!--WRAPPER-->

        <div class="wrapper">

            <!--TOP NAV BAR-->

            <div class="nav-bar">
                <ul>
                    <li>
                        <a href="index.html" class="active-page">Home</a>
                    </li>
                    <li>
                    <a href="status.html">Status</a>
                    </li>
                    <li>
                        <a href="info.html">Info</a>
                    </li>
                    <li>
                        <a href="">Gamemodes</a> 
                        <ul>
                            <li>
                                <a href="survival.html">Survival</a>
                            </li>
                            <li>
                                <a href="pure-pvp.html">Pure-PVP</a>
                            </li>
                            <li>
                                <a href="gamesworld.html">Gamesworld</a>
                            </li>
                        </ul>
                    </li>
                    <li>
                        <a href="rules.html">Rules</a>
                    </li>
                    <li>
                        <a href="vote.html">Vote</a>
                    </li>
                    <li>
                        <a href="contact.html">Contact</a>
                    </li>
                </ul>
            </div>

            <!--INDEX CONTENT-->

            <div id="index-banner">
                <img class="bottom" src="images/SPAWN IMAGE.png" height="360" alt="INDEX BANNER">
                <img class="top" src="images/SURVIVAL IMAGE - GAMEMODES.png" height="360" alt="INDEX BANNER 2">
            </div>

            <div id="welcome-text">                                        
                <h3>Welcome to CRAFT412</h3>
                <h3>We are currently running version 1.8.1</h3>
                <h3>Survival / PurePVP / GamesWorld</h3>                                       
            </div>

            <div id="trailer-title">                                   
                <h4><br>SERVER TRAILER</h4>
            </div>

            <div id="trailer-video">
                <iframe width="832" height="468" frameborder="0" src="http://www.youtube.com/embed/dfbWw757Iow"></iframe>
            </div>
        </div>                                                                                                                            

        <!--GLOBAL HTML-->
        <!--BOTTOM FOOTER SECTION-->

        <div class="bottom-footer">                    
            <span id="created-by">
                <p>© 2015 GR412</p>
            </span>
            <span id="social-media-youtube"> 
                <a href="https://www.youtube.com/user/GR412" target="_blank">
                    <img src="images/CRAFT412 - YouTube Button.png" width="40" height="40" alt="YOUTUBE">
                </a>
            </span>
            <span id="social-media-twitch"> 
                <a href="http://www.twitch.tv/gr412" target="_blank">
                    <img src="images/CRAFT412 - Twitch Button.png" width="35" height="35" alt="TWITCH">
                </a>
            </span>

            <span id="social-media-twitter"> 
                <a href="https://twitter.com/GR412" target="_blank">
                    <img src="images/CRAFT412 - Twitter Button.png" width="55" height="55" alt="TWITTER">
                </a>
            </span>

            <span id="social-media-facebook"> 
                <a href="https://www.facebook.com/GR412" target="_blank">
                    <img src="images/CRAFT412 - Facebook Button.png" width="45" height="40" alt="FACEBOOOK">
                </a>
            </span>
        </div>
    </body>

</html>

I haven't done anything except for fix indentation and put most elements on their own line to make your code more readable.


You should never put a break (<br>) tag inside of header (<h4>) tag like this

  <h4><br>SERVER TRAILER</h4>

and while we are at it, you don't need to enclose a header tag in a div tag just to add an ID or a Class to it.

This:

<div id="trailer-title">       
    <h4><br>SERVER TRAILER</h4>
</div>

Becomes this

<h4 id="trailer-title">SERVER TRAILER</h4>

so if you have a single element and want to add a class or an ID to it, you don't need to enclose it inside of a div


With this little bit of code:

<span id="created-by">
    <p>© 2015 GR412</p>
</span>

I disagree with @EBrown. I agree that you shouldn't have a paragraph tag (<p>) inside of a span (<span>) it's not correct in any HTML language.

But, the paragraph tag can stand on it's own here, you were only using the span tag to add a style hook, the ID attribute, to the paragraph tag which is incorrect usage. Add the ID directly to the paragraph tag like this

<p id="created-by">© 2015 GR412</p>

You actually make this mistake through out the HTML.

In the footer:

<span id="social-media-youtube"> 
    <a href="https://www.youtube.com/user/GR412" target="_blank">
        <img src="images/CRAFT412 - YouTube Button.png" width="40" height="40" alt="YOUTUBE">
    </a>
</span>

The anchor tag (<a>) can stand on it's own without the span tag, so add the style hook attribute (id or class) to the anchor tag.

All tags can be given the style hook attributes id or class except <html> and <body>. you can actually add attributes to these elements but it isn't usually recommended or actually done in any environment.


It looks like you are using HTML 5, so you can take your navigation HTML

<div class="nav-bar">

and move it all into a menu tag (<menu>) and actually style it using the tag as the styling hook itself, you won't need any attributes on this tag.

And since we are using HTML 5 you can also do the same thing with your header and your footer HTML

Header

<div class="banner">
    <div id="banner-image">
        <img src="images/SiteBackground.png" width="100%" height="143.5" alt="SiteBackground">
    </div> 

    <div id="logo"> 
        <img src="images/CRAFT412 - Logo.png" width="500" height="281" alt="CRAFT412">
    </div>

    <div id="ip-box">
        <span id="ip-text">SERVER IP 
        craft412.serveminecraft.net</span>
    </div>

    <div id="teamspeak-box">
        <a href="ts3server://craft412.serveminecraft.net:9987">
            <span id="teamspeak-box_2"></span>
            <span id="teamspeak-text">TEAMSPEAK</span>
            <img  id="teamspeak-image" src="images/CRAFT412 - Box - Teamspeak.png" alt="TEAMSPEAK">
        </a>
    </div>

    <div id="red-bar"></div>
</div>

Becomes this, with some other changes as well

<header>
    <img src="images/SiteBackground.png" width="100%" height="143.5" alt="Site Background" id="banner-image">
    <img src="images/CRAFT412 - Logo.png" width="500" height="281" alt="CRAFT412" id="logo">
    <div id="ip-box">
        <span id="ip-text">SERVER IP 
        craft412.serveminecraft.net</span>
    </div>
    <div id="teamspeak-box">
        <a href="ts3server://craft412.serveminecraft.net:9987">
            <span id="teamspeak-box_2"></span>
            <span id="teamspeak-text">TEAMSPEAK</span>
            <img  id="teamspeak-image" src="images/CRAFT412 - Box - Teamspeak.png" alt="TEAMSPEAK">
        </a>
    </div>
    <div id="red-bar"></div>
</header>

Footer

<div class="bottom-footer">                    
    <span id="created-by">
        <p>© 2015 GR412</p>
    </span>
    <span id="social-media-youtube"> 
        <a href="https://www.youtube.com/user/GR412" target="_blank">
            <img src="images/CRAFT412 - YouTube Button.png" width="40" height="40" alt="YOUTUBE">
        </a>
    </span>
    <span id="social-media-twitch"> 
        <a href="http://www.twitch.tv/gr412" target="_blank">
            <img src="images/CRAFT412 - Twitch Button.png" width="35" height="35" alt="TWITCH">
        </a>
    </span>

    <span id="social-media-twitter"> 
        <a href="https://twitter.com/GR412" target="_blank">
            <img src="images/CRAFT412 - Twitter Button.png" width="55" height="55" alt="TWITTER">
        </a>
    </span>

    <span id="social-media-facebook"> 
        <a href="https://www.facebook.com/GR412" target="_blank">
            <img src="images/CRAFT412 - Facebook Button.png" width="45" height="40" alt="FACEBOOOK">
        </a>
    </span>
</div>

Becomes this

<footer>                    
    <p id="created-by">© 2015 GR412</p>
    <span id="social-media-youtube"> 
        <a href="https://www.youtube.com/user/GR412" target="_blank">
            <img src="images/CRAFT412 - YouTube Button.png" width="40" height="40" alt="YOUTUBE">
        </a>
    </span>
    <span id="social-media-twitch"> 
        <a href="http://www.twitch.tv/gr412" target="_blank">
            <img src="images/CRAFT412 - Twitch Button.png" width="35" height="35" alt="TWITCH">
        </a>
    </span>
    <span id="social-media-twitter"> 
        <a href="https://twitter.com/GR412" target="_blank">
            <img src="images/CRAFT412 - Twitter Button.png" width="55" height="55" alt="TWITTER">
        </a>
    </span>
    <span id="social-media-facebook"> 
        <a href="https://www.facebook.com/GR412" target="_blank">
            <img src="images/CRAFT412 - Facebook Button.png" width="45" height="40" alt="FACEBOOOK">
        </a>
    </span>
</footer>
\$\endgroup\$

Your Answer

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