Skip to content
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

[Closes #4249] Contribute back upstream menu management from IGAD #4250

Merged
merged 4 commits into from
Feb 27, 2019

Conversation

afabiani
Copy link
Member

In this PR the menu management system implemented for IGAD has been generalized and simplified.

Issue: #4249

Three new models have been added in the geonode 'base' app model python module:

  • MenuPlaceholder - a logical container for menus
  • Menu - the link container
  • Menu item - internal or external links shown in each menu

Two custom template tags have been implemented for the menus rendering in the templates.

The get_menu assignment tag takes the placeholder name as argument and return, as output, a dictionary like this:
{ <Menu: menu_0>: <QuerySet [<MenuItem: menu_item_0>, <MenuItem: menu_item_1>, ... ]>, ... }
It can be used everywhere (for example to perfom a dropdown menu or an accordion panel) following these steps:
1 - load the custom tag {% load base_tags %}
2 - call the tag {% get_menu '<placeholder_name>' %}

The render_nav_menu inclusion tag does the same job but with a predefined html rendering (geonode/base/templates/menu.html). It is prepared for being a navbar dropdown menu without any further front-end implementation needed.
It should be called inside the extra_tab block of the 'base.html' template:
{% block extra_tab %}
{% render_nav_menu '<placeholder_name>' %}
{% endblock extra_tab %}

No rendering options have been implemented yet.
No bdd test implemented.

Unittest in geonode/base/tests.py.

@afabiani
Copy link
Member Author

Smoke tests are temporarily broken, they will be fixed once we merge this one
#4240

@capooti
Copy link
Member

capooti commented Feb 22, 2019

it is interesting, though I'd prefer to see this as a contribute module

@francbartoli
Copy link
Member

I guess this would affect the template for additional nav menus on geonode-project also. I'm inclined to agree with Paolo for a contrib app

@afabiani
Copy link
Member Author

@capooti @francbartoli by default it does not affect the templates, in fact in order to add a new menu item you would need to both:

  1. Create a placeholder key on admin
  2. Add the placeholder on the template

This PR proposes, along with documentation explaining how to add more, at leas one placeholder already created on base.html called 'TOPBAR_MENU', just to avoid that standard users not haveing carefully read the documentation, will try to add menus from admin without seeing the outcomes.

Nevertheless, we can remove the default placeholder from base.html and fixtures.

That said, if you still prefer moving this to a contrib module, no problems.

Can you please cast your votes?

@francbartoli
Copy link
Member

@afabiani In such case +0 for me. It can also stay in the code base

@afabiani afabiani merged commit 9292659 into master Feb 27, 2019
@afabiani afabiani deleted the GNIP_4249 branch February 27, 2019 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants