-
-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extended navigation bar #221
Conversation
This commit: * adds `navbar_element` template tag * adds 4 new positions in the navigation bar (on top of the page) * adds a search form in the navbar - a shortcut to the official search page
Introduction of search form in the top navbar made some of our tests complaining (XPath was returning two forms instead of one form for one specific test). This commit adds `role="form"` accessibility attribute that helps us match specific form by XPath. (ElementTree's XPath is limited and doesn't allow this kind of syntax: `.//form[not(@ROLE='search')]`)
On Sun, Mar 08, 2015 at 03:28:11PM -0700, Piotr Banaszkiewicz wrote:
It looks a bit wierd to me to have a fixed-width for the page content |
Why do we need TEMPLATE_CONTEXT_PROCESSORS (it looks like you're only The implementation looks reasonable to me, but I'd list everything |
Instead of hard-coding current default TEMPLATE_CONTEXT_PROCESSORS, just import these from Django default settings and add one processor we need (for `request` object in templates).
Every "View all *" page is now available instantly from the navbar. Additionally util pages, like bulk add or search or export, were grouped together into "Utilities" dropdown.
Links in "Menu" section were moved to the navbar.
On Mon, Mar 09, 2015 at 02:38:27AM -0700, Piotr Banaszkiewicz wrote:
Can you spin that off into it's own issue? I don't want to Provided that others aren't concerned by the fluix/fixed styling issue If the TEMPLATE_CONTEXT_PROCESSORS change isn't compatible with Django Django>=1.7,<1.8 |
Done in #224. (BTW: I did not add this feature because it is hard to do in Django ORM…)
I have full HD resolution (1920x1080) - it makes most webpages look empty on the sides. Below is a preview of a fixed-width navbar: I think I like it more. I'll change accordingly. There's one issue with this many items in the navbar. It looks strange below 1200px width:
There's more to templates: https://docs.djangoproject.com/en/1.8/ref/settings/#std:setting-TEMPLATES I can add a conditional that discovers if we're in 1.8 yet and sets up If you agree with my idea, I'll do this in separate PR. |
On Mon, Mar 09, 2015 at 10:32:59AM -0700, Piotr Banaszkiewicz wrote:
Maybe we should push tasks/badges/airports into the dropdown and
Sounds good to me, and a separate PR for this would be great. |
I was unable to make Bootstrap collapse reasonably, so I moved Tasks, Badges, Airports to "More" menu - as you suggested. Now it looks fine and the odd visual glitch happens only between 768px and 991px. I think it's manageable. If you give me a green light, I'll go ahead and merge. |
1. Moved "Tasks", "Badges", "Airports" to "Utilities" menu 2. Renamed "Utilities" to "More" 3. Fixed brand link (was "/", is "/workshops") 4. Removed fake links from user's menu
On Mon, Mar 09, 2015 at 10:50:01AM -0700, Piotr Banaszkiewicz wrote:
Looks good to me, but again [1,2], I'm not the maintainer ;). I'd |
On Mon, Mar 09, 2015 at 11:00:42AM -0700, Piotr Banaszkiewicz wrote:
Heh, that looks like a good change ;).
The world won't explode if this PR waits for his return ;). I can't |
On Wed, Mar 11, 2015 at 11:44:35AM -0700, Piotr Banaszkiewicz wrote:
For things like this that are likely to land without further edits, I |
Based on the feedback from @wking, I'm merging this one. |
This PR:
navbar_element
template tagpage
Ref #194