Merge pull request #266 from alphagov/better-placeholder-guidance

Fix bugs and gripes with adding and editing templates

app/assets/stylesheets/_grids.scss

@@ -2,7 +2,6 @@
   @include grid-column(3/4);
 }
 
-
 .column-one-eighth {
   @include grid-column(1/8);
 }
@@ -19,3 +18,11 @@
 .bottom-gutter-2-3 {
   margin-bottom: $gutter * 2/3;
 }
+
+.align-with-heading {
+  display: block;
+  text-align: center;
+  margin-top: 45px;
+  padding-left: 2px;
+  padding-right: 2px;
+}

app/assets/stylesheets/views/edit-template.scss

@@ -3,7 +3,7 @@
   &-placeholder-hint {
     display: block;
     padding-top: 20px;
-    color: $secondary-text-colour;
+    //color: $secondary-text-colour;
   }
 
 }

app/main/views/send.py

@@ -37,8 +37,8 @@ send_messages_page_headings = {
 
 
 manage_templates_page_headings = {
-    'email': 'Manage templates',
-    'sms': 'Manage templates'
+    'email': 'Email templates',
+    'sms': 'Text message templates'
 }
 
 

app/main/views/templates.py

@@ -13,6 +13,11 @@ form_objects = {
     'sms': SMSTemplateForm
 }
 
+page_headings = {
+    'email': 'email',
+    'sms': 'text message'
+}
+
 
 @main.route("/services/<service_id>/templates/add-<template_type>", methods=['GET', 'POST'])
 @login_required
@@ -42,7 +47,8 @@ def add_service_template(service_id, template_type):
         'views/edit-{}-template.html'.format(template_type),
         form=form,
         template_type=template_type,
-        service_id=service_id
+        service_id=service_id,
+        heading_action='Add'
     )
 
 
@@ -70,7 +76,8 @@ def edit_service_template(service_id, template_id):
         form=form,
         service_id=service_id,
         template_id=template_id,
-        template_type=template['template_type']
+        template_type=template['template_type'],
+        heading_action='Edit'
     )
 
 

app/templates/main_nav.html

@@ -10,9 +10,9 @@
   </ul>
   {% elif current_user.has_permissions(['manage_templates']) %}
   <ul>
-    <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Message templates</a></li>
+    <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Text message templates</a></li>
     <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Email templates</a></li>
-    <li><a href="{{ url_for('.letters_stub', service_id=service_id) }}">Send letters</a></li>
+    <li><a href="{{ url_for('.letters_stub', service_id=service_id) }}">Letter templates</a></li>
   </ul>
   {% endif %}
   {% if current_user.has_permissions(['manage_users', 'manage_settings']) %}

app/templates/views/choose-template.html

@@ -10,55 +10,64 @@
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-large">{{ page_heading }}</h1>
+  {% if not templates %}
 
-  <form method="POST" enctype="multipart/form-data">
+    <h1 class="heading-large">{{ page_heading }}</h1>
 
-    {% if templates %}
-      {% if not has_jobs %}
-        {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters'], or_=True) %}
-        {{ banner(
-          """
-            Send yourself a test message
-          """,
-          subhead='Next step',
-          type="tip"
-        )}}
-        {% endif %}
-      {% endif %}
-      <div class="grid-row">
-        {% for template in templates %}
-          <div class="column-two-thirds">
-            {% if 'email' == template_type %}
-              {{ email_message(
-                template.subject,
-                template.formatted_as_markup,
-                name=template.name
-              ) }}
-            {% elif 'sms' == template_type %}
-              {{ sms_message(template.formatted_as_markup, name=template.name) }}
-            {% endif %}
-          </div>
-          <div class="column-one-third">
-            <div class="sms-message-use-links">
-              {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
-              <a href="{{ url_for(".send_messages", service_id=service_id, template_id=template.id) }}">Add recipients</a>
-              <a href="{{ url_for(".send_message_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
-              {% endif %}
-              {% if current_user.has_permissions(['manage_templates']) %}
-              <a href="{{ url_for(".edit_service_template", service_id=service_id, template_id=template.id) }}">Edit template</a>
-              {% endif %}
-            </div>
-          </div>
-        {% endfor %}
-      </div>
+    {% if current_user.has_permissions(['manage_templates']) %}
+      <a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button">Add a new template</a>
     {% endif %}
 
-    <p>
-      {% if current_user.has_permissions(['manage_templates']) %}
-      <a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button">Add a new template</a>
-      {% endif %}
-    </p>
+  {% else %}
+
+    <div class="grid-row">
+      <div class="column-two-thirds">
+        <h1 class="heading-large">{{ page_heading }}</h1>
+      </div>
+      {% if current_user.has_permissions(['manage_templates']) %}
+        <div class="column-one-third">
+          <a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button align-with-heading">Add new template</a>
+        </div>
+      {% endif %}
+    </div>
+
+    {% if not has_jobs %}
+      {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters'], or_=True) %}
+      {{ banner(
+        """
+          Send yourself a test message
+        """,
+        subhead='Next step',
+        type="tip"
+      )}}
+      {% endif %}
+    {% endif %}
+    <div class="grid-row">
+      {% for template in templates %}
+        <div class="column-two-thirds">
+          {% if 'email' == template_type %}
+            {{ email_message(
+              template.subject,
+              template.formatted_as_markup,
+              name=template.name
+            ) }}
+          {% elif 'sms' == template_type %}
+            {{ sms_message(template.formatted_as_markup, name=template.name) }}
+          {% endif %}
+        </div>
+        <div class="column-one-third">
+          <div class="sms-message-use-links">
+            {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
+            <a href="{{ url_for(".send_messages", service_id=service_id, template_id=template.id) }}">Add recipients</a>
+            <a href="{{ url_for(".send_message_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
+            {% endif %}
+            {% if current_user.has_permissions(['manage_templates']) %}
+            <a href="{{ url_for(".edit_service_template", service_id=service_id, template_id=template.id) }}">Edit template</a>
+            {% endif %}
+          </div>
+        </div>
+      {% endfor %}
+    </div>
+  {% endif %}
 
-  </form>
 {% endblock %}

app/templates/views/edit-email-template.html

@@ -3,12 +3,14 @@
 {% from "components/page-footer.html" import page_footer %}
 
 {% block page_title %}
-  {{ h1 }} – GOV.UK Notify
+  {{ heading_action }} email template – GOV.UK Notify
 {% endblock %}
 
 {% block maincolumn_content %}
 
-    <h1 class="heading-large">Edit email template</h1>
+    <h1 class="heading-large">
+      {{ heading_action }} email template
+    </h1>
 
     <form method="post">
       <div class="grid-row">
@@ -21,17 +23,19 @@
         </div>
         <div class="column-one-third">
           <label for='template_content' class='edit-template-placeholder-hint'>
-            Add placeholders using double brackets, eg Your thing
-            is due on ((date))
+            <p>
+              Add personalisation with double brackets, eg Dear ((name))
+            </p>
+            <p>
+              You’ll fill in the real data when you send a message.
+            </p>
           </label>
         </div>
       </div>
       {{ page_footer(
         'Save',
         delete_link=url_for('.delete_service_template', service_id=service_id, template_id=template_id) if template_id or None,
-        delete_link_text='Delete this template',
-        back_link=url_for('.choose_template', template_type=template_type, service_id=service_id),
-        back_link_text='Cancel'
+        delete_link_text='Delete this template'
       ) }}
     </form>
 

app/templates/views/edit-sms-template.html

@@ -3,12 +3,14 @@
 {% from "components/page-footer.html" import page_footer %}
 
 {% block page_title %}
-  {{ h1 }} – GOV.UK Notify
+  {{ heading_action }} text message template – GOV.UK Notify
 {% endblock %}
 
 {% block maincolumn_content %}
 
-    <h1 class="heading-large">Edit text message template</h1>
+    <h1 class="heading-large">
+      {{ heading_action }} text message template
+    </h1>
 
     <form method="post">
       <div class="grid-row">
@@ -20,17 +22,19 @@
         </div>
         <div class="column-one-third">
           <label for='template_content' class='edit-template-placeholder-hint'>
-            Add placeholders using double brackets, eg Your thing
-            is due on ((date))
+            <p>
+              Add personalisation with double brackets, eg Dear ((name))
+            </p>
+            <p>
+              You'll have to add the real data when you send a message.
+            </p>
           </label>
         </div>
       </div>
       {{ page_footer(
         'Save',
         delete_link=url_for('.delete_service_template', service_id=service_id, template_id=template_id) if template_id or None,
-        delete_link_text='Delete this template',
-        back_link=url_for('.choose_template', service_id=service_id, template_type=template_type),
-        back_link_text='Cancel'
+        delete_link_text='Delete this template'
       ) }}
     </form>
 

app/templates/views/invite-user.html

@@ -10,7 +10,7 @@ Manage users – GOV.UK Notify
 {% block maincolumn_content %}
 
   <h1 class="heading-large">
-    {{ user.name or user.email_localpart or "Add a new team member" }}
+    {{ user.name or user.email_localpart or "Invite a team member" }}
   </h1>
 
   <div class="grid-row">

app/templates/views/letters.html

@@ -7,7 +7,11 @@
 {% block maincolumn_content %}
 
   <h1 class="heading-large">
-    Send letters
+    {% if current_user.has_permissions(['send_letters']) %}
+      Send letters
+    {% else %}
+      Letter templates
+    {% endif %}
   </h1>
 
   <p>

app/templates/views/manage-users.html

@@ -16,11 +16,16 @@ Manage users – GOV.UK Notify
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-large">
-    Manage team
-  </h1>
-
-  <a href="{{ url_for('.invite_user', service_id=service_id) }}" class="button">Invite a team member</a>
+  <div class="grid-row">
+    <div class="column-two-thirds">
+      <h1 class="heading-large">
+        Manage team
+      </h1>
+    </div>
+    <div class="column-one-third">
+      <a href="{{ url_for('.invite_user', service_id=service_id) }}" class="button align-with-heading">Invite team member</a>
+    </div>
+  </div>
 
   {% call(item) list_table(
     users, caption='Active', **table_options

app/templates/views/service-settings.html

@@ -7,7 +7,7 @@
 
 {% block maincolumn_content %}
 
-    <h1 class="heading-large">Service settings</h1>
+    <h1 class="heading-large">Manage settings</h1>
 
     {{ browse_list([
       {

tests/app/main/views/test_manage_users.py

@@ -123,7 +123,7 @@ def test_should_show_page_for_inviting_user(
             client.login(api_user_active)
             response = client.get(url_for('main.invite_user', service_id=55555))
 
-        assert 'Add a new team member' in response.get_data(as_text=True)
+        assert 'Invite a team member' in response.get_data(as_text=True)
         assert response.status_code == 200
 
 
@@ -277,6 +277,6 @@ def test_user_cant_invite_themselves(
 
         assert response.status_code == 200
         page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-        assert page.h1.string.strip() == 'Add a new team member'
+        assert page.h1.string.strip() == 'Invite a team member'
         form_error = page.find('span', class_='error-message').string.strip()
         assert form_error == "You can't send an invitation to yourself"