Try out the statistical article hero wtih the hero component from the design system #91
Conversation
cms/core/templatetags/util_tags.py
Outdated
| @register.filter(name="breadcrumbs") | ||
| def breadcrumbs(page: "Page") -> list[dict[str, str]]: |
There was a problem hiding this comment.
suggestion
| @register.filter(name="breadcrumbs") | |
| def breadcrumbs(page: "Page") -> list[dict[str, str]]: | |
| @jinja2.pass_context | |
| def breadcrumbs(context: jinja2.runtime.Context, page: "Page") -> list[dict[str, str]]: |
then you can do .get_url(request=context.get("request")).
@register.filter(name="breadcrumbs") is for the Django template library, which we only use for the Wagtail admin
There was a problem hiding this comment.
Could you also update the breadcrumbs template to remove https://github.com/ONSdigital/dis-wagtail/blob/main/cms/jinja2/templates/components/navigation/breadcrumbs.html#L4-L14 and change https://github.com/ONSdigital/dis-wagtail/blob/main/cms/jinja2/templates/components/navigation/breadcrumbs.html#L19 to page|breadcrumbs ?
There was a problem hiding this comment.
Query re "@register.filter(name="breadcrumbs") is for the Django template library, which we only use for the Wagtail admin" - it seems to be being used for the social_image and social_text filters currently which are used on the front-end not in the admin.
There was a problem hiding this comment.
I think it would make sense to use the navigation app for the breadcrumbs like the ones in @sanjeevz3009's merge request so will update.
There was a problem hiding this comment.
it seems to be being used for the social_image and social_text filters currently which are used on the front-end not in the admin.
that was part of kit and it makes sense to clean them
There was a problem hiding this comment.
Happy to leave them if that makes sense - did you mean 'keep them' not 'clean them'?
There was a problem hiding this comment.
I will leave the breadcrumbs template for now and see what becomes of this pull request - it may be that we can get rid of it entirely once we're using the new heroes more consistently.
| {% set releases_text = _("View latest release") %} | ||
| {% endif %} | ||
| {% set next_release_date_text = page.next_release_date|date("DATE_FORMAT") or _("To be announced") %} | ||
| {% with breadcrumbs=page|breadcrumbs %} |
There was a problem hiding this comment.
I believe you can pass page|breadcrumbs directly to the macro.
| {% if page.is_census %} | ||
| {% import "templates/components/census/census-logo.html" as census_logo %} | ||
| {% set census_html = ['<div class="ons-container">'|safe, census_logo] | join %} | ||
| {% set census_html = [census_html, "</div>"|safe] | join %} | ||
| {% else %} | ||
| {% set census_html = "" %} | ||
| {% endif %} |
There was a problem hiding this comment.
you could do something like:
{% set census_html -%}
{% if page.is_census %}
<div class="ons-container">{% import "templates/components/census/census-logo.html"</div>
{% endif %}
{%- endset %}which should be much cleaner, IMHO
| page_depth = 2 | ||
| for ancestor_page in page.get_ancestors().specific().defer_streamfields(): | ||
| if not ancestor_page.is_root(): | ||
| if ancestor_page.depth <= page_depth: |
There was a problem hiding this comment.
| page_depth = 2 | |
| for ancestor_page in page.get_ancestors().specific().defer_streamfields(): | |
| if not ancestor_page.is_root(): | |
| if ancestor_page.depth <= page_depth: | |
| for ancestor_page in page.get_ancestors().specific().defer_streamfields(): | |
| if not ancestor_page.is_root(): | |
| # Root has depth 1, Homepage 2 | |
| if ancestor_page.depth <= 2: |
What is the context of this PR?
This is an initial test to try out the new grey hero on the statistical article page - relating to https://jira.ons.gov.uk/browse/CMS-307
It is very much draft and not intended to be merged yet - it shows the two hero blocks one above the other on the statistical article page, in order to highlight some of the styling and implementation issues with switching, which are going to be discussed with the design system team.
Pushing this as I would like some initial feedback on the change to move the breadcrumb logic to the back-end, which is necessary in order to use the
breadcrumbsoption in the hero.Update: breadcrumb filter is now moved to be a global function, and I have followed @sanjeevz3009's approach in #85 and created a
navigation_tagsfile.How to review
You can check this branch out to test locally if desired, but it is not intended to be tested - more here to get feedback on the breadcrumbs approach.
Follow-up Actions