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

show current page in primary nav #133

Merged
merged 1 commit into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions cdhweb/pages/templatetags/core_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
from django.template.loader import render_to_string
from django.utils import timezone

from cdhweb.pages.snippets import Footer, PrimaryNavigation, SecondaryNavigation, SiteAlert
from cdhweb.pages.snippets import (
Footer,
PrimaryNavigation,
SecondaryNavigation,
SiteAlert,
)

register = template.Library()

Expand Down Expand Up @@ -77,14 +82,22 @@ def _get_primary_nav_items():
return l1_menu_items


@register.simple_tag()
def primary_nav_dict():
@register.simple_tag(takes_context=True)
def primary_nav_dict(context):
"""
Return the primary navigation data as a dict, for use with the 'json_script' filter.
"""
primary_nav_items = _get_primary_nav_items()
l1_menu_item_data = [_l1_item_to_dict(item) for item in primary_nav_items]

current_path = context["request"].path

for item in l1_menu_item_data:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to do this, given we've used a different approach in the template?

Copy link

@liamjohnston liamjohnston Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're asking whether we need it in both the template and the JSON blob, then the answer is yes :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. In my head we were using the same data for both

if current_path.startswith(item["link_url"]):
item["is_current"] = True
else:
item["is_current"] = False

primary_nav_data = {
"primary_nav": {
"l1_menu_items": l1_menu_item_data,
Expand Down Expand Up @@ -187,4 +200,14 @@ def site_alerts(context):
.exclude(display_until__lt=now)
)
data = {"site_alerts": site_alerts, "request": context.get("request")}
return data
return data


@register.filter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wild that this isn't a default tag (given some of the things that are)

def starts_with(value, arg):
"""
Usage, {% if value|starts_with:"arg" %}
"""
if value:
return value.startswith(arg)
return None
2 changes: 1 addition & 1 deletion templates/includes/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
<ul class="main-nav-desktop__list">
{% for item in primary_nav.l1_menu_items %}
<li>
<a href="{{ item.link_url }}" class="main-nav-desktop__item">
<a href="{{ item.link_url }}" class="main-nav-desktop__item {% if request.path|starts_with:item.link_url %} active {% endif %}" >
<span>{{ item.title }}</span>
{# Reserve space for dropdown icon, to stop jank when the React version replaces this version. #}
{% if item.l2_items.all %}
Expand Down
Loading