Better way to use pseudo elements

Question

Here is a JSFiddle of an accordion menu that i've done. I have used :before to add the icons.

My question is, is there a better way to Add different icon to each li item?

My code is as follows:

HTML Structure:

    <ul id="accordion">
  <li><div id="profile">Profile</div></li>
  <li><div id="messages">Messages</div>
    <ul class="submenu">
      <li>Inbox</li>
      <li>Sent</li>
      <li>Drafts</li>
    </ul>  
  </li>
  <li><div id="settings">Settings</div></li>
  <li><div id="logout">Logout</div></li>
</ul>

CSS:

    body {
background-image: url(http://subtlepatterns.subtlepatterns.netdna-cdn.com/patterns/tiny_grid.png);
}

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

#accordion{
  margin: 20% auto 80% auto;
  width:200px;
  height:auto;
  background-color:#212830;
  border-radius:5px;
  color: #BECEDB;
  font-family: 'Maven Pro', sans-serif;
  border: 1px solid #191E24;
  box-shadow: 0px 0px 5px 2px #666666;
}

#accordion > li > div{
  background-color:#383C48;
  font-weight:700;
  -moz-box-shadow:inset 0 1px 1px -1px #A1B3C9;
  -webkit-box-shadow:inset 0 1px 1px -1px #A1B3C9;
  box-shadow:inset 0 1px 1px -1px #A1B3C9;
  border-bottom:1px solid #191E24;
}

#accordion > li > div, #accordion > li > ul > li{
  padding:10px 15px;
} 

#accordion li{
  cursor:pointer;
}

#accordion > li:first-child >div{
  border-radius: 5px 5px 0 0;
}

#accordion > li:last-child >div{
  border-radius: 0 0 5px 5px;
}

#profile:before,
#messages:before,
#settings:before,
#logout:before,
.submenu>li:before
{
  font-family:FontAwesome;
}

#profile:before{
  content:"\f007  ";
}
#messages:before{
  content:"\f0e0";
}

#settings:before{
  content:"\f013";
}

#logout:before{
  content:"\f08b";
}

.submenu li:before{
  content:"\f0da"
}

#accordion *:before{
  margin-right:10px;
}

.submenu li{
  margin-left:10px;
}

JS code: (No problem with this)

 $("#accordion > li > div").click(function(){
  if(false == $(this).next().is(':visible')){
        $('#accordion ul').slideUp(400);
    }
  $(this).next().slideToggle(300);
 });

$('#accordion ul:eq(0)').show();

Also I would like to know if there are other areas this can be optimized.
Thank you

Answer 1

First things first: You have a navigation but refuse to use anchor tags. Why? This is potentially harmful. How are you going to navigate if you don't have links? The only possible way would be completely relying on JavaScript. All users with disabled JavaScript have a useless list of words instead.

HTML:

• Use a tags instead of the div's in your HTML (Use <a href="#"> to test this)
• I'd suggest using classes instead of ID's in this scenario as well. The lower specificity of classes will ease your live later. Overwriting an ID instead could be hard

CSS:

• If you use actual links for your navigation, there is no need for cursor: pointer; on list-items.
• If you want the icons to be clickable as well, add the pseudo-elements to the links instead of the list-items
• Don't reset the general styling for lists on the type selector ul. Doing this makes you unable to use lists as regular lists in text etc. You can reset the styles for the navigation on #accordion
• Adding a class accordion-item to your list-items could help you. You don't selecting like this: #profile:before, #messages:before, #settings:before...

You have some things to consider now. There are more things to improve, but I'd like to see a new question for an updated navigation with the given advice.

Answer 2

I'm going to answer this question:

is there a better way to Add different icon to each li item?

The short answer is, yes. The better way is to use the classes that FontAwesome provides out of the box, rather than duplicating it in your own css. In this example I would make the following changes to the html:

<ul id="accordion">
  <li><div id="profile"><i class="fa fa-user"></i>Profile</div></li>
  <li><div id="messages"><i class="fa fa-envelope"></i>Messages</div>
    <ul class="submenu">
      <li><i class="fa fa-caret-right"></i>Inbox</li>
      <li><i class="fa fa-caret-right"></i>Sent</li>
      <li><i class="fa fa-caret-right"></i>Drafts</li>
    </ul>  
  </li>
  <li><div id="settings"><i class="fa fa-cog"></i>Settings</div></li>
  <li><div id="logout"><i class="fa fa-sign-out"></i>Logout</div></li>
</ul>

See this forked fiddle. Note that I had to fix your reference to the fontawesome css, which was giving me a 404 error, and I also deleted the bit of your css that was doing all the ":before" classes.

As a side note, I totally agree with @kleinfreund on almost all his points.

Answer 3

Since you have already other answers , I will focus on different issues

This :

#accordion > li:first-child >div{
  border-radius: 5px 5px 0 0;
}

#accordion > li:last-child >div{
  border-radius: 0 0 5px 5px;
}

Is not needed. You have already border-radius on the container.

And the same for that:

#accordion > li > div{
  background-color:#383C48;

Not a big issue, but the duplicity of styles makes your life harder later. (When you are changing the border-radius and see that it doesn't change anything)