3
\$\begingroup\$

First time posting in Code Review! I have started learning front-end development to complement my UI design skillset and am starting with CSS Grid and bootstrap.

Have a look at the snippets. I think the 2 work quite well, using CSS Grid for layout and the Bootstrap library for things like buttons, forms and navs etc.

Can you let me know if there is anything I can do to make my code more scalable and more toward production standard?

Thanks!

body{
    margin: 0%;
    padding: 0%;
    background-image: url('../img/background.jpg');
    background-position: center center;
    background-repeat: no-repeat;
    background-attachment: fixed;
    background-size: cover;
    background-color: #464646;
    overflow: hidden;
}

.logo{
    height: 30px;
    margin: 50px;
}

.container-fluid{
    height: 100vh;
    padding: 0;
    margin: 0;
    display: grid;
    grid-template-columns: repeat(2, 1fr);
    grid-template-areas:
        "L R"
}


.left-side{
    grid-area: L;
    height: 100vh;
    margin: 0;
    padding: 0;

}

.right-side{
    grid-area: R;
    background: linear-gradient(-135deg, rgb(255, 0, 0,0.8), rgba(167,0,0,0.8));
    height: 100vh;
    margin-right: 0;
    text-align: center;
}



.right-inner{
    margin: 40% auto;
    max-width: 500px;
    min-width: 200px;
    height: 100vh;
}


.welcome{
    color: #fff;
    text-align: left;
    font-family: "UniversNext", sans-serif;
    font-size: 36px;
    margin-top: 0;
    padding-top: 0;
    margin-bottom: 30px;
}

.cust-name{
    color: #fff;
    font-family: "UniversNext", sans-serif;

}


.log-in{
    background-color: transparent;
    border: 1px solid #fff;
    color: #fff;
    padding: 9px;
    height: 38px;
    width: 100px;
    font-size: 14px;
    font-family: sans-serif;
    float: left;
}


.log-in:hover{
    background-color: #fff;
    border: 1px solid #fff;
    color: #253038;
}

.log-in:active{
    background-color: rgb(221, 221, 221);
    border: 1px solid #fff;
    color: #253038;
}

.log-in:focus{
    background-color: rgb(221, 221, 221);
    border: 1px solid #fff;
    color: #253038;
}


.password{
    margin-bottom: 20px;
}


.form-control{
    min-height: 38px;
}

.col-form-label-sm{
    color: #fff;
    text-align: center;
    font-family: sans-serif;
}



p{
    text-align: left;
    padding-top: 10px;
    color: #fff;
    font-family: sans-serif;
    letter-spacing: 0.4px;

}

a{
    text-align: left;
    padding-top: 10px;
    color: #fff;
    font-family: sans-serif;
    letter-spacing: 0.4px;
}

a:hover{
    text-align: left;
    padding-top: 10px;
    color: #fff;
    font-family: sans-serif;
    letter-spacing: 0.4px;
    text-decoration: underline;
}
<!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">
    <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u" crossorigin="anonymous">
    <link rel="stylesheet" href="./css/style.css">
    <link rel="icon" href="./img/favicon.png" type="image/png">
    <title>site</title>
</head>


<body>
    <div class="container-fluid">
            <div class="left-side">
                <img src="./img/logo.png" alt="Logo" class="logo">
            </div>

            <!-- Right hand side -->

            <div class="right-side">
                            <div class="right-inner">
                                <h3 class="welcome">Welcome back <span class="cust-name">Alan!</span></h3>    
                                        <form>
                                            <div class="form-group row">
                                            <div class="col-sm-10">
                                                <input type="email" class="form-control" id="inputEmail3" placeholder="Email">
                                            </div>
                                            </div>
                                            <div class="form-group row">
                                            <div class="col-sm-10">
                                                <input type="password" class="form-control password" id="inputPassword3" placeholder="Password">
                                            </div>
                                            </div>
                                            <div class="form-group row">
                                                <div class="col-sm-10">
                                                    <input class="btn btn-primary log-in" id="log-in" type="submit" value="Log in">
                                                    <p>Forgot your password?</p>
                                                </div>
                                            </div>
                                        </form> 
                                        
                                        <p>Don't have an account? <a href="#">Register</a></p>
                            </div>    
            </div>

            <!-- right hand side End -->
    </div>
</body>









</html>

Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

Semantics and UX

You can adjust a few things that will improve the overall UX of your login-page:

Document outline / structure

To improve the overall structure of your document use sectioning elements like header, main, section etc.:

<header class="left-side">
    <img src="./img/logo.png" alt="Logo" class="logo">
</header>

<main class="right-side">
    <h1>Welcome back</h1>
</main>

Keep in mind that headings are hierarchical from h1 to h6 in HTML. So it might make sense to have a h3 as the first heading in a sectioning element, but it really depends on your content.

See also on MDN "Sections and Outlines of an HTML5 Document".

Don't use placeholder as labels

I know, it's easy to set up and it instantly looks nice. But this is not the intended use-case. See the specs:

The placeholder attribute represents a short hint (a word or short phrase) intended to aid the user with data entry when the control has no value.

The placeholder attribute should not be used as a replacement for a label. For a longer hint or other advisory text, place the text next to the control.

From W3C "4.10.5.3.10. The placeholder attribute"

There a are a lot of downsides of using placeholders as labels, like:

  • once something is typed into a field, you have to rely on the short-term memory of the user, to remember which field was which
  • the form can get ambiguous as soon as you have pre-filled elements as well
  • it reduces accessibility and usability for users with cognitive, mobility, fine motor skill or vision impairments

Instead use label-elements:

<label>
    Email
    <input type="email">
</label>

<label for="form-email">Email</label>
<input id="form-email" type="email">

You can read more about this on the Nielson Norman Group "Placeholders in Form Fields Are Harmful " or on Smashing Magazine "Don’t Use The Placeholder Attribute".

"Forgot your password"-Link

This is a paragraph, which will do nothing, when the user clicks on it. Make it an anchor, that takes the user to the "forgot password"-form:

<a href="">Forgot your password?</a>

You don't necessarily need to wrap it into an p-element, but that's just optional.

Indentation

I would reduce the amount of indentation to make the source more readable. It also looks like the form-element is nested within the h3. This could be improved:

<h3 class="welcome">Welcome back <span class="cust-name">Alan!</span></h3>

<form>
</form>

On the other hand some nested elements could be indented to increase readability: 

<div class="form-group row">
    <div class="col-sm-10">
        <input type="email" class="form-control" id="inputEmail3" placeholder="Email">
    </div>
</div>

CSS

Don't limit the page's size

… unless there's a very specific use-case of your UI.

You set overflow: hidden; on the body-element. Then you set height: 100vh; on the container that holds all content. That can work, if you have a fluid layout that adopts to all screen sizes. But usually it's a bad idea as you don't know how high the viewport is, whether the user zoomed in etc. You see it already, when running the snippet – You can't scroll down to use the login form. This should be fixed.

Improve this answer
\$\endgroup\$
1
  • 1
    \$\begingroup\$ I was thinking of posting my second grid layout for review here. I hope I get as thorough a review as above. Nice one insertusernamehere ! \$\endgroup\$
    – Kerry7777
    Aug 14, 2019 at 12:30

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.