Hey Eduardo,

Thanks for the change. I like the new look a lot better.

I have some ideas on the implementation, though. I think rather than capturing the page name in the URLs, we should keep all logic about the current page in the view instead. I think we can actually 

The way we can do this is to implement a new templatetag for the admin UI for adding a nav item. We'd use it like:

{% admin_subnav "url-name" _("Text") %}

The implementation for this would add the <li>...</li>, resolving url-name using reverse() (something I've wanted to do for a while, instead of hard-coding the paths -- this means we need to name those URLs too), and applying the "active" class if needed.

The template tag would determine the active URL by comparing the result of reverse(url_name) with request.path.

The nice thing here is that all logic for this goes right into the template tag, and we have a nice reusable thing in the templates that doesn't have to care about the active state itself.

How does that sound?

Also, ping me on IRC at some point. I seem to have issues e-mailing your account.

Should be indented 2 spaces.

We should make sure that -6px is correct for this case. If you increase your text zoom a lot, does it look okay? We may want to use em.

Should just be indented 2 spaces.

Oooops. I had forgotten the shortcut for it, heheh.

Blank line between these.

Actually, it might be better to render using a template which would have this logic. While technically slower, it keeps presentation in the templates.

You can do this with an inclusion tag, which the docs should cover.

Can remove these blank lines.

Can you specifically do name="general", etc. on these?

Hmm, maybe we should call these rbadmintags, so that there's no possible conflict with Django's own admin stuff.

You should use _("General"), etc. so that it's internationalized.

You'll need to {% load i18n %} for this, I think.

Hi Eduardo.

Just a few small stylistic things and then it's ready to commit :) We need to set you up on GitHub with commit access, so let us know what your GitHub username is (if you have one) and we'll set it up.

This would be better formatted as:

return {
    'url': url,
    'name': name,
    'current': url == request.path,
}

Make sure the {...} aligns with the r'...'.

Also, name="logging" would be better on its own line, aligned with r'..' and {..}

Swap these. Alphabetical order.

Variables (as opposed to template tags) should be in the form of {{url}}.

Really sorry, but I didn't think of this before. Can you change the names to settings-general, settings-email, etc? Less chance of collisions with other things. Make sure to update the template as well.

Then it's ready to commit! :) You'll need to edit your .git/config and change origin's url to git@github.com:reviewboard/reviewboard.git. Then squash this into one commit in master preferably so it doesn't appear as a merge, add the "Reviewed at http://reviews.review-board.org/r/1029/" and then `git push origin master`.