CJSCommonPlatform / govuk_single_page_pdk

Single Page Platform Development Kit
http://cjscommonplatform.github.io/govuk_single_page_pdk/
13 stars 20 forks source link

navigation/menus/top-menu #55

Open vygis opened 8 years ago

trevorsaint commented 8 years ago

As lovely as BEM is (I'm a big fan) GDS don't adopt this naming convension. Please stick to the general naming pattern GDS use. For old and new components. We need consistency with everything we code. The same applies for everything else.

trevorsaint commented 8 years ago

This element is a menu. A menu is a navigational element and best practise for coding this up would be an unordered list.

I would also recommend against using any form of grid for an element like this.

I have not seen the design pattern for this element but would expect the dividers to be for all except the first item. If this is the case you don't require a class for the list item, but could add a modifier class on the ul like 'menu menu-dividers'.

I would mark this code up something along the lines of this:

<ul class="menu">
   <li class="is-active"><a href="#">First page</a></li>
   <li><a href="#">Second page</a></li>
   <li><a href="#">Third page</a></li>
</ul>

If this is a main menu for a page you could go one step further:

<nav class="menu" role="navigation" aria-label="Main menu">
  <ul>
    <li class="is-active"><a href="#">First page</a></li>
    <li><a href="#">Second page</a></li>
    <li><a href="#">Third page</a></li>
  </ul>
</nav>

:)

trevorsaint commented 8 years ago

A component like this should not really be using any grid classes.