Grow your CSS skills. Land your dream job.

Please critique my technique

  • # July 21, 2009 at 8:01 pm

    Hi, I’m pretty new to jQuery as a whole and really have appreciated the help and feedback of these forums. I just was hoping someone could tell me if this is the best way to do this. I finally got this thing working (haven’t added all the sweet stuff to it).

    http://www.citrusbunny.com/
    *It’s the "Contact Us" on the right side – supposed to slide out*

    through my research it seems there are a ton of different ways to get jQuery to do what you want. Was this the best way to execute this? Should I have done it differently?

    I just want to make sure I’m doing it right rather than "just making it work".

    This is the code for the slide:

    stylesheet.css

    Code:
    li.action {
    font-family: georgia;
    font-size: 20px;
    color: #777;
    margin-left:15px;
    }
    li.action:hover {
    color: #000;
    }
    #contactslide {
    height: 200px;
    width: 250px;
    background-color:#ffffff;
    margin-left: 40px;
    padding:0px;
    margin-top:0px;
    margin-bottom:0px;
    }

    indexcustom.php

    Code:
    • Contact Us

    520-555-5555

    Header.php

    Code:
    < ?php wp_enqueue_script("jquery"); ?>

    < ?php wp_head(); ?>

    # July 22, 2009 at 11:30 am

    Nope – that looks just fine! The only important thing is that it makes sense to you and anyone else that will be coding on the site.

    # July 27, 2009 at 5:24 am

    Few things to note, avoid using inline styling

    Code:

    For questions of any kind, please call us Monday – Friday 8am to 5pm. Otherwise email us and we will contact you in 2 shakes of a lambs tail.

    520-205-2306

    Code:

    For questions of any kind, please call us Monday – Friday 8am to 5pm. Otherwise email us and we will contact you in 2 shakes of a lambs tail.

    520-205-2306

    // css file
    div#contactslide p { text-align: center; }
    div#contactslide p.contactNumber { font-family: georgia; color: rgb(119, 119, 119); font-weight: bold; font-size: 30px;}

    Its bad practice and clean code is just better for the eyes ;)


    2nd thing

    In css its encouraged to use as many elements to select one (I.E. == html body div.content div#nav li.action {….)
    When using jquery, its just the opposite

    Code:
    jQuery(“li.action”).click(function(){….

    What this is doing is selecting every single <li> on the page…. stores them and then filters all those <li> looking for the class name "action"

    note: Selecting by a class is slower than id

    3rd thing

    Code:
  • Contact Us

This is the only element with the class of "action", Classes generally apply to multiple elements and this is the clearly the only li that is going to have the action therefor should be an id

The Element is also supposed to be click able and you dont get the pointer when hovering fixable by css (cursor: pointer;) or better yet

Code:

It is supposed to be a clickable element its always good to not reinvent the browser recreating the way it defaultly acts

4th Thing

Notice the little jump at the end of the slide down and beginning of the slide up?

Code:
#contactslide {
background-color:#FFFFFF;
height:200px;
margin-bottom:0;
margin-left:40px;
margin-top:0;
padding:0;
width:250px;
}

remove the height, it will fix that

Code:
#contactslide {
background-color:#FFFFFF;

margin-bottom:0;
margin-left:40px;
margin-top:0;
padding:0;
width:250px;
}


5th thing….

It seems like you could remove a handful of elements that dont need to be there (ie div id=’lower’) that just has the height of 5px… you can just apply top margin to the header element

And im tired….

Viewing 3 posts - 1 through 3 (of 3 total)

You must be logged in to reply to this topic.

*May or may not contain any actual "CSS" or "Tricks".