Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite 
    <!DOCTYPE HTML>
<html lang="en">
<head>  
    <meta charset="utf-8">
    <title>Login and Registration</title>
    <link rel="stylesheet" href="reset.css" type="text/css">
    <link rel="stylesheet" href="style.css" type="text/css">
    <script src="http://code.jquery.com/jquery-1.8.1.js"></script>
    <script>
        $(function() {           
            var tabContainers = $('section.tabs > article');
            tabContainers.hide().filter('#login').show();

            $('.ultabs a').click(function () {
                tabContainers.hide();
                tabContainers.filter(this.hash).show();
                $('.ultabs a').removeClass('selected');
                $(this).addClass('selected');
                return false;
            });
        });
        function check_email() {
            var email=$("#textEmail").val();
            $.ajax(
            {
                type:"POST",
                url:"register.php",
                data: { 
                'email': email
                },
                success:function(msg) {
                    $("#checkEmail").html(msg);
                }
            });
            return false;
        }
        function check_password() {
            var pass=$("#textPassword").val();
            $.ajax(
            {
                type:"POST",
                url:"register.php",
                data: { 
                'pass': pass
                },
                success:function(msg) {
                    $("#checkPassword").html(msg);
                }
            });
            return false;
        }
    </script>
</head>
<body>
<div id="siteWrapper">
    <header id="siteHeader">
        <h1>Header</h1>
    </header>
    <nav id="siteNav">
        <h1>Navigation</h1>
        <ul class="ultabs">
            <li><a href="#login">Login</a></li>
            <li><a href="#register">Register</a></li>
        </ul>
    </nav>
    <section id="siteContent" class="tabs">
        <h1>Section</h1>
        <article id="login">
            <h1>Login</h1>
            <table>
                <tr>
                    <td class="message" colspan="2"></td>
                </tr>
                <tr>
                    <td>E-mail:</td><td><input type="email"></td>
                </tr>
                <tr>
                    <td class="message" colspan="2"></td>
                </tr>
                <tr>
                    <td>Password:</td><td><input type="password"></td>
                </tr>
                <tr>
                    <td colspan="2"><input type="button"></td>
                </tr>
            </table>
        </article>
        <article id="register">
            <h1>Register</h1>
            <table>
                <tr>
                    <td class="message" colspan="2"></td>
                </tr>   
                <tr>
                    <td>Name:</td><td><input type="text"></td>
                </tr>
                <tr>
                    <td id="checkEmail" class="message" colspan="2"></td>
                </tr>
                <tr>
                    <td>E-mail:</td><td><input id="textEmail" type="email" onblur="return check_email()"></td>
                </tr>
                <tr>
                    <td id="checkPassword" class="message" colspan="2"></td>
                </tr>
                <tr>
                    <td>Password:</td><td><input id="textPassword" type="password" onblur="return check_password()"></td>
                </tr>
                <tr>
                    <td class="message" colspan="2"></td>
                </tr>
                <tr>
                    <td>Re-type:</td><td><input type="password"></td>
                </tr>
                <tr>
                    <td colspan="2"><input type="button"></td>
                </tr>
            </table>
        </article>
    </section>
    <footer id="siteFooter">
        <h1>Footer</h1>
    </footer>
</div>
</body>
</html>
share|improve this question
2  
Well, you can start by not using tables for layout. That's what CSS is for. – Joseph Silber Sep 7 '12 at 14:46
i just use a little table because i am having a hard time aligning the label from the textboxes :( thanks a lot – Brained Washed Sep 7 '12 at 15:48

2 Answers

up vote 5 down vote accepted

Your code look great. Here are a few tips.

1)

Don't use inline javascript. Especially when using jQuery.

$(el).blur( handler(eventObject) )

Old Javasript:

$(function() {           
    var tabContainers = $('section.tabs > article');
    tabContainers.hide().filter('#login').show();
    //...

New Javasript:

$(function() {           
    var tabContainers = $('section.tabs > article');
    tabContainers.hide().filter('#login').show();
    $( "#textEmail" ).blur( check_email );
    $( "#textPassword" ).blur( check_password );        
    //...

Old HTML: 

<tr>
    <td>E-mail:</td><td><input id="textEmail" type="email" onblur="return check_email()"></td>
</tr>
<tr>
    <td id="checkPassword" class="message" colspan="2"></td>
</tr>
<tr>
    <td>Password:</td><td><input id="textPassword" type="password" onblur="return check_password()"></td>
</tr>

New HTML:

<tr>
    <td>E-mail:</td><td><input id="textEmail" type="email"></td>
</tr>
<tr>
    <td id="checkPassword" class="message" colspan="2"></td>
</tr>
<tr>
    <td>Password:</td><td><input id="textPassword" type="password"></td>
</tr>

2)

Try not to mix html, javascript, and css. Save all your javascript and css in external files, then import them.

Old Code:

<script>
    $(function() {           
        var tabContainers = $('section.tabs > article');
    //...

New Code:

<script src="main.js"></script>

3)

Tables should be used for displaying data, not for design.

Why not to use tables for design

How to use divs to make tables

4)

Use $.post instead of $.ajax.

Old Code:

function check_email() {
    var email=$("#textEmail").val();

    $.ajax(
    {
        type:"POST",
        url:"register.php",
        data: { 
        'email': email
        },
        success:function(msg) {
            $("#checkEmail").html(msg);
        }
    });
    return false;
}

New Code:

function check_email() {
    $.post( "register.php", { 
        "email": $("#textEmail").val() 
        }, 
        function(msg){
            $("#checkEmail").html(msg);
    });
    return false;
}

5)

Use a namespace for url or other constants. Example:

var URLS = { 
    register: "register.php" 
};
share|improve this answer
in no. 1 where should I put this line of code $("#textEmail").blur( check_email ); ? – Brained Washed Sep 8 '12 at 5:38
In no. 2, all my js should be on external files including the src="code.jquery.com/jquery-1.8.1.js";? in no. 5 how can I use that? Hehe thank you so much for helping really appreciate it :) – Brained Washed Sep 8 '12 at 5:42
no. 1: In a file like js/main.js . no 2: No, only for the javascript in the html file. It's best not to modify plugin files. no. 5, URLS.register == "register.php" – Larry Battle Sep 8 '12 at 7:00
can you give me a full example of number 1 Thanks – Brained Washed Sep 8 '12 at 13:47
@BrainedWashed I updated my answer. Hope that helps. – Larry Battle Sep 9 '12 at 17:07
show 1 more comment
up vote 0 down vote

Regarding the outline of your document:

  • I think that you shouldn't use article for your "Login" and "Register" forms, because these are not "in principle, independently distributable or reusable, e.g. in syndication" (HTML5 spec). You should use section for them.
  • What is the real heading content for the one you labeled "Section"? It may be possible that this "parent" section is not needed (and may be even wrong, depending on your content).
  • You should use a h2 for the footer element, because it is not a sectioning element. It's the footer to your whole document, so it should be in the scope of the site header (→ one level lower). Otherwise (if you want to use h1 here) you should use a sectioning element in the footer element, probably aside or section (depending on your content)

You can check the outline of your document there: http://gsnedders.html5.org/outliner/

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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