patricktalmadge/bootstrapper — pull request #48 by Anahkiasen (2012-09-10 14:55:50+00:00)
Title: Refactoring of Navbar class (with inverted option)

The current Navbar class is a mess – it's more of a function than a class, and both its syntax and its number of arguments make it more painful to use it than raw HTML. This is the case for a lot of classes in Bootstrapper that don't follow the Laravel elegance principles by trying to do too much with one function. The very example on how to use Navbar is ~50 lines long.
So since I thought I found something good while refactoring the Buttons class to mix static and instances, I tried doing the same with the Navbar class. **This is *not* your mother's pull request so read carefully before merging**.

--------

* First of all, the new Navbar class can be created two ways : `Navbar::create` and `Navbar::inverse` to allow the creation of inverted navbars.
* A lot of the arguments that used to be in `Navbar::create` were delegated to chained methods, which not only makes them opt-ins but allow for a clearer syntax.

The only arguments remaining in Navbar::create are the one that actually concern the top navbar wraper : $attributes, and $type (FIX_TOP, etc.)
Everything else is now a method, the methods are the following :

* `with_brand($brand, $brand_url)` adds a brand to the current navbar
* `autoroute([true/false])` activates/desactivates the autoroute function when creating the navbar
* `collapsible()` makes the navbar collapsible
* `with_menus($menus, $attributes)` adds an entry to the current menus, and I can't stress enough the *adds an entry* part, like when you do `array[]`, meaning `with_menus()` can be called several times on the same Navbar with different contents (both strings and arrays like the old class used to manage)

--------

Final example of use :

```php
{{
Navbar::inverse(array('class' => 'myclass'), Navbar::FIX_TOP)
->with_brand('Website', 'http://www.site.com/')
->with_menus(array(array('label' => 'Home', 'url' => '#'), array('label' => 'Back', 'url' => '#')))
->with_menus('

')
->collapsible()
->autoroute(false)
}}
```

Simple, readable, Laravelish. Everything is optional, you can totally not call `with_brand` and never have to deal with arguments in the way or anything else.
As with all my pull requests today, this is a stub, if you see ways to make it better I'd be glad that we all discuss it.
As I also said, since a few more classes in Bootstrapper suffer from TooMuchArgumentosis, if this refactoring is in people's opinion "better" and would like to see it applied to other classes, that's what I had planned. I just prefer seeing first what you guys think of it on one class before doing the whole thing.