Improved usability by adding a tabbish feel to admin navbar.
Review Request #1029 — Created Sept. 8, 2009 and submitted
When configuring ReviewBoard is really easy to loose track on which tab you are, since there was no visual cue to that. There was a css class already in admin.css called '#admin-subnav li a.active', but it was never used. So I hooked it up in the code, so it stared being used. But then it clashed too much, and the tabs didn't feel like tabs. So I changed it to include a background and a border so users can instantly recognize where they are. Since the breadcrumbs part has a little gradient, I had to expand that gradient otherwise the background clashed.
Tested result in Safari 4.0, Firefox 3.5 and 3.0 and Chromium Nightly.
Screenshots
- Change Summary:
-
Updated the diff to include the template tag suggestion by Christian and fixed space issues in CSS.
-
-
-
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.
-
-
-
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 {..} -
-
-
-
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`.
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.