From 3617f2e936aadbdf71d582e58d88a16629ee5ca2 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 1 Feb 2016 11:26:38 +0000
Subject: [PATCH] Move service and user nav to proposition header
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit moves user-related navigation into the proposition header (the black
bar) at the top of the site. It adds some custom SASS to override GOV.UK
template and align these navigation items to the right (because it looks
better).

It then removes the service chooser dropdown (and its associated SASS and JS) in
favour of a link alongside the user-related navigation items. ‘Switch service’
is the best language for this that we’ve come up with so far.

This means that the only way of adding a new service is from the `/services`
page. So this commit removes the redirect if you land on this page with only one
service (else it would prevent you from ever being able to add more).
---
 app/assets/javascripts/detailsPolyfill.js     | 199 ------------------
 app/assets/stylesheets/app.scss               |  18 +-
 .../stylesheets/components/dropdown.scss      |  32 ---
 .../components/management-navigation.scss     |  36 ----
 .../stylesheets/components/navigation.scss    |  12 +-
 app/assets/stylesheets/main.scss              |   2 -
 app/main/views/add_service.py                 |   2 +-
 app/main/views/choose_service.py              |   6 -
 app/templates/admin_template.html             |  40 ++--
 app/templates/main_nav.html                   |   3 +
 gulpfile.babel.js                             |   1 -
 tests/app/main/views/test_add_service.py      |   2 +-
 12 files changed, 45 insertions(+), 308 deletions(-)
 delete mode 100644 app/assets/javascripts/detailsPolyfill.js
 delete mode 100644 app/assets/stylesheets/components/dropdown.scss
 delete mode 100644 app/assets/stylesheets/components/management-navigation.scss

diff --git a/app/assets/javascripts/detailsPolyfill.js b/app/assets/javascripts/detailsPolyfill.js
deleted file mode 100644
index d1e5a3ae9..000000000
--- a/app/assets/javascripts/detailsPolyfill.js
+++ /dev/null
@@ -1,199 +0,0 @@
-// Copied from:
-// https://github.com/alphagov/govuk_elements/blob/4926897dc7734db2fc5e5ebb6acdc97f86e22e50/public/javascripts/vendor/details.polyfill.js
-
-// When this is moved to GOV.UK Frontend Toolkit, we should import it from there
-// instead
-
-// <details> polyfill
-// http://caniuse.com/#feat=details
-
-// FF Support for HTML5's <details> and <summary>
-// https://bugzilla.mozilla.org/show_bug.cgi?id=591737
-
-// http://www.sitepoint.com/fixing-the-details-element/
-
-(function () {
-  'use strict';
-
-  var NATIVE_DETAILS = typeof document.createElement('details').open === 'boolean';
-
-  // Add event construct for modern browsers or IE
-  // which fires the callback with a pre-converted target reference
-  function addEvent(node, type, callback) {
-    if (node.addEventListener) {
-      node.addEventListener(type, function (e) {
-        callback(e, e.target);
-      }, false);
-    } else if (node.attachEvent) {
-      node.attachEvent('on' + type, function (e) {
-        callback(e, e.srcElement);
-      });
-    }
-  }
-
-  // Handle cross-modal click events
-  function addClickEvent(node, callback) {
-    // Prevent space(32) from scrolling the page
-    addEvent(node, 'keypress', function (e, target) {
-      if (target.nodeName === 'SUMMARY') {
-        if (e.keyCode === 32) {
-          if (e.preventDefault) {
-            e.preventDefault();
-          } else {
-            e.returnValue = false;
-          }
-        }
-      }
-    });
-    // When the key comes up - check if it is enter(13) or space(32)
-    addEvent(node, 'keyup', function (e, target) {
-      if (e.keyCode === 13 || e.keyCode === 32) { callback(e, target); }
-    });
-    addEvent(node, 'mouseup', function (e, target) {
-      callback(e, target);
-    });
-  }
-
-  // Get the nearest ancestor element of a node that matches a given tag name
-  function getAncestor(node, match) {
-    do {
-      if (!node || node.nodeName.toLowerCase() === match) {
-        break;
-      }
-    } while (node = node.parentNode);
-
-    return node;
-  }
-
-  // Create a started flag so we can prevent the initialisation
-  // function firing from both DOMContentLoaded and window.onload
-  var started = false;
-
-  // Initialisation function
-  function addDetailsPolyfill(list) {
-
-    // If this has already happened, just return
-    // else set the flag so it doesn't happen again
-    if (started) {
-      return;
-    }
-    started = true;
-
-    // Get the collection of details elements, but if that's empty
-    // then we don't need to bother with the rest of the scripting
-    if ((list = document.getElementsByTagName('details')).length === 0) {
-      return;
-    }
-
-    // else iterate through them to apply their initial state
-    var n = list.length, i = 0;
-    for (i; i < n; i++) {
-      var details = list[i];
-
-      // Save shortcuts to the inner summary and content elements
-      details.__summary = details.getElementsByTagName('summary').item(0);
-      details.__content = details.getElementsByTagName('div').item(0);
-
-      // If the content doesn't have an ID, assign it one now
-      // which we'll need for the summary's aria-controls assignment
-      if (!details.__content.id) {
-        details.__content.id = 'details-content-' + i;
-      }
-
-      // Add ARIA role="group" to details
-      details.setAttribute('role', 'group');
-
-      // Add role=button to summary
-      details.__summary.setAttribute('role', 'button');
-
-      // Add aria-controls
-      details.__summary.setAttribute('aria-controls', details.__content.id);
-
-      // Set tabIndex so the summary is keyboard accessible for non-native elements
-      // http://www.saliences.com/browserBugs/tabIndex.html
-      if (!NATIVE_DETAILS) {
-        details.__summary.tabIndex = 0;
-      }
-
-      // Detect initial open state
-      var openAttr = details.getAttribute('open') !== null;
-      if (openAttr === true) {
-        details.__summary.setAttribute('aria-expanded', 'true');
-        details.__content.setAttribute('aria-hidden', 'false');
-      } else {
-        details.__summary.setAttribute('aria-expanded', 'false');
-        details.__content.setAttribute('aria-hidden', 'true');
-        if (!NATIVE_DETAILS) {
-          details.__content.style.display = 'none';
-        }
-      }
-
-      // Create a circular reference from the summary back to its
-      // parent details element, for convenience in the click handler
-      details.__summary.__details = details;
-
-      // If this is not a native implementation, create an arrow
-      // inside the summary
-      if (!NATIVE_DETAILS) {
-
-        var twisty = document.createElement('i');
-
-        if (openAttr === true) {
-          twisty.className = 'arrow arrow-open';
-          twisty.appendChild(document.createTextNode('\u25bc'));
-        } else {
-          twisty.className = 'arrow arrow-closed';
-          twisty.appendChild(document.createTextNode('\u25ba'));
-        }
-
-        details.__summary.__twisty = details.__summary.insertBefore(twisty, details.__summary.firstChild);
-        details.__summary.__twisty.setAttribute('aria-hidden', 'true');
-
-      }
-    }
-
-    // Define a statechange function that updates aria-expanded and style.display
-    // Also update the arrow position
-    function statechange(summary) {
-
-      var expanded = summary.__details.__summary.getAttribute('aria-expanded') === 'true';
-      var hidden = summary.__details.__content.getAttribute('aria-hidden') === 'true';
-
-      summary.__details.__summary.setAttribute('aria-expanded', (expanded ? 'false' : 'true'));
-      summary.__details.__content.setAttribute('aria-hidden', (hidden ? 'false' : 'true'));
-
-      if (!NATIVE_DETAILS) {
-        summary.__details.__content.style.display = (expanded ? 'none' : '');
-
-        var hasOpenAttr = summary.__details.getAttribute('open') !== null;
-        if (!hasOpenAttr) {
-          summary.__details.setAttribute('open', 'open');
-        } else {
-          summary.__details.removeAttribute('open');
-        }
-      }
-
-      if (summary.__twisty) {
-        summary.__twisty.firstChild.nodeValue = (expanded ? '\u25ba' : '\u25bc');
-        summary.__twisty.setAttribute('class', (expanded ? 'arrow arrow-closed' : 'arrow arrow-open'));
-      }
-
-      return true;
-    }
-
-    // Bind a click event to handle summary elements
-    addClickEvent(document, function(e, summary) {
-      if (!(summary = getAncestor(summary, 'summary'))) {
-        return true;
-      }
-      return statechange(summary);
-    });
-  }
-
-  // Bind two load events for modern and older browsers
-  // If the first one fires it will set a flag to block the second one
-  // but if it's not supported then the second one will fire
-  addEvent(document, 'DOMContentLoaded', addDetailsPolyfill);
-  addEvent(window, 'load', addDetailsPolyfill);
-
-})();
diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss
index d52635054..90771c9cb 100644
--- a/app/assets/stylesheets/app.scss
+++ b/app/assets/stylesheets/app.scss
@@ -21,8 +21,12 @@
   }
 }
 
+.phase-banner-beta {
+  display: inline-block;
+}
+
 .phase-tag {
-    @include phase-tag(beta);
+  @include phase-tag(beta);
 }
 
 @media (min-width: 641px) {
@@ -44,6 +48,18 @@
   white-space: nowrap;
 }
 
+@include media(desktop) {
+
+  #proposition-menu {
+    float: right;
+  }
+
+  #global-header .header-proposition #proposition-links li {
+    padding: 0 0 0 15px;
+  }
+
+}
+
 a:visited {
   color: $link-colour;
 }
diff --git a/app/assets/stylesheets/components/dropdown.scss b/app/assets/stylesheets/components/dropdown.scss
deleted file mode 100644
index 7f87f0c49..000000000
--- a/app/assets/stylesheets/components/dropdown.scss
+++ /dev/null
@@ -1,32 +0,0 @@
-%dropdown,
-.dropdown {
-
-  position: relative;
-
-  &-toggle {
-
-    color: $link-colour;
-    position: relative;
-    cursor: pointer;
-    outline-offset: 2px;
-
-    &:hover {
-      color: $link-hover-colour;
-      text-decoration: underline;
-    }
-
-  }
-
-  a {
-
-    display: block;
-    margin-top: 7px;
-    padding-left: 20px;
-
-    &:hover {
-      text-decoration: underline;
-    }
-
-  }
-
-}
diff --git a/app/assets/stylesheets/components/management-navigation.scss b/app/assets/stylesheets/components/management-navigation.scss
deleted file mode 100644
index d0e3f97a6..000000000
--- a/app/assets/stylesheets/components/management-navigation.scss
+++ /dev/null
@@ -1,36 +0,0 @@
-.management-navigation {
-
-  @extend %site-width-container;
-  @include core-16();
-  border-bottom: 1px solid $border-colour;
-  padding: 10px 0 8px;
-
-  a {
-
-    &:link,
-    &:visited {
-      text-decoration: none;
-    }
-
-    &:hover {
-      color: $link-hover-colour;
-      text-decoration: underline;
-    }
-
-  }
-
-  &-account {
-
-    @include media(tablet) {
-
-      text-align: right;
-
-      a {
-        margin-left: 20px;
-      }
-
-    }
-
-  }
-
-}
diff --git a/app/assets/stylesheets/components/navigation.scss b/app/assets/stylesheets/components/navigation.scss
index 9ccb97e4a..8c384cdbd 100644
--- a/app/assets/stylesheets/components/navigation.scss
+++ b/app/assets/stylesheets/components/navigation.scss
@@ -2,18 +2,18 @@
 
   padding: 3px 0 0 0;
 
-  li {
-    @include core-16();
-    margin: 3px 0;
-  }
-
-  ul {
+  ul, h2 {
+    @include core-16;
     border-bottom: 1px solid $border-colour;
     margin: 10px 20px 0 0;
     list-style-type: none;
     padding: 0 0 8px 0;
   }
 
+  h2 {
+    font-weight: bold;
+  }
+
   li {
     margin: 7px 0 0 0;
   }
diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss
index 38d1547e1..8a624473c 100644
--- a/app/assets/stylesheets/main.scss
+++ b/app/assets/stylesheets/main.scss
@@ -41,8 +41,6 @@
 @import 'components/banner';
 @import 'components/textbox';
 @import 'components/browse-list';
-@import 'components/management-navigation';
-@import 'components/dropdown';
 @import 'components/email-message';
 @import 'components/api-key';
 
diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py
index 5e5a7e50b..27472bbc5 100644
--- a/app/main/views/add_service.py
+++ b/app/main/views/add_service.py
@@ -10,7 +10,7 @@ from app.main.forms import AddServiceForm
 def add_service():
     form = AddServiceForm(services_dao.find_all_service_names)
     services = services_dao.get_services(current_user.id)
-    if len(services) > 0:
+    if len(services) == 0:
         heading = 'Set up notifications for your service'
     else:
         heading = 'Add a new service'
diff --git a/app/main/views/choose_service.py b/app/main/views/choose_service.py
index 609050fac..39b0b9f8e 100644
--- a/app/main/views/choose_service.py
+++ b/app/main/views/choose_service.py
@@ -8,12 +8,6 @@ from app.main import main
 @login_required
 def choose_service():
     services = services_dao.get_services(current_user.id)
-    # If there is only one service redirect
-    # to the service dashboard.
-    if len(services['data']) == 1:
-        session['service_name'] = services['data'][0]['name']
-        return redirect(url_for(
-            'main.service_dashboard', service_id=services['data'][0]['id']))
     return render_template(
         'views/choose-service.html',
         services=[services_dao.ServicesBrowsableItem(x) for x in services['data']])
diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html
index 84687acff..4c0123923 100644
--- a/app/templates/admin_template.html
+++ b/app/templates/admin_template.html
@@ -28,10 +28,26 @@
 {% block header_class %}with-proposition{% endblock %}
 {% block proposition_header %}
   <div class="header-proposition">
-    <div class='content'>
+    <div class="content">
       <div class="phase-banner-beta">
         <strong class="phase-tag">BETA</strong>
       </div>
+      {% if current_user.is_authenticated() %}
+        <a href="#proposition-links" class="js-header-toggle menu">Menu</a>
+        <nav id="proposition-menu">
+          <ul id="proposition-links">
+            <li>
+              <a href="{{ url_for('main.user_profile') }}">{{ current_user.name }}</a>
+            </li>
+            <li>
+              <a href="{{ url_for('main.choose_service') }}">Switch service</a>
+            </li>
+            <li>
+              <a href="{{ url_for('main.sign_out')}}">Sign out</a>
+            </li>
+          </ul>
+        </nav>
+      {% endif %}
     </div>
   </div>
 {% endblock %}
@@ -48,28 +64,6 @@
 
 {% block content %}
 
-  {% if current_user.is_authenticated() and session['service_name'] != None %}
-    <nav class="management-navigation">
-      <div class="grid-row">
-        <div class="column-half">
-          <details class="dropdown">
-            <summary class="dropdown-toggle">
-              {{ session.get('service_name', 'Choose service') }}
-            </summary>
-            <div>
-              <a href="{{ url_for('main.choose_service') }}">Switch service</a>
-              <a href="{{ url_for('main.add_service') }}">Add a new service to GOV.UK Notify</a>
-            </div>
-          </details>
-        </div>
-        <div class="column-half management-navigation-account">
-          <a href="{{ url_for('main.user_profile') }}">{{ current_user.name }}</a>
-          <a href="{{ url_for('main.sign_out')}}">Sign out</a>
-        </div>
-      </div>
-    </nav>
-  {% endif %}
-
   <main id="content" role="main" class="page-container">
       {% with messages = get_flashed_messages(with_categories=true) %}
         {% if messages %}
diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html
index 609d6eef5..1b90a5a79 100644
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -1,4 +1,7 @@
 <nav class="navigation">
+  <h2 class="navigation-service-name">
+    {{ session.get('service_name', 'Choose service') }}
+  </h2>
   <ul>
     <li><a href="{{ url_for('.service_dashboard', service_id=service_id) }}">Dashboard</a></li>
   </ul>
diff --git a/gulpfile.babel.js b/gulpfile.babel.js
index c7427d856..0ee79b662 100644
--- a/gulpfile.babel.js
+++ b/gulpfile.babel.js
@@ -34,7 +34,6 @@ gulp.task('javascripts', () => gulp
   .src([
     paths.npm + 'govuk_frontend_toolkit/javascripts/govuk/modules.js',
     paths.npm + 'govuk_frontend_toolkit/javascripts/govuk/selection-buttons.js',
-    paths.src + 'javascripts/detailsPolyfill.js',
     paths.src + 'javascripts/apiKey.js',
     paths.src + 'javascripts/highlightTags.js',
     paths.src + 'javascripts/main.js'
diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py
index 2ec2f4061..6904bfb45 100644
--- a/tests/app/main/views/test_add_service.py
+++ b/tests/app/main/views/test_add_service.py
@@ -14,7 +14,7 @@ def test_get_should_render_add_service_template(app_,
             client.login(api_user_active)
             response = client.get(url_for('main.add_service'))
             assert response.status_code == 200
-            assert 'Set up notifications for your service' in response.get_data(as_text=True)
+            assert 'Add a new service' in response.get_data(as_text=True)
 
 
 def test_should_add_service_and_redirect_to_next_page(app_,