2
\$\begingroup\$

I'm new to programming and web design, and I'm looking for ways to improve my code. What are some things I do well and things I do poorly? How could I optimize my code in any way?

/* Global -------------------------- */

body {
    font: 1.5rem/1.5 Lato, Arial, Helvetica, Sans-serif;
    background: #e3e3e3;
    color: rgb(71, 71, 71);
    text-align: center;
}

.content-wrapper {
    width: 95%;
    margin: auto;
}


/* Header -------------------------- */

.logo {
    font-weight: bold;
}

.info h3 {
    display: inline-block;
}

.navigation-wrapper,
.navbar-width {
    width: 95%;
    margin: auto;
}

header {
    padding-bottom: .5rem;
}

header nav a {
    display: block;
    background: rgb(184, 184, 184);
    margin-bottom: .5rem;
    color: white;
    text-shadow: 0 0 8px #000;
}

header nav a:hover,
#active {
    background: rgb(211, 211, 211);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}

.info a img {
    padding-left: .5rem;
}

.info a img:hover {
    -webkit-filter: brightness(130%);
    filter: brightness(130%);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}


/* Footer ------------------------ */

.footer-grid {
    display: -ms-grid;
    display: grid;
    -ms-grid-columns: (1fr)[3];
    grid-template-columns: repeat(3, 1fr);
}

footer {
    background: rgb(34, 34, 34);
    color: white;
    font-size: 1.2rem;
    padding: .3rem;
    margin-top: 1rem;
}

footer .row-a {
    vertical-align: middle;
    line-height: 80px;
}

footer .row-b a {
    color: white;
    background: transparent !important;
}

footer .row-b a:hover,
#foot-active {
    color: rgb(136, 136, 136);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}

footer .row-c a:hover {
    -webkit-filter: brightness(120%);
    filter: brightness(120%);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}


/* Landing Image -------------------------- */

.landing-image {
    background: url('https://images.pexels.com/photos/91217/pexels-photo-91217.jpeg?w=1260&h=750&auto=compress&cs=tinysrgb') no-repeat center;
    background-size: 100% 100%;
    min-height: 15rem;
}

.landing-text {
    padding-top: 6.5rem;
}

.landing-text h3 {
    background: rgba(255, 255, 255, 0.2);
    color: white;
    text-shadow: 0 0 8px #000;
}


/* Main Content -------------------------- */

.image-a {
    background: url('https://images.pexels.com/photos/434252/pexels-photo-434252.jpeg?w=1260&h=750&auto=compress&cs=tinysrgb') no-repeat center;
    background-size: 100% 100%;
    height: 200px;
    width: auto;
}

.image-b {
    background: url('https://images.pexels.com/photos/3018/cup-mug-water-tea.jpg?w=1260&h=750&auto=compress&cs=tinysrgb') no-repeat center;
    background-size: 100% 100%;
    height: 200px;
    width: auto;
}

.image-a:hover,
.image-b:hover {
    opacity: 0.6;
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}

.main-content h2 {
    background: rgba(0, 0, 0, 0.2);
    color: white;
    text-shadow: 0 0 8px #000;
    text-transform: uppercase;
    margin: 1rem 0;
}

.main-content p {
    text-align: left;
    margin-left: .5rem;
}


/* Clearfix ------------------ */

.clearfix::after {
    content: "";
    clear: both;
    display: table;
}

/* Media Queries --------------------------- */

@media (min-width: 700px) {
    /* Header ---------------------- */
    .nav-grid {
        display: -ms-grid;
        display: grid;
        -ms-grid-columns: 1fr 1fr;
        grid-template-columns: 1fr 1fr;
    }
    .info {
        padding-top: 1rem;
    }
    /* Navigation Bar -------------------- */
    header nav li {
        float: left;
    }
    header nav a {
        padding: .5rem 1rem;
        margin: 0;
    }
    header nav {
        display: -webkit-box;
        display: -ms-flexbox;
        display: flex;
        -webkit-box-pack: center;
        -ms-flex-pack: center;
        justify-content: center;
    }
    .navbar-bg {
        background: rgb(184, 184, 184);
    }
    .navbar-width {
        width: 100%;
    }
    /* Main Content --------------------- */
    .main-content img {
        height: 400px;
    }
    .content-grid {
        display: -ms-grid;
        display: grid;
        -ms-grid-columns: 1fr 1fr;
        grid-template-columns: 1fr 1fr;
        grid-template-areas: "part-a" "part-b" "image-a" "image-b";
        grid-gap: 1rem;
    }
    .part-a {
        -ms-grid-row: 1;
        -ms-grid-column: 1;
        grid-area: part-a;
        -ms-grid-column: 1;
        -ms-grid-column-span: 1;
        grid-column: 1/2;
        -ms-grid-row: 1;
        grid-row: 1;
    }
    .part-b {
        -ms-grid-row: 2;
        -ms-grid-column: 1;
        grid-area: part-b;
        -ms-grid-column: 2;
        -ms-grid-column-span: 2;
        grid-column: 2/4;
        -ms-grid-row: 1;
        grid-row: 1;
    }
    .image-a {
        -ms-grid-row: 3;
        -ms-grid-column: 1;
        grid-area: image-a;
        -ms-grid-column: 1;
        -ms-grid-column-span: 1;
        grid-column: 1/2;
        -ms-grid-row: 2;
        grid-row: 2;
    }
    .image-b {
        -ms-grid-row: 4;
        -ms-grid-column: 1;
        grid-area: image-b;
        -ms-grid-column: 2;
        -ms-grid-column-span: 2;
        grid-column: 2/4;
        -ms-grid-row: 2;
        grid-row: 2;
    }
}

@media (min-width: 1000px) {
    .navigation-wrapper {
        width: 70%;
    }
    .content-wrapper {
        width: 75%;
    }
}

@media (min-width: 1000px) {
    .navigation-wrapper {
        width: 75%;
    }
    .content-wrapper {
        width: 65%;
    }
}


/**
   * Remove list styling.
   */

ul {
    padding: 0;
    margin: 0;
    list-style-type: none;
}


/* Remove padding & margin from text elements. */

h1,
h2,
h3,
h4,
h5,
h6,
p,
a {
    padding: 0;
    margin: 0;
}


/* Remove underline from links */

a {
    text-decoration: none;
}


/* Box sizing */

*,
*:before,
*:after {
    -moz-box-sizing: border-box;
    -webkit-box-sizing: border-box;
    box-sizing: border-box;
}
<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Louis Young | Site 3</title>
    <link rel="icon" href="./img/favicon.ico" type="image/x-icon" />
    <link rel="shortcut icon" href="./img/favicon.ico" type="image/x-icon" />
    <link rel="stylesheet" type="text/css" href="http://fonts.googleapis.com/css?family=Lato">
    <link rel="stylesheet" href="./css/normalize.css">
    <link rel="stylesheet" href="./css/style.css">
    <link rel="stylesheet" href="./css/mediaquery.css">
</head>

<body>

    <!-- Header & Navigation ------------------------------------------------ -->

    <header>

        <div class="navigation-wrapper clearfix">

            <div class="nav-grid">
                <div class="logo">
                    <h1>LOGO HERE</h1>
                </div>
                <div class="info">
                    <h3>Tel: 01223 123 456</h3>
                    <a href="#"><img src="./img/facebook.png" alt="Facebook"></a>
                    <a href="#"><img src="./img/twitter.png" alt="Twitter"></a>
                </div>
            </div>
        </div>
        <div class="navbar-bg navbar-width">
            <nav>
                <ul>
                    <li><a href="../">Home</a></li>
                    <li><a id="active" href="#">Site 3</a></li>
                    <li><a href="#">Contact</a></li>
                </ul>
            </nav>
        </div>

    </header>

    <!-- Main Content Start ------------------------------------------------ -->

    <main>

        <!-- Landing Image ------------------------------------------------ -->

        <section>
            <div class="landing-image">
                <div class="landing-text">
                    <h3>Lorem ipsum dolor sit</h3>
                </div>
            </div>
        </section>

        <!-- Main Content ------------------------------------------------ -->

        <section>
            <div class="main-content content-wrapper">
                <div class="content-grid">
                    <div class="part-a">
                        <h2>Headings</h2>
                        <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Consequatur voluptatem labore delectus, distinctio excepturi aliquid facere ipsum dolore laboriosam, esse eos sunt, quibusdam doloremque quod voluptas. Alias error nobis
                            a?
                        </p>
                    </div>

                    <div class="image-a">

                    </div>

                    <div class="part-b">
                        <h2>Headings</h2>
                        <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Consequatur voluptatem labore delectus, distinctio excepturi aliquid facere ipsum dolore laboriosam, esse eos sunt, quibusdam doloremque quod voluptas. Alias error nobis
                            a?
                        </p>
                    </div>

                    <div class="image-b">

                    </div>

                </div>
            </div>
        </section>

    </main>

    <!-- Footer ------------------------------------------------ -->

    <footer>
        <div class="content-wrapper">
            <div class="footer-grid">

                <div class="row-a">
                    <h3>LOGO</h3>
                </div>

                <div class="row-b">
                    <nav>
                        <ul>
                            <li><a id="foot-active" href="#">Home</a></li>
                            <li><a href="#">About</a></li>
                            <li><a href="#">Contact</a></li>
                        </ul>
                    </nav>
                </div>

                <div class="row-c">
                    <h3>Connect</h3>
                    <a href="#"><img src="./img/facebook.png" alt="Facebook"></a>
                    <a href="#"><img src="./img/twitter.png" alt="Twitter"></a>
                    <a href="#"><img src="./img/instagram.png" alt="Instagram"></a>
                </div>

            </div>
        </div>
    </footer>

</body>

</html>

NOTE: I have 3 stylesheets. One for my styling, another for media queries and a normalize so there may be some differences in the example.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Question

What are some things I do well

The styles have good separation - e.g. header, footer, etc. The selectors also make good use of ids and class names, where appropriate. Also, the media queries make the layout responsive, which is great.

... and things I do poorly? I wouldn't say there are things that are done poorly but some things could be improved. See the next section.

Feedback

important rule

I see one instance of an important rule:

footer .row-b a {
    color: white;
    background: transparent !important;
}

It is bad practice1 (see also the section under heading Why CSS !important Should Be Used Carefully on this post for a more detailed explanation) so if possible, try to find a selector has higher specificity and remove the !important.

Repeat styles

There are a few selectors that have varying styles and some repition:

header nav a:hover,
#active {
    background: rgb(211, 211, 211);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}
.info a img:hover {
    -webkit-filter: brightness(130%);
    filter: brightness(130%);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}

One could take the common styles (i.e. for transitions) out and put them in a separate block:

header nav a:hover,
#active {
    background: rgb(211, 211, 211);
}
header nav a:hover,
#active,
.info a img:hover {
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}
.info a img:hover {
    -webkit-filter: brightness(130%);
    filter: brightness(130%);
    -webkit-transition: 0.2s;
    -o-transition: 0.2s;
    transition: 0.2s;
}

But I know that if the transitions for one set of selectors needs to be changed then those changes would need to be reversed. The same is true for the image styles for .image-a and .image-b (except the background: url() style).

Similarly, I was initially thinking of suggesting that the styles for .content-wrapper (i.e. width: 95%; margin: auto;) be combined into one block for the nearly identical block for .navigation-wrapper, .navbar-width but I see you have those in separate sections.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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