From bcf6b6da772e5f484b1b9254cde5acd6cef30bda Mon Sep 17 00:00:00 2001
From: LB <mail@lb.ee>
Date: Thu, 16 Nov 2017 00:58:50 +0700
Subject: [PATCH] Fixes breadcrumbs on ModelAdmin inspect and choose parent
 view (#4029)

---
 CHANGELOG.txt                                 |  1 +
 docs/releases/2.2.rst                         |  1 +
 .../templates/wagtailadmin/shared/header.html |  1 +
 .../scss/breadcrumbs_page.scss                | 11 +++++
 .../templates/modeladmin/choose_parent.html   | 13 +++---
 .../includes/header_with_breadcrumb.html      |  5 +++
 .../templates/modeladmin/inspect.html         | 13 +++---
 .../modeladmin/tests/test_page_modeladmin.py  | 44 +++++++++++++++++++
 .../tests/test_simple_modeladmin.py           | 28 ++++++++++++
 9 files changed, 105 insertions(+), 12 deletions(-)
 create mode 100644 wagtail/contrib/modeladmin/static_src/wagtailmodeladmin/scss/breadcrumbs_page.scss
 create mode 100644 wagtail/contrib/modeladmin/templates/modeladmin/includes/header_with_breadcrumb.html

diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index a85bd8da67..0fbeed30ce 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -8,6 +8,7 @@ Changelog
  * Added `annotate_score` support to PostgreSQL search backend (Bertrand Bordage)
  * Pillow's image optimisation is now applied when saving PNG images (Dmitry Vasilev)
  * Fix: Handle all exceptions from `Image.get_file_size` (Andrew Plummer)
+ * Fix: Fix display of breadcrumbs in ModelAdmin (LB (Ben Johnston))
 
 
 2.1 (22.05.2018)
diff --git a/docs/releases/2.2.rst b/docs/releases/2.2.rst
index ff6b17c107..b76d6dfb32 100644
--- a/docs/releases/2.2.rst
+++ b/docs/releases/2.2.rst
@@ -21,6 +21,7 @@ Bug fixes
 ~~~~~~~~~
 
  * Handle all exceptions from ``Image.get_file_size`` (Andrew Plummer)
+ * Fix display of breadcrumbs in ModelAdmin (LB (Ben Johnston))
 
 Upgrade considerations
 ======================
diff --git a/wagtail/admin/templates/wagtailadmin/shared/header.html b/wagtail/admin/templates/wagtailadmin/shared/header.html
index e927bb03e9..c822520e3a 100644
--- a/wagtail/admin/templates/wagtailadmin/shared/header.html
+++ b/wagtail/admin/templates/wagtailadmin/shared/header.html
@@ -18,6 +18,7 @@
     add_text - text for the 'add' button
 {% endcomment %}
 <header class="{% if merged %}merged{% endif %} {% if tabbed %}tab-merged{% endif %} {% if search_form %}hasform{% endif %}">
+    {% block breadcrumb %}{% endblock %}
     <div class="row nice-padding">
         <div class="left">
             <div class="col header-title">
diff --git a/wagtail/contrib/modeladmin/static_src/wagtailmodeladmin/scss/breadcrumbs_page.scss b/wagtail/contrib/modeladmin/static_src/wagtailmodeladmin/scss/breadcrumbs_page.scss
new file mode 100644
index 0000000000..72826de561
--- /dev/null
+++ b/wagtail/contrib/modeladmin/static_src/wagtailmodeladmin/scss/breadcrumbs_page.scss
@@ -0,0 +1,11 @@
+@import 'wagtailadmin/scss/variables';
+
+.breadcrumb {
+    margin: -1.2em 0 2em;
+}
+
+@media screen and (min-width: $breakpoint-mobile) {
+    .breadcrumb {
+        margin-top: -1.8em;
+    }
+}
diff --git a/wagtail/contrib/modeladmin/templates/modeladmin/choose_parent.html b/wagtail/contrib/modeladmin/templates/modeladmin/choose_parent.html
index 59d20d0f31..1bc3b000e7 100644
--- a/wagtail/contrib/modeladmin/templates/modeladmin/choose_parent.html
+++ b/wagtail/contrib/modeladmin/templates/modeladmin/choose_parent.html
@@ -1,11 +1,12 @@
 {% extends "wagtailadmin/base.html" %}
-{% load i18n admin_static %}
+{% load i18n modeladmin_tags admin_static %}
 
 {% block titletag %}{{ view.get_meta_title }}{% endblock %}
 
 {% block extra_css %}
     {% include "wagtailadmin/pages/_editor_css.html" %}
     <link rel="stylesheet" href="{% static 'wagtailmodeladmin/css/choose_parent_page.css' %}" type="text/css"/>
+    <link rel="stylesheet" href="{% static 'wagtailmodeladmin/css/breadcrumbs_page.css' %}" type="text/css"/>
 {% endblock %}
 
 {% block extra_js %}
@@ -14,12 +15,12 @@
 
 
 {% block content %}
-<div>
 
-    {% block header %}
-        {% include "modeladmin/includes/breadcrumb.html" %}
-        {% include "wagtailadmin/shared/header.html" with title=view.get_page_title subtitle=view.get_page_subtitle icon=view.header_icon %}
-    {% endblock %}
+{% block header %}
+    {% include "modeladmin/includes/header_with_breadcrumb.html" with title=view.get_page_title subtitle=view.get_page_subtitle icon=view.header_icon tabbed=True %}
+{% endblock %}
+
+<div>
 
     <div class="nice-padding">
         <h2>{% blocktrans %}Choose a parent page{% endblocktrans %}</h2>
diff --git a/wagtail/contrib/modeladmin/templates/modeladmin/includes/header_with_breadcrumb.html b/wagtail/contrib/modeladmin/templates/modeladmin/includes/header_with_breadcrumb.html
new file mode 100644
index 0000000000..5725979eed
--- /dev/null
+++ b/wagtail/contrib/modeladmin/templates/modeladmin/includes/header_with_breadcrumb.html
@@ -0,0 +1,5 @@
+{% extends "wagtailadmin/shared/header.html" %}
+
+{% block breadcrumb %}
+    {% include "modeladmin/includes/breadcrumb.html" %}
+{% endblock %}
diff --git a/wagtail/contrib/modeladmin/templates/modeladmin/inspect.html b/wagtail/contrib/modeladmin/templates/modeladmin/inspect.html
index e42ae16840..ba8d876d9f 100644
--- a/wagtail/contrib/modeladmin/templates/modeladmin/inspect.html
+++ b/wagtail/contrib/modeladmin/templates/modeladmin/inspect.html
@@ -1,10 +1,11 @@
 {% extends "wagtailadmin/base.html" %}
-{% load i18n %}
+{% load i18n admin_static %}
 
 {% block titletag %}{{ view.get_meta_title }}{% endblock %}
 
 {% block extra_css %}
     {{ view.media.css }}
+    <link rel="stylesheet" href="{% static 'wagtailmodeladmin/css/breadcrumbs_page.css' %}" type="text/css"/>
 {% endblock %}
 
 {% block extra_js %}
@@ -12,12 +13,12 @@
 {% endblock %}
 
 {% block content %}
-    <div>
 
-        {% block header %}
-            {% include "modeladmin/includes/breadcrumb.html" %}
-            {% include "wagtailadmin/shared/header.html" with title=view.get_page_title subtitle=view.get_page_subtitle icon=view.header_icon %}
-        {% endblock %}
+    {% block header %}
+        {% include "modeladmin/includes/header_with_breadcrumb.html" with title=view.get_page_title subtitle=view.get_page_subtitle icon=view.header_icon tabbed=True %}
+    {% endblock %}
+
+    <div>
 
         {% block content_main %}
             <div class="nice-padding">
diff --git a/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py
index 3def1d606a..7bb67346a6 100644
--- a/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py
+++ b/wagtail/contrib/modeladmin/tests/test_page_modeladmin.py
@@ -310,3 +310,47 @@ class TestModeratorAccess(TestCase):
     def test_delete_permitted(self):
         response = self.client.get('/admin/tests/eventpage/delete/4/')
         self.assertEqual(response.status_code, self.expected_status_code)
+
+
+class TestHeaderBreadcrumbs(TestCase, WagtailTestUtils):
+    """
+        Test that the <ul class="breadcrumbs">... is inserted within the
+        <header> tag for potential future regression.
+        See https://github.com/wagtail/wagtail/issues/3889
+    """
+    fixtures = ['test_specific.json']
+
+    def setUp(self):
+        self.login()
+
+    def test_choose_parent_page(self):
+        response = self.client.get('/admin/tests/eventpage/choose_parent/')
+
+        # check correct templates were used
+        self.assertTemplateUsed(response, 'modeladmin/includes/breadcrumb.html')
+        self.assertTemplateUsed(response, 'wagtailadmin/shared/header.html')
+
+        # check that home breadcrumb link exists
+        self.assertContains(response, '<li class="home"><a href="/admin/" class="icon icon-home text-replace">Home</a></li>', html=True)
+
+        # check that the breadcrumbs are after the header opening tag
+        content_str = str(response.content)
+        position_of_header = content_str.index('<header')  # intentionally not closing tag
+        position_of_breadcrumbs = content_str.index('<ul class="breadcrumb">')
+        self.assertLess(position_of_header, position_of_breadcrumbs)
+
+    def test_choose_inspect_page(self):
+        response = self.client.get('/admin/tests/eventpage/inspect/4/')
+
+        # check correct templates were used
+        self.assertTemplateUsed(response, 'modeladmin/includes/breadcrumb.html')
+        self.assertTemplateUsed(response, 'wagtailadmin/shared/header.html')
+
+        # check that home breadcrumb link exists
+        self.assertContains(response, '<li class="home"><a href="/admin/" class="icon icon-home text-replace">Home</a></li>', html=True)
+
+        # check that the breadcrumbs are after the header opening tag
+        content_str = str(response.content)
+        position_of_header = content_str.index('<header')  # intentionally not closing tag
+        position_of_breadcrumbs = content_str.index('<ul class="breadcrumb">')
+        self.assertLess(position_of_header, position_of_breadcrumbs)
diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py
index 52fc99ddc1..f199175f87 100644
--- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py
+++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py
@@ -486,3 +486,31 @@ class TestQuoting(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         response = self.client.get('/admin/modeladmintest/token/delete/Irregular_5FName/')
         self.assertEqual(response.status_code, 200)
+
+
+class TestHeaderBreadcrumbs(TestCase, WagtailTestUtils):
+    """
+        Test that the <ul class="breadcrumbs">... is inserted within the
+        <header> tag for potential future regression.
+        See https://github.com/wagtail/wagtail/issues/3889
+    """
+    fixtures = ['modeladmintest_test.json']
+
+    def setUp(self):
+        self.login()
+
+    def test_choose_inspect_model(self):
+        response = self.client.get('/admin/modeladmintest/author/inspect/1/')
+
+        # check correct templates were used
+        self.assertTemplateUsed(response, 'modeladmin/includes/breadcrumb.html')
+        self.assertTemplateUsed(response, 'wagtailadmin/shared/header.html')
+
+        # check that home breadcrumb link exists
+        self.assertContains(response, '<li class="home"><a href="/admin/" class="icon icon-home text-replace">Home</a></li>', html=True)
+
+        # check that the breadcrumbs are before the first header closing tag
+        content_str = str(response.content)
+        position_of_header_close = content_str.index('</header>')
+        position_of_breadcrumbs = content_str.index('<ul class="breadcrumb">')
+        self.assertGreater(position_of_header_close, position_of_breadcrumbs)