Fix brand preview in admin branding settings page (#1783)

* style(brand preview): update size to fit properly; use new brand preview partial

* test: update tests

* chore: formatting

app/assets/javascripts/previewPane.js

@@ -23,7 +23,7 @@
     `<iframe src="/_${previewType}?${buildQueryString([
       "branding_style",
       branding_style,
-    ])}" class="branding-preview"></iframe>`,
+    ])}" class="branding-preview border-4 border-gray-200" width="600px" height="345px"></iframe>`,
   );
 
   function buildQueryString() {

app/main/views/index.py

@@ -11,6 +11,7 @@
     request,
     url_for,
 )
+from flask_babel import _
 from flask_login import current_user
 from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES
 from notifications_utils.template import HTMLEmailTemplate, LetterImageTemplate
@@ -153,61 +154,30 @@ def email_template():
 
     template = {
         "subject": "foo",
-        "content": (
-            "Lorem Ipsum is simply dummy text of the printing and typesetting "
-            "industry.\n\nLorem Ipsum has been the industry’s standard dummy "
-            "text ever since the 1500s, when an unknown printer took a galley "
-            "of type and scrambled it to make a type specimen book. "
-            "\n\n"
-            "# History"
-            "\n\n"
-            "It has "
-            "survived not only"
-            "\n\n"
-            "* five centuries"
-            "\n"
-            "* but also the leap into electronic typesetting"
-            "\n\n"
-            "It was "
-            "popularised in the 1960s with the release of Letraset sheets "
-            "containing Lorem Ipsum passages, and more recently with desktop "
-            "publishing software like Aldus PageMaker including versions of "
-            "Lorem Ipsum."
-            "\n\n"
-            "^ It is a long established fact that a reader will be distracted "
-            "by the readable content of a page when looking at its layout."
-            "\n\n"
-            "The point of using Lorem Ipsum is that it has a more-or-less "
-            "normal distribution of letters, as opposed to using ‘Content "
-            "here, content here’, making it look like readable English."
-            "\n\n\n"
-            "1. One"
-            "\n"
-            "2. Two"
-            "\n"
-            "10. Three"
-            "\n\n"
-            "This is an example of an email sent using Notification."
-            "\n\n"
-            "https://www.notifications.service.gov.uk"
+        "content": "# Email preview\n{}\n{}".format(
+            _("An example email showing the {} at the top left.").format(brand_name),
+            _("The canada wordmark is displayed at the bottom right"),
         ),
     }
 
     if not bool(request.args):
-        resp = make_response(str(HTMLEmailTemplate(template)))
+        resp = make_response(render_template("views/email-branding/_preview.html", template=str(HTMLEmailTemplate(template))))
     else:
         resp = make_response(
-            str(
-                HTMLEmailTemplate(
-                    template,
-                    fip_banner_english=fip_banner_english,
-                    fip_banner_french=fip_banner_french,
-                    brand_text=brand_text,
-                    brand_colour=brand_colour,
-                    brand_logo=brand_logo,
-                    logo_with_background_colour=logo_with_background_colour,
-                    brand_name=brand_name,
-                )
+            render_template(
+                "views/email-branding/_preview.html",
+                template=str(
+                    HTMLEmailTemplate(
+                        template,
+                        fip_banner_english=fip_banner_english,
+                        fip_banner_french=fip_banner_french,
+                        brand_text=brand_text,
+                        brand_colour=brand_colour,
+                        brand_logo=brand_logo,
+                        logo_with_background_colour=logo_with_background_colour,
+                        brand_name=brand_name,
+                    )
+                ),
             )
         )
 

app/templates/views/email-branding/_preview.html

@@ -1,5 +1,8 @@
 <div class="max-w-5xl mx-2">
   <div class="py-0 px-0 px-gutterHalf box-border border border-gray-300" id="template_preview">
+    {% if '/_email' in request.base_url %}
+      <link href="https://fonts.googleapis.com/css?family=Flow+Block&display=swap" rel="stylesheet" />
+    {% endif %}
     <template shadowrootmode="open">
 
       <style nonce="{{ request_nonce }}">

app/templates/views/service-settings/set-email-branding.html

@@ -16,11 +16,13 @@
     back_link=url_for('.service_settings', service_id=current_service.id)
   ) }}
   {% call form_wrapper(data_kwargs={'preview-type': 'email'}) %}
+
+    <h2 class="heading-medium">{{ _('Logo preview') }}</h2>
     <div class="grid-row contain-floats">
       <div class="w-full float-left py-0 px-0 px-gutterHalf box-border preview-pane">
       </div>
     </div>
-    <div class="grid-row contain-floats">
+    <div class="grid-row contain-floats mt-10">
       <div class="w-full float-left py-0 px-0 px-gutterHalf box-border">
         {{ live_search(target_selector='.multiple-choice', show=True, form=search_form, label='Search branding styles by name') }}
         {{ radios(form.branding_style) }}

tests/app/main/views/test_email_preview.py

@@ -7,12 +7,13 @@
 
 @pytest.mark.parametrize("query_args, result", [({}, True), ({"fip_banner_english": "false"}, "false")])
 def test_renders(client, mocker, query_args, result):
-    mocker.patch("app.main.views.index.HTMLEmailTemplate.__str__", return_value="rendered")
+    mocker.patch("app.main.views.index.HTMLEmailTemplate.__str__", return_value="rendered_content")
 
     response = client.get(url_for("main.email_template", **query_args))
+    page = BeautifulSoup(response.data.decode("utf-8"), "html.parser")
 
     assert response.status_code == 200
-    assert response.get_data(as_text=True) == "rendered"
+    assert "rendered_content" in str(page.contents)
 
 
 def test_displays_govuk_branding_by_default(client):
@@ -36,10 +37,7 @@ def test_displays_both_branding(client, mock_get_email_branding_with_both_brand_
 
     assert page.find("img", attrs={"src": re.compile("example.png$")})
 
-    assert (
-        page.select("body > table:nth-of-type(2) table > tr:nth-of-type(1) > td:nth-of-type(2)")[0].get_text().strip()
-        == "Organisation text"
-    )  # brand text is set
+    assert "Organisation text" in str(page.contents)  # brand text is set
 
 
 def test_displays_org_branding(client, mock_get_email_branding):
@@ -54,10 +52,8 @@ def test_displays_org_branding(client, mock_get_email_branding):
     assert not page.find("a", attrs={"href": "https://www.gov.uk"})
     assert page.find("img", attrs={"src": re.compile("example.png")})
     assert not page.select("body > table > tr > td[bgcolor='#f00']")  # banner colour is not set
-    assert (
-        page.select("body > table:nth-of-type(1) > tr:nth-of-type(1) > td:nth-of-type(1)")[0].get_text().strip()
-        == "Organisation text"
-    )  # brand text is set
+
+    assert "Organisation text" in str(page.contents)  # brand text is set
 
 
 def test_displays_org_branding_with_banner(client, mock_get_email_branding_with_custom_logo_with_background_colour_brand_type):
@@ -71,7 +67,7 @@ def test_displays_org_branding_with_banner(client, mock_get_email_branding_with_
     assert not page.find("a", attrs={"href": "https://www.gov.uk"})
     assert page.find("img", attrs={"src": re.compile("example.png")})
     assert page.select("body > table > tr > td[bgcolor='#f00']")  # banner colour is set
-    assert page.select("body > table table > tr > td > span")[0].get_text().strip() == "Organisation text"  # brand text is set
+    assert "Organisation text" in str(page.contents)  # brand text is set
 
 
 def test_displays_org_branding_with_banner_without_brand_text(client, mock_get_email_branding_without_brand_text):
@@ -86,4 +82,4 @@ def test_displays_org_branding_with_banner_without_brand_text(client, mock_get_e
     assert not page.find("a", attrs={"href": "https://www.gov.uk"})
     assert page.find("img", attrs={"src": re.compile("example.png")})
     assert page.select("body > table > tr > td[bgcolor='#f00']")  # banner colour is set
-    assert not page.select("body > table table > tr > td > span") == 0  # brand text is not set
+    assert "Organisation text" not in str(page.contents)  # brand text is set