2
\$\begingroup\$

I created this simple responsive layout for my web apps and website projects. I'd like to ask if you see any flaws or bad practices in this code? A working demo can be seen here http://codepen.io/anon/pen/QdavpZ/ Thank you in advance, for any efforts.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Responsive Layout Example</title>
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/5.0.0/normalize.min.css">
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/milligram/1.3.0/milligram.min.css">
</head>
<body>
<style>
    .midori-header * {
        padding: 0;
        margin: 0;
        color: white;
        font-size: 1.6rem;
        line-height: 3rem;
    }

    .midori-header {
        margin: 0;
        padding: 0;
        overflow: hidden;
        background-color: #333;
        color: white;
        position: fixed; /* Set the navbar to fixed position */
        top: 0; /* Position the navbar at the top of the page */
        width: 100%; /* Full width */
        z-index: 999;
    }

    .midori-header > h1 {
        padding: 14px 16px;
        display: inline-block;
        float: left;
        text-decoration: none;
        margin: 0;
    }

    ul.midori-navigation {
        list-style-type: none;
        margin: 0;
        padding: 0;
        overflow: hidden;
    }

    /* Style the links inside the list items */
    ul.midori-navigation li a {
        display: inline-block;
        text-align: center;
        padding: 14px 16px;
        text-decoration: none;
        transition: 0.3s;
        font-size: 17px;
    }

    /* Float the list items side by side */
    ul.midori-navigation li {
        float: left;
        margin: 0;
    }

    ul.midori-navigation.vertical {
        position: relative;
    }

    ul.midori-navigation.vertical li {
        float: none;
        display: inline;
        text-align: left;
    }

    ul.midori-navigation.vertical li a {
        display: block;
        text-align: left;
    }

    .main {
        margin-top: 8rem;
    }

    .content {
        margin-top: 3rem;
    }

    .sidebar {
        height: 75vh;
    }

    @media screen and (min-width: 680px) {
        .midori-header * {
            float: right;
        }
    }

    /* The "responsive" class is added to the topnav with JavaScript when the user clicks on the icon. This class makes the topnav look good on small screens */
    @media screen and (max-width: 680px) {

        .midori-header > h1 {
            float:none;
            width:100%;
            text-align: center;
        }

        .sidebar {
            height: auto;
            text-align: center;
        }

        .main {
            margin-top: 2rem;
        }

        .content {
            margin-top: auto;
        }

        .midori-header {
            position: relative;
        }

        ul.midori-navigation.horizontal {
            position: relative;
        }

        ul.midori-navigation.horizontal li {
            float: none;
            display: inline;
        }

        ul.midori-navigation.horizontal li a {
            display: block;
            text-align: center;
        }

        ul.midori-navigation.vertical li a {
            display: block;
            text-align: center;
        }
    }

</style>
<header class="midori-header">
    <h1>Title</h1>
    <ul class="midori-navigation horizontal">
        <li><a href="#">Link</a></li>
        <li><a href="#">Link</a></li>
        <li><a href="#">Link</a></li>
        <li><a href="#">Link</a></li>
        <li><a href="#">Link</a></li>
    </ul>
</header>
<div class="container main">
    <div class="row">
        <div class="column sidebar">
            <ul class="midori-navigation vertical">
                <li><a href="#">Link</a></li>
                <li><a href="#">Link</a></li>
                <li><a href="#">Link</a></li>
                <li><a href="#">Link</a></li>
                <li><a href="#">Link</a></li>
            </ul>
        </div>
        <div class="column column-75 content">
            <h2>Title 2</h2>
            <p>
                Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et
                dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip
                ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu
                fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia
                deserunt mollit anim id est laborum.
            </p>

        </div>
    </div>
</div>
<footer class="container">
    <div class="row">
        <p>
            Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et
            dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex
            ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat
            nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit
            anim id est laborum.
        </p>
    </div>
</footer>

</body>
</html>
Improve this question
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Don't have time to write up a full review, but thumbs up for making use of the header/footer tags! The amount of code I still see which is just div soup blows my mind. The only thing I'm not a fan of is how your navigation links are all contained within one li - it'd make more sense to have a list item for each link. \$\endgroup\$ – Joe Clay Jan 30 '17 at 10:59
  • 1
    \$\begingroup\$ Ah sorry, that was my mistake. updating it. thank you for comment. \$\endgroup\$ – Midori Kocak Jan 30 '17 at 11:08
  • 1
    \$\begingroup\$ Easy mistake to make :) At this point your HTML looks pretty much how I'd opt to code it - my CSS is a bit rusty though, so I'll let someone else weigh in on whether changes could be made there. \$\endgroup\$ – Joe Clay Jan 30 '17 at 11:21
  • 1
    \$\begingroup\$ < 415 px, it becomes unreadable. \$\endgroup\$ – Cody Gray Jan 30 '17 at 14:48
  • \$\begingroup\$ Should I add a min-width to body? \$\endgroup\$ – Midori Kocak Jan 30 '17 at 18:33
6
\$\begingroup\$

Although the code looks good, it's a good coding practice to separate out the CSS and HTML code. Like for example maintain your file structure as follows

  • index.html

  • CSS

    • someFileName.css

In your index.html file put all your HTML code mentioned above. Put your code between the <style> tags in a .css file. Similarly you could put your JavaScript/.Net files in another folder (in case you use them). Use the below code between your <head> tags.

<link rel="stylesheet" type="text/css" href="CSS/someFileName.css">

Reason : In complex/large websites, the code becomes too messy with CSS styling here and there. Maintaining the site becomes a difficult task. It's therefore a good practice to separate them out. Try debugging some of your favourite sites and check their structures.(open the website and press F12 if you're using Chrome/Firefox. In the Sources tab you'll see the structure to the left. Also go to the Elements tab and check the code. You'll not notice any CSS/JS code within the HTML code. Check out some simple sites like this)

Also check out the codepen.

Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your comment. I am sure that it will be good for readers. I printed the code here like this, because I also was willing to have my CSS code reviewed too. It was for presentation purposes here actually. I totally agree with your comment. \$\endgroup\$ – Midori Kocak Jan 30 '17 at 13:38
  • 4
    \$\begingroup\$ @MidoriKocak We do not have any problems with people posting their HTML, CSS and Javascript in the same question with separate code blocks for each aspect. In fact, we even have special code snippets for such posts that show an example. \$\endgroup\$ – Nzall Jan 30 '17 at 14:33

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.