From 8a7dd1f5a1272f3701cde369c3ea314baec1f2ca Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Thu, 14 Dec 2023 16:37:55 +0000 Subject: [PATCH] Don't use legacy header when breadcrumbs are used --- client/scss/components/_header.scss | 3 ++- .../templates/wagtailadmin/generic/base.html | 7 ++--- .../templates/wagtailadmin/generic/form.html | 12 +++++++++ .../templates/wagtailadmin/generic/index.html | 8 +++++- .../wagtailadmin/generic/listing.html | 4 ++- .../tests/viewsets/test_model_viewset.py | 26 +++++++++++++++--- wagtail/snippets/tests/test_snippets.py | 27 +++++++++---------- wagtail/snippets/tests/test_usage.py | 9 +++---- wagtail/snippets/tests/test_viewset.py | 22 ++++++++------- wagtail/snippets/views/snippets.py | 2 ++ 10 files changed, 81 insertions(+), 39 deletions(-) diff --git a/client/scss/components/_header.scss b/client/scss/components/_header.scss index 5f07ff6407..197031d0d0 100644 --- a/client/scss/components/_header.scss +++ b/client/scss/components/_header.scss @@ -11,7 +11,8 @@ margin: 0; } - h1 { + h1, + .w-header__title { @apply w-h1; position: relative; } diff --git a/wagtail/admin/templates/wagtailadmin/generic/base.html b/wagtail/admin/templates/wagtailadmin/generic/base.html index cef1557c02..e2c574ce3e 100644 --- a/wagtail/admin/templates/wagtailadmin/generic/base.html +++ b/wagtail/admin/templates/wagtailadmin/generic/base.html @@ -34,12 +34,13 @@ {% endfragment %} {# Ensure all necessary variables are passed explicitly here #} - {# TODO: Pass `title=header_title` once slim_header and main_header are mutually exclusive (to prevent multiple h1) #} - {% include "wagtailadmin/shared/headers/slim_header.html" with breadcrumbs_items=breadcrumbs_items side_panels=side_panels history_url=history_url search_url=index_results_url search_form=search_form filters=filters actions=actions only %} + {% include "wagtailadmin/shared/headers/slim_header.html" with breadcrumbs_items=breadcrumbs_items side_panels=side_panels history_url=history_url title=header_title search_url=index_results_url search_form=search_form filters=filters actions=actions only %} {% endif %} {% endblock %} {% block main_header %} - {% include "wagtailadmin/shared/header.html" with title=page_title subtitle=page_subtitle icon=header_icon only %} + {% if not breadcrumbs_items %} + {% include "wagtailadmin/shared/header.html" with title=page_title subtitle=page_subtitle icon=header_icon only %} + {% endif %} {% endblock %} {% endblock %} diff --git a/wagtail/admin/templates/wagtailadmin/generic/form.html b/wagtail/admin/templates/wagtailadmin/generic/form.html index 4f5173c8c8..30f728f563 100644 --- a/wagtail/admin/templates/wagtailadmin/generic/form.html +++ b/wagtail/admin/templates/wagtailadmin/generic/form.html @@ -2,6 +2,18 @@ {% load i18n wagtailadmin_tags %} {% block bodyclass %}editor-view{% endblock %} +{% block main_header %} + {% if breadcrumbs_items %} +
+

+ {{ page_subtitle }} +

+
+ {% else %} + {{ block.super }} + {% endif %} +{% endblock %} + {% block main_content %} {% block before_form %}{% endblock %}
diff --git a/wagtail/admin/templates/wagtailadmin/generic/index.html b/wagtail/admin/templates/wagtailadmin/generic/index.html index f95b7ac394..8e4f5d3a87 100644 --- a/wagtail/admin/templates/wagtailadmin/generic/index.html +++ b/wagtail/admin/templates/wagtailadmin/generic/index.html @@ -16,12 +16,18 @@ {% endif %} {% endblock %} {% endfragment %} - {% include "wagtailadmin/shared/header.html" with title=page_title subtitle=page_subtitle action_url=header_action_url action_text=header_action_label action_icon=header_action_icon base_actions=base_actions extra_actions=extra_actions icon=header_icon search_url=search_url search_form=search_form search_results_url=index_results_url only %} + {% if not breadcrumbs_items %} + {% include "wagtailadmin/shared/header.html" with title=page_title subtitle=page_subtitle action_url=header_action_url action_text=header_action_label action_icon=header_action_icon base_actions=base_actions extra_actions=extra_actions icon=header_icon search_url=search_url search_form=search_form search_results_url=index_results_url only %} + {% endif %} {% endblock %} {% block listing %}
+ {% if breadcrumbs_items and locale %} + {# TODO: temporarily put the locale selector here since we no longer use the legacy header #} + {% include 'wagtailadmin/shared/locale_selector.html' with theme="large" %} + {% endif %} {% include view.results_template_name|default:"wagtailadmin/generic/listing_results.html" %}
{% if filters and not breadcrumbs_items %} diff --git a/wagtail/admin/templates/wagtailadmin/generic/listing.html b/wagtail/admin/templates/wagtailadmin/generic/listing.html index 9943b2efb5..8626ba5459 100644 --- a/wagtail/admin/templates/wagtailadmin/generic/listing.html +++ b/wagtail/admin/templates/wagtailadmin/generic/listing.html @@ -2,7 +2,9 @@ {% load i18n %} {% block main_header %} - {% include "wagtailadmin/shared/header.html" with title=page_title subtitle=page_subtitle action_url=header_action_url action_text=header_action_label icon=header_icon only %} + {% if not breadcrumbs_items %} + {% include "wagtailadmin/shared/header.html" with title=page_title subtitle=page_subtitle action_url=header_action_url action_text=header_action_label icon=header_icon only %} + {% endif %} {% endblock %} {% block content %} diff --git a/wagtail/admin/tests/viewsets/test_model_viewset.py b/wagtail/admin/tests/viewsets/test_model_viewset.py index 7ce31d720c..5aaa524b67 100644 --- a/wagtail/admin/tests/viewsets/test_model_viewset.py +++ b/wagtail/admin/tests/viewsets/test_model_viewset.py @@ -135,12 +135,9 @@ class TestTemplateConfiguration(WagtailTestUtils, TestCase): response, "

Some extra custom content

", html=True ) - def test_wagtail_admin_template_mixin_variables(self): + def test_wagtail_admin_template_mixin_variables_with_legacy_header(self): pk = quote(self.custom.pk) cases = { - "index": ([], "Feature complete toys", None), - "add": ([], "New", "Feature complete toy"), - "edit": ([pk], "Editing", str(self.custom)), "delete": ([pk], "Delete", str(self.custom)), } for view_name, (args, title, subtitle) in cases.items(): @@ -161,6 +158,27 @@ class TestTemplateConfiguration(WagtailTestUtils, TestCase): icon = h1.select_one("svg use[href='#icon-media']") self.assertIsNotNone(icon) + def test_wagtail_admin_template_mixin_variables(self): + pk = quote(self.custom.pk) + cases = { + "index": ([], "Feature complete toys", None), + "add": ([], "New", "Feature complete toy"), + "edit": ([pk], "Editing", str(self.custom)), + } + for view_name, (args, title, subtitle) in cases.items(): + with self.subTest(view_name=view_name): + response = self.client.get(self.get_custom_url(view_name, args=args)) + soup = self.get_soup(response.content) + h1 = soup.select_one("h1") + expected_h1 = title + if subtitle: + expected_h1 = f"{title}: {subtitle}" + self.assertIsNotNone(h1) + self.assertEqual(h1.get_text(strip=True), expected_h1) + icon = h1.select_one("svg use[href='#icon-media']") + # Icon is no longer rendered in the h1 with the slim header in place + self.assertIsNone(icon) + class TestCustomColumns(WagtailTestUtils, TestCase): def setUp(self): diff --git a/wagtail/snippets/tests/test_snippets.py b/wagtail/snippets/tests/test_snippets.py index e03347cf0d..ea6f19e812 100644 --- a/wagtail/snippets/tests/test_snippets.py +++ b/wagtail/snippets/tests/test_snippets.py @@ -352,9 +352,9 @@ class TestLocaleSelectorOnList(WagtailTestUtils, TestCase): reverse("wagtailsnippets_snippetstests_translatablesnippet:add") + "?locale=en" ) - self.assertContains( - response, f'' - ) + soup = self.get_soup(response.content) + add_button = soup.select_one(f'a[href="{add_url}"]') + self.assertIsNotNone(add_button) self.assertContains( response, f'No translatable snippets have been created. Why not add one', @@ -370,9 +370,9 @@ class TestLocaleSelectorOnList(WagtailTestUtils, TestCase): # Check that the add URLs don't include the locale add_url = reverse("wagtailsnippets_snippetstests_translatablesnippet:add") - self.assertContains( - response, f'' - ) + soup = self.get_soup(response.content) + add_button = soup.select_one(f'a[href="{add_url}"]') + self.assertIsNotNone(add_button) self.assertContains( response, f'No translatable snippets have been created. Why not add one', @@ -385,9 +385,9 @@ class TestLocaleSelectorOnList(WagtailTestUtils, TestCase): # Check that the add URLs don't include the locale add_url = reverse("wagtailsnippets_tests_advert:add") - self.assertContains( - response, f'' - ) + soup = self.get_soup(response.content) + add_button = soup.select_one(f'a[href="{add_url}"]') + self.assertIsNotNone(add_button) self.assertContains( response, f'No adverts have been created. Why not add one', @@ -4271,11 +4271,10 @@ class TestSnippetHistory(WagtailTestUtils, TestCase): html=True, ) - # Should use the latest draft title in the header subtitle - self.assertContains( - response, - 'Draft-enabled Bar, In Draft', - ) + soup = self.get_soup(response.content) + sublabel = soup.select_one(".w-breadcrumbs__sublabel") + # Should use the latest draft title in the breadcrumbs sublabel + self.assertEqual(sublabel.get_text(strip=True), "Draft-enabled Bar, In Draft") @override_settings(WAGTAIL_I18N_ENABLED=True) def test_get_with_i18n_enabled(self): diff --git a/wagtail/snippets/tests/test_usage.py b/wagtail/snippets/tests/test_usage.py index 7f4f8322b8..4b390115f7 100644 --- a/wagtail/snippets/tests/test_usage.py +++ b/wagtail/snippets/tests/test_usage.py @@ -68,11 +68,10 @@ class TestSnippetUsageView(WagtailTestUtils, TestCase): ) ) - # Should use the latest draft title in the header subtitle - self.assertContains( - response, - 'Draft-enabled Bar, In Draft', - ) + soup = self.get_soup(response.content) + sublabel = soup.select_one(".w-breadcrumbs__sublabel") + # Should use the latest draft title in the breadcrumbs sublabel + self.assertEqual(sublabel.get_text(strip=True), "Draft-enabled Bar, In Draft") def test_usage(self): # resave so that usage count gets updated diff --git a/wagtail/snippets/tests/test_viewset.py b/wagtail/snippets/tests/test_viewset.py index 53a526472c..f692ce02cb 100644 --- a/wagtail/snippets/tests/test_viewset.py +++ b/wagtail/snippets/tests/test_viewset.py @@ -69,6 +69,8 @@ class BaseSnippetViewSetTests(WagtailTestUtils, TestCase): class TestCustomIcon(BaseSnippetViewSetTests): + # TODO: decide what to do with this test, since the new designs after + # Universal Listings and unified breadcrumbs/header don't have icons model = FullFeaturedSnippet def setUp(self): @@ -82,11 +84,13 @@ class TestCustomIcon(BaseSnippetViewSetTests): def test_get_views(self): pk = quote(self.object.pk) views = [ - ("list", []), + # TODO: Some of these views have been migrated to use the slim_header + # only, so there is no header_icon anymore. + # ("list", []), ("add", []), ("edit", [pk]), ("delete", [pk]), - ("usage", [pk]), + # ("usage", [pk]), ("unpublish", [pk]), ("workflow_history", [pk]), ("revisions_revert", [pk, self.revision_1.id]), @@ -99,18 +103,16 @@ class TestCustomIcon(BaseSnippetViewSetTests): self.assertEqual(response.status_code, 200) self.assertEqual(response.context["header_icon"], "cog") self.assertContains(response, "icon icon-cog", count=1) - # TODO: Make the list view use the shared header template - if view_name != "list": - self.assertTemplateUsed(response, "wagtailadmin/shared/header.html") + self.assertTemplateUsed(response, "wagtailadmin/shared/header.html") def test_get_history(self): response = self.client.get(self.get_url("history", [quote(self.object.pk)])) self.assertEqual(response.status_code, 200) - self.assertTemplateUsed(response, "wagtailadmin/shared/header.html") - # History view icon is not configurable for consistency with pages - self.assertEqual(response.context["header_icon"], "history") - self.assertContains(response, "icon icon-history") - self.assertNotContains(response, "icon icon-cog") + self.assertTemplateUsed( + response, + "wagtailadmin/shared/headers/slim_header.html", + ) + self.assertTemplateNotUsed(response, "wagtailadmin/shared/header.html") def test_get_workflow_history_detail(self): # Assign default workflow to the snippet model diff --git a/wagtail/snippets/views/snippets.py b/wagtail/snippets/views/snippets.py index d62ba87697..9ae13dd8cf 100644 --- a/wagtail/snippets/views/snippets.py +++ b/wagtail/snippets/views/snippets.py @@ -845,6 +845,7 @@ class SnippetViewSet(ModelViewSet): workflow_history_detail_url_name=self.get_url_name( "workflow_history_detail" ), + _show_breadcrumbs=False, ) @property @@ -858,6 +859,7 @@ class SnippetViewSet(ModelViewSet): object_icon=self.icon, header_icon="list-ul", workflow_history_url_name=self.get_url_name("workflow_history"), + _show_breadcrumbs=False, ) @property