root_level in show_sub_menu #1344

Merged
merged 20 commits into from Apr 8, 2013

Conversation

Projects
None yet
6 participants
Contributor

andrewschoen commented Jul 12, 2012

I end up creating a menu like this in just about every project. What I always need to do is print a sub_menu that starts after the root_level specified and not at the selected node. This will include all ancestors, descendants of the selected node (up to root_level), plus every sibling of the selected node and all their descendants.

@superdmp and I discussed a menu like this before. Does anybody else ever need to create menus this way?

Contributor

evildmp commented Jul 12, 2012

Nearly - mine need to be up to the nearest soft root (rather than some level).

And we don't print descendants of siblings (i.e. nephews and nieces). How about a nephews argument: 0 means no nephews, 1 means show immediate nephews, 2 means show nephews and great-nephews, and so on?

This pull request passes (merged 8151be6 into dc00224).

Contributor

andrewschoen commented Jul 12, 2012

@evildmp good idea, not sure if I would use it much but I guess having the option is good. How about an argument for showing the descendants of ancestors (uncles, aunts?) as well?

I wonder if anybody else will get any use out of this, or is it just us and our crazy university menus?

I wouldn't want to get too argument crazy, but if we think it'll be useful I can spend a bit of time on it.

This pull request passes (merged 89cfdb0 into dc00224).

Member

digi604 commented Aug 15, 2012

So what is the difference from

{% show_sub_menu 100 0 %}

to

{% show_menu 1 100 0 100 %}

?

Contributor

andrewschoen commented Aug 15, 2012

Here is an example of show_sub_menu 100 0

sub_menu

And here is the show_menu 1 100 0 100

show_menu

They are pretty similar but my version is showing more child pages. I've attempted to pull in more children pages with show_menu using the third parameter, but then I end up getting some of my apphook menus include, which I don't want.

Here's an example of show_menu 1 100 100 100 (the last two nav items are from the Polls apphook)

show_menu

Contributor

andrewschoen commented Aug 15, 2012

Honestly, if there is a way to get this to work without this patch that's great. I just have never been able to get my menus to work as designed using show_menu. I could very well just be missing something though.

This is a good site as an example of when I needed to use this patch. I needed to show all the nav from the active main navigation, including all descendants without showing anything from another main navigation node (Admissions, Athletics, etc.). I was never able to get that to work with show_menu alone.

http://www.rockhurst.edu/academics/international/study-abroad/france/

Contributor

andrewschoen commented Aug 15, 2012

Maybe these are a better example. I added Content Page 2 with it's own set of descendants underneath it.

With show_sub_menu 100 0, the menu under Content Page stays the same.

sub_menu

And here is show_menu 1 100 100 100 with Content Page 2 added. Subpage 5, 6 and child 4 should not show up as it's in the Content Page 2 tree.

show_menu

Member

digi604 commented Aug 15, 2012

Ok... confirmed this. I would like an other change though:

{% show_sub_menu 100 0 %} should start at level 0... so level 0 is displayed.

{% show_sub_menu 100 1 %} should display what you currently have.

Contributor

andrewschoen commented Aug 15, 2012

Alright, that makes sense. What's your thought on the nephews argument @evildmp suggested? I've started work on that as well, just about finished really.

Member

digi604 commented Aug 15, 2012

let me see code or the API

Contributor

andrewschoen commented Aug 15, 2012

ok, I pushed up my latest code. Still a work in progress but all the tests I wrote for it pass. This is the commit you'll want to look at.

andrewschoen/django-cms@f6e0b47

This pull request passes (merged fdf8462 into c3aa982).

Owner

andrewschoen commented on 7577ab5 Aug 15, 2012

This commit really isn't related to the PR, but I noticed this when I was working on it. Why go back to the db when create_page returns the page object anyway? This speed up the test suite by a couple seconds for me.

This pull request passes (merged 7577ab5 into c3aa982).

This pull request passes (merged f58a6cf into c3aa982).

Contributor

andrewschoen commented Aug 16, 2012

Ok, so the last commit actually doesn't pass the '{% show_sub_menu 100 0 %} should start at level 0... so level 0 is displayed.' requirement. I'll work on that.

The more I think about it though, do we need to let them specify a level? It actually creates weird situations where no menu is displayed at all. For example, I pass 3 to it but I'm at level 1, I get no menu.

As an alternative we could just make root_level (named something else) just a boolean argument. If True, the menu would root at the root node of that tree. So, show every node after the root node for the active tree.

Contributor

andrewschoen commented Aug 17, 2012

ok, show_sub_menu 100 0 works as expected now. Disregard my previous suggestion, this way is better.

This pull request passes (merged a4b8d68 into c3aa982).

Member

digi604 commented Mar 28, 2013

please remerge so we can get this bugger in for 2.4

Member

digi604 commented Mar 28, 2013

and please update release notes :)

digi604 added a commit that referenced this pull request Apr 8, 2013

@digi604 digi604 merged commit a4b8d68 into divio:develop Apr 8, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment