8
\$\begingroup\$

I'm using simple script to handle tab menu like this:

HTML:

<nav id="personal">
    <ul class="clearfix">
        <li id="personal-info">Info</li>
        <li id="personal-orders">Orders</li>
        <li id="personal-models">Models</li>
        <li id="personal-balance">Balance</li>
    </ul>
</nav>

<div id="personal-content"></div>

and CoffeeScript:

jQuery ($) ->
 class Nav
  constructor: (@elements) ->
    self = @
    wHash = document.location.hash.replace("#", "") or "info"

    # activate element by location hash
    @.activate document.getElementById "personal-#{wHash}"

    # bind element activation on click
    elements.click ->
      self.activate @

  activate: (elem) ->
    @elements.removeClass "active"
    $(elem).addClass "active"

    # set a proper hash
    wHash = elem.id.split("-")[1]
    document.location.hash = "##{wHash}"

    # load proper content into an inner section
    # it works very fast and is the reason i use this type of navigation in the first place

    $("#personal-content").load "/user/#{wHash}"

 new Nav $("#personal").find("li")

This code works, but i'm pretty sure that it's very far away from best practices, and can be greatly improved, as these are my first tries in CoffeeScript. So the question is, can this code be beautified in some way?

\$\endgroup\$
7
\$\begingroup\$

You're building a navigation, however there are no links. It's highly recommended to use anchor tags for linking purposes.

Your naming is a bit loose. What is personal in your context? Please specify that. Even something like personal-navigation is quite loose, but still better.

I've changed your markup a bit to reflect my small suggestions. Note that I changed the ID's from your list items to classes. ID's are only for elements which will occur only once a page in any situation. You can't say that for sure for single navigation items.

    <nav id="personal-navigation">
        <ul class="clearfix">
            <li class="personal-info"><a href="#">Info</a></li>
            <li class="personal-orders"><a href="#">Orders</a></li>
            <li class="personal-models"><a href="#">Models</a></li>
            <li class="personal-balance"><a href="#">Balance</a></li>
        </ul>
    </nav>

    <div id="personal-content"></div>
\$\endgroup\$
3
  • \$\begingroup\$ personal is the name of the page this navigation bar is located at, it is a personal cabinet, where you can see your info, orders, models and stuff. I'm quite sure, that my li id's are unique, and as this page isn't meant for indexing of any sort I'm fine with that, keeping in mind that using ids is way more convenient when you want to get one single DOM element, and faster too. My question goes for the scripting part, not markup. \$\endgroup\$ – Eternal1 Feb 10 '14 at 17:36
  • 2
    \$\begingroup\$ You've put the html on your question so I was assuming this is part of your question. I'm no scripting guy, so I can't tell whether getting ID's or classes is faster. If this is the way you do it, it's perfectly fine. Keep in mind answers here are no orders but suggestions. ;) \$\endgroup\$ – kleinfreund Feb 10 '14 at 17:38
  • 2
    \$\begingroup\$ My comment doesn't mean I don't appreciate Your effort to help :) \$\endgroup\$ – Eternal1 Feb 10 '14 at 17:41

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.