Extended navigation bar #221

pbanaszkiewicz · 2015-03-08T21:51:51Z

This PR:

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

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')]`)

pbanaszkiewicz · 2015-03-08T22:28:10Z

Preview:

wking · 2015-03-08T23:22:58Z

On Sun, Mar 08, 2015 at 03:28:11PM -0700, Piotr Banaszkiewicz wrote:

Preview: …

It looks a bit wierd to me to have a fixed-width for the page content
but a fluid width for the nav bar. Personally, I'd prefer a fluid
layout everywhere.

wking · 2015-03-08T23:30:03Z

Why do we need TEMPLATE_CONTEXT_PROCESSORS (it looks like you're only
adding django.core.context_processors.request to the defaults 1)? I
guess it's for request.path in navbar_element?

The implementation looks reasonable to me, but I'd list everything
from the “Menu” column of the main page and then drop that column once
the navbar was populated.

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.

pbanaszkiewicz · 2015-03-09T09:38:26Z

Thanks @wking for the feedback, I changed things accordingly.

Here's little preview:

I had an idea to put a third column in Dashboard: "Events with no instructors", but it turned out too complicated… @gvwilson do you think it would be useful to have such events listed in the dashboard?

wking · 2015-03-09T16:38:27Z

On Mon, Mar 09, 2015 at 02:38:27AM -0700, Piotr Banaszkiewicz wrote:

I had an idea to put a third column in Dashboard: "Events with no
instructors", but it turned out too complicated…

Can you spin that off into it's own issue? I don't want to
overcomplicate this navbar PR.

Provided that others aren't concerned by the fluix/fixed styling issue
1, I think this is ok to merge.

If the TEMPLATE_CONTEXT_PROCESSORS change isn't compatible with Django
1.8, we probably want to update the requirements.txt to have:

Django>=1.7,<1.8

pbanaszkiewicz · 2015-03-09T17:32:58Z

Can you spin that off into it's own issue? I don't want to overcomplicate this navbar PR.

Done in #224. (BTW: I did not add this feature because it is hard to do in Django ORM…)

Provided that others aren't concerned by the fluix/fixed styling issue [1], I think this is ok to merge.

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:

If the TEMPLATE_CONTEXT_PROCESSORS change isn't compatible with Django 1.8, we probably want to update the requirements.txt to have: Django>=1.7,<1.8

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 TEMPLATES for >=1.8 or TEMPLATE_CONTEXT_PROCESSORS, TEMPLATE_DEBUG, TEMPLATE_DIRS, TEMPLATE_LOADERS, TEMPLATE_STRING_IF_INVALID for <1.8.

If you agree with my idea, I'll do this in separate PR.

Fixed-width navbar looks better: https://cloud.githubusercontent.com/assets/72821/6560285/49d523d2-c689-11e4-800f-efab414ba3b0.png

wking · 2015-03-09T17:39:02Z

On Mon, Mar 09, 2015 at 10:32:59AM -0700, Piotr Banaszkiewicz wrote:

There's one issue with this many items in the navbar. It looks
strange below 1200px width:

Maybe we should push tasks/badges/airports into the dropdown and
rename it from “Utilities” to “More”? I think Bootstrap has a way to
trigger a full menu collapse at a give screen size, maybe we just want
to do that?

I can add a conditional that discovers if we're in 1.8 yet…

Sounds good to me, and a separate PR for this would be great.

pbanaszkiewicz · 2015-03-09T17:50:00Z

I think Bootstrap has a way to trigger a full menu collapse at a give screen size, maybe we just want to do that?

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

wking · 2015-03-09T17:57:13Z

On Mon, Mar 09, 2015 at 10:50:01AM -0700, Piotr Banaszkiewicz wrote:

If you give me a green light, I'll go ahead and merge.

Looks good to me, but again [1,2], I'm not the maintainer ;). I'd
recommend having a clear maintainer who performs (or delegates) review
to avoid things like the recent #200 (with discussion about some new
environment variables) being followed immediately by #204 and #206,
both of which (I think) should have been handled (or at least
discussed) before #200 landed. Of course, @gvwilson merged #200, so
maybe it doesn't matter ;).

pbanaszkiewicz · 2015-03-09T18:00:41Z

@wking I just updated this PR with 8d0a538 seconds before your email arrived. I agree we need to know who's in charge, but I guess @gvwilson is AFK at the moment.

wking · 2015-03-09T18:09:38Z

On Mon, Mar 09, 2015 at 11:00:42AM -0700, Piotr Banaszkiewicz wrote:

I just updated this PR with 8d0a538 seconds before your email
arrived.

Heh, that looks like a good change ;).

I agree we need to know who's in charge, but I guess @gvwilson is
AFK at the moment.

The world won't explode if this PR waits for his return ;). I can't
deploy anything I merge anyway, so it's quite likely that regardless
of when this gets merged, it won't get deployed until he's back.

pbanaszkiewicz · 2015-03-11T18:44:34Z

Hey @wking

this is blocking #223 (I certainly won't explode because of that :) ). I think I'll just make a PR for #223 that's based on this PR (#221).

wking · 2015-03-11T18:52:49Z

On Wed, Mar 11, 2015 at 11:44:35AM -0700, Piotr Banaszkiewicz wrote:

I think I'll just make a PR for #223 that's based on this PR (#221).

For things like this that are likely to land without further edits, I
don't see a problem with stacking PRs like that. On the other hand, I
don't see much point in getting too far out ahead of the maintainer
;).

pbanaszkiewicz · 2015-04-03T19:52:14Z

Based on the feedback from @wking, I'm merging this one.

Extended navigation bar

pbanaszkiewicz added 2 commits March 8, 2015 22:49

Add a navigation anchors to the navigation bar

9f48c33

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

pbanaszkiewicz added 3 commits March 9, 2015 09:22

settings.py: import default templ. ctx processors

1745424

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).

Navbar: add all entries from Menu from index page

723056a

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.

index: remove menu links

e3918b6

Links in "Menu" section were moved to the navbar.

This was referenced Mar 9, 2015

Add selection to debrief session #216

Merged

Link to debrief page + datepicker #223

Closed

wking mentioned this pull request Mar 9, 2015

Enhance Amy, a Django application for managing workshops numfocus/gsoc#6

Closed

pbanaszkiewicz mentioned this pull request Mar 9, 2015

Idea: "events with no instructors" on the main page #224

Closed

Navbar: switch from fluid to fixed-width

92f03e5

Fixed-width navbar looks better: https://cloud.githubusercontent.com/assets/72821/6560285/49d523d2-c689-11e4-800f-efab414ba3b0.png

Navbar: cleanup

8d0a538

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

wking mentioned this pull request Mar 26, 2015

Proposal Draft for the project: Enhance Amy, a Django application for managing workshops numfocus/gsoc#39

Merged

pbanaszkiewicz added a commit that referenced this pull request Apr 3, 2015

Merge pull request #221 from pbanaszkiewicz/new-navbar

cba3ec5

Extended navigation bar

pbanaszkiewicz merged commit cba3ec5 into carpentries:master Apr 3, 2015

pbanaszkiewicz deleted the new-navbar branch April 3, 2015 19:57

pbanaszkiewicz mentioned this pull request Apr 3, 2015

Add a navigation bar. #194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extended navigation bar #221

Extended navigation bar #221

pbanaszkiewicz commented Mar 8, 2015

pbanaszkiewicz commented Mar 8, 2015

wking commented Mar 8, 2015

wking commented Mar 8, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 11, 2015

wking commented Mar 11, 2015

pbanaszkiewicz commented Apr 3, 2015

Extended navigation bar #221

Extended navigation bar #221

Conversation

pbanaszkiewicz commented Mar 8, 2015

pbanaszkiewicz commented Mar 8, 2015

wking commented Mar 8, 2015

wking commented Mar 8, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 9, 2015

wking commented Mar 9, 2015

pbanaszkiewicz commented Mar 11, 2015

wking commented Mar 11, 2015

pbanaszkiewicz commented Apr 3, 2015