From c6f54966bfa01356079f52ad9be3b0a561314c93 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 14 Dec 2017 11:58:40 +0000
Subject: [PATCH] Change tables to scroll in-page, not full screen
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There were three problems with showing tables fullscreen:
- it was over-optimised for very big spreadsheets, whereas most users
  will only have a few columns in their files
- it was jarring to go from full screen and back to the normal layout
- it was a bit change for existing users, where we prefer incremental
  changes that make things better without disrupting people’s work
  (where possible)

So this commit changes the big table to scroll horizontally in the page,
not take up the full width of the page.

From the fullscreen table it keeps:
- the shimming method to keep the horizontal scrollbar at the bottom of
  the screen at all times

It introduces some more refinements to make it nicer to use:
- fixing the first column, so you always know what row you’re on
- adding shadows indicate where there is content that’s scrolled outside
  the edges of the container
---
 app/assets/javascripts/fullscreenTable.js     |  94 ++++++++--
 .../components/fullscreen-table.scss          | 166 ++++++++++--------
 .../stick-at-top-when-scrolling.scss          |  21 +++
 app/assets/stylesheets/components/table.scss  |   3 +-
 app/templates/fullscreen_template.html        |  23 ---
 app/templates/views/check/column-errors.html  |  86 ++++-----
 app/templates/views/check/ok.html             |  52 +++---
 app/templates/views/check/row-errors.html     |  86 +++++----
 8 files changed, 303 insertions(+), 228 deletions(-)
 delete mode 100644 app/templates/fullscreen_template.html

diff --git a/app/assets/javascripts/fullscreenTable.js b/app/assets/javascripts/fullscreenTable.js
index 69d49fa1c..5d4080b2f 100644
--- a/app/assets/javascripts/fullscreenTable.js
+++ b/app/assets/javascripts/fullscreenTable.js
@@ -6,13 +6,22 @@
     this.start = function(component) {
 
       this.$component = $(component);
-      this.nativeHeight = this.$component.innerHeight();
+      this.$table = this.$component.find('table');
+      this.nativeHeight = this.$component.innerHeight() + 20; // 20px to allow room for scrollbar
       this.topOffset = this.$component.offset().top;
 
-      this.insertShim();
+      this.insertShims();
+      this.maintainWidth();
       this.maintainHeight();
+      this.toggleShadows();
 
-      $(window).on('scroll resize', this.maintainHeight);
+      $(window)
+        .on('scroll resize', this.maintainHeight)
+        .on('resize', this.maintainWidth);
+
+      this.$scrollableTable
+        .on('scroll', this.toggleShadows)
+        .on('scroll', this.maintainHeight);
 
       if (
         window.GOVUK.stopScrollingAtFooter &&
@@ -23,20 +32,75 @@
 
     };
 
-    this.insertShim = () => this.$component.after(
-      $("<div class='fullscreen-shim'/>").css({
-        'height': this.nativeHeight - this.topOffset,
-        'top': this.topOffset
-      })
-    );
+    this.insertShims = () => {
 
-    this.maintainHeight = () => this.$component.css({
-      'max-height': Math.min(
-        $(window).height() - this.topOffset + $('html, body').scrollTop(),
+      this.$table.wrap('<div class="fullscreen-scrollable-table"/>');
+
+      this.$component
+        .append(
+          this.$component.find('.fullscreen-scrollable-table')
+            .clone()
+            .addClass('fullscreen-fixed-table')
+            .removeClass('fullscreen-scrollable-table')
+            .attr('role', 'presentation')
+        )
+        .append(
+          '<div class="fullscreen-right-shadow" />'
+        )
+        .after(
+          $("<div class='fullscreen-shim'/>").css({
+            'height': this.nativeHeight,
+            'top': this.topOffset
+          })
+        );
+
+      this.$scrollableTable = this.$component.find('.fullscreen-scrollable-table');
+      this.$fixedTable = this.$component.find('.fullscreen-fixed-table');
+
+    };
+
+    this.maintainHeight = () => {
+
+      let height = Math.min(
+        $(window).height() - this.topOffset + $('html, body').scrollTop() + 5,
         this.nativeHeight
-      ),
-      'min-height': $(window).height() - this.topOffset
-    });
+      );
+
+      this.$scrollableTable.outerHeight(height);
+      this.$fixedTable.outerHeight(height);
+
+    };
+
+    this.maintainWidth = () => {
+
+      let indexColumnWidth = this.$fixedTable.find('.table-field-index').outerWidth();
+
+      this.$scrollableTable
+        .css({
+            'width': this.$component.parent('main').width() - indexColumnWidth,
+            'margin-left': indexColumnWidth
+        });
+
+      this.$fixedTable
+        .width(indexColumnWidth + 4);
+
+    };
+
+    this.toggleShadows = () => {
+
+      this.$fixedTable
+        .toggleClass(
+          'fullscreen-scrolled-table',
+          this.$scrollableTable.scrollLeft() > 0
+        );
+
+      this.$component.find('.fullscreen-right-shadow')
+        .toggleClass(
+          'visible',
+          this.$scrollableTable.scrollLeft() < (this.$table.width() - this.$scrollableTable.width())
+        );
+
+    };
 
   };
 
diff --git a/app/assets/stylesheets/components/fullscreen-table.scss b/app/assets/stylesheets/components/fullscreen-table.scss
index 74593a9ed..b552fcbd8 100644
--- a/app/assets/stylesheets/components/fullscreen-table.scss
+++ b/app/assets/stylesheets/components/fullscreen-table.scss
@@ -1,59 +1,68 @@
-body.with-fullscreen {
-
-  #global-header,
-  #global-header-bar {
-    display: none;
-  }
-
-  #footer {
-    height: 0;
-    overflow: hidden;
-    border-color: $white;
-  }
-
-  .shim {
-    margin-bottom: 5px;
-  }
-
-  &::-webkit-scrollbar {
-    -webkit-appearance: none;
-  }
-
-  &::-webkit-scrollbar:vertical {
-    width: 11px;
-  }
-
-  &::-webkit-scrollbar-thumb {
-    border-radius: 8px;
-    border: 2px solid $white;
-    background-color: rgba(0, 0, 0, .5);
-  }
-
-  &::-webkit-scrollbar-track {
-    background-color: $white;
-    border-radius: 8px;
-  }
-
-}
-
-
 .fullscreen {
 
-  &-header {
-    padding: $gutter-half $gutter-half 0 $gutter-half;
-    margin-top: -$gutter-half;
-  }
-
   &-content {
 
-    width: 100%;
     background: $white;
     z-index: 10;
-    overflow-x: scroll;
     overflow-y: hidden;
     box-sizing: border-box;
     position: absolute;
-    padding-left: $gutter-half;
+    margin: 5px 0 $gutter 0;
+    padding: 0 0 0 0;
+    overflow: hidden;
+    border-bottom: 1px solid $border-colour;
+
+    .table {
+
+      margin-bottom: 0;
+
+      tr:last-child {
+        td {
+          border-bottom: 1px solid $white;
+        }
+      }
+
+    }
+
+    th,
+    .table-field-error-label,
+    .table-field-center-aligned {
+      white-space: nowrap;
+    }
+
+  }
+
+  &-right-shadow {
+
+    position: absolute;
+    top: 0;
+    right: 0;
+    width: 4px;
+    height: 100%;
+    z-index: 200;
+
+    &.visible {
+      transition: box-shadow 0.3s ease-in-out;
+      box-shadow: inset -1px 0 0 0 $border-colour, inset -3px 0 0 0 rgba($border-colour, 0.2);
+    }
+
+  }
+
+  &-scrollable-table {
+
+    overflow-x: auto;
+    overflow-y: hidden;
+
+    .table-field-heading-first,
+    .table-field-index {
+      display: none;
+    }
+
+    .table-field-center-aligned {
+      position: relative;
+      z-index: 150;
+      background: $white;
+    }
 
     &::-webkit-scrollbar {
       -webkit-appearance: none;
@@ -61,6 +70,7 @@ body.with-fullscreen {
 
     &::-webkit-scrollbar:horizontal {
       height: 11px;
+      background-color: $white;
     }
 
     &::-webkit-scrollbar-thumb {
@@ -74,24 +84,43 @@ body.with-fullscreen {
       border-radius: 8px;
     }
 
-    .banner-dangerous {
-      margin: $gutter-half $gutter-half 0 $gutter-half;
-      position: sticky;
-      left: 0;
+  }
+
+  &-fixed-table {
+
+    position: absolute;
+    top: 0;
+    overflow: hidden;
+
+    .table-field-heading {
+      visibility: hidden;
     }
 
-    .table {
-      border-right: $gutter-half solid $white;  // border used as padding
+    .table-field-center-aligned {
+      width: 0;
+      position: relative;
+      z-index: 100;
+      visibility: hidden;
     }
 
-    th,
-    .table-field-error-label {
-      white-space: nowrap;
+    .table-field-heading-first,
+    .table-field-index {
+      transition: none;
+      position: relative;
+      z-index: 200;
+      background: $white;
     }
 
-    .table-show-more-link {
-      border: none;
-      text-align: left;
+  }
+
+  &-scrolled-table {
+
+    padding-bottom: 20px;
+
+    .table-field-heading-first,
+    .table-field-index {
+      transition: box-shadow 0.3s ease-in-out;
+      box-shadow: 1px 0 0 0 $border-colour, 3px 0 0 0 rgba($border-colour, 0.2);
     }
 
   }
@@ -100,25 +129,6 @@ body.with-fullscreen {
     width: 100%;
     position: relative;
     z-index: 9;
-    background: $white;
-  }
-
-  &-sticky-bar {
-
-    z-index: 20;
-    padding-right: 0;
-
-    .page-footer-back-link {
-      position: absolute;
-      right: $gutter-half;
-      top: 20px;
-    }
-
-    .file-upload-button,
-    .file-upload-button-cancel {
-      margin: 0 0 0 $gutter-half;
-    }
-
   }
 
 }
diff --git a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss
index cd87884f1..cee9ffd16 100644
--- a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss
+++ b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss
@@ -16,9 +16,24 @@
     margin-bottom: 20px;
   }
 
+  .back-to-top-link {
+
+    position: absolute;
+    top: $gutter;
+    right: $gutter-half;
+    opacity: 0;
+    transition: opacity 0.1s ease-in-out;
+
+    @include ie-lte(8) {
+      display: none;
+    }
+
+  }
+
 }
 
 .content-fixed {
+
   position: fixed;
   top: 0;
   background: $white;
@@ -28,6 +43,12 @@
   border-bottom: 1px solid $border-colour;
   box-shadow: 0 2px 0 0 rgba($border-colour, 0.2);
   transition: background 0.6s ease-in-out, margin-top 0.4s ease-out;
+
+  .back-to-top-link {
+    opacity: 1;
+    transition: opacity 0.6s ease-in-out;
+  }
+
 }
 
 .shim {
diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss
index c86d5cc12..5bd4c68c0 100644
--- a/app/assets/stylesheets/components/table.scss
+++ b/app/assets/stylesheets/components/table.scss
@@ -209,10 +209,9 @@
 .table-show-more-link {
   @include core-16;
   color: $secondary-text-colour;
-  margin-top: -30px;
   margin-bottom: $gutter * 1.3333;
   border-bottom: 1px solid $border-colour;
-  padding: 0.75em 0 0.5625em 0;
+  padding: 10px 0 10px 0;
   text-align: center;
 }
 
diff --git a/app/templates/fullscreen_template.html b/app/templates/fullscreen_template.html
deleted file mode 100644
index 11be0fa67..000000000
--- a/app/templates/fullscreen_template.html
+++ /dev/null
@@ -1,23 +0,0 @@
-{% extends "admin_template.html" %}
-
-{% block inside_header %}{% endblock %}
-{% block proposition_header %}{% endblock %}
-
-{% block body_classes %} with-fullscreen {% endblock %}
-
-{% block footer_top %}{% endblock %}
-{% block footer_support_links %}{% endblock %}
-
-{% block content %}
-  <main role="main">
-    <div class="fullscreen-header">
-      {% block fullscreen_pre_title %}{% endblock %}
-    </div>
-    <div class="fullscreen-sticky-bar js-stick-at-top-when-scrolling">
-      {% block fullscreen_title %}{% endblock %}
-    </div>
-    <div class="fullscreen-content" data-module='fullscreen-table'>
-      {% block fullscreen_content %}{% endblock %}
-    </div>
-  </main>
-{% endblock %}
diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html
index 6f3b626d1..4fec14c21 100644
--- a/app/templates/views/check/column-errors.html
+++ b/app/templates/views/check/column-errors.html
@@ -114,54 +114,58 @@
     {% endcall %}
   </div>
 
-  <div class="bottom-gutter-3-2">
-    {% if request.args.from_test %}
-      <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
-    {% else %}
-      {{file_upload(form.file, button_text='Re-upload your file')}}
-    {% endif %}
+
+  <div class="js-stick-at-top-when-scrolling">
+    <div class="form-group">
+      {% if request.args.from_test %}
+        <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
+      {% else %}
+        {{file_upload(form.file, button_text='Re-upload your file')}}
+      {% endif %}
+    </div>
+    <a href="#content" class="back-to-top-link">Back to top</a>
   </div>
 
   {% if not request.args.from_test %}
 
     <h2 class="heading-medium" id="{{ file_contents_header_id }}">{{ original_file_name }}</h2>
 
-    {% call(item, row_number) list_table(
-      recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
-      caption=original_file_name,
-      caption_visible=False,
-      field_headings=[
-        '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
-      ] + recipients.column_headers
-    ) %}
-      {% call index_field() %}
-        <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
-          {{ item.index + 2 }}
-        </span>
-      {% endcall %}
-      {% for column in recipients.column_headers %}
-        {% if item['columns'][column].error and not recipients.missing_column_headers %}
-          {% call field() %}
-            <span>
-              <span class="table-field-error-label">{{ item['columns'][column].error }}</span>
-              {{ item['columns'][column].data if item['columns'][column].data != None }}
-            </span>
-          {% endcall %}
-        {% elif item['columns'][column].ignore %}
-          {{ text_field(item['columns'][column].data or '', status='default') }}
-        {% else %}
-          {{ text_field(item['columns'][column].data or '') }}
-        {% endif %}
-      {% endfor %}
-      {% if item['columns'].get(None) %}
-        {% for column in item['columns'][None].data %}
-          {{ text_field(column, status='default') }}
+    <div class="fullscreen-content" data-module="fullscreen-table">
+      {% call(item, row_number) list_table(
+        recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
+        caption=original_file_name,
+        caption_visible=False,
+        field_headings=[
+          '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
+        ] + recipients.column_headers
+      ) %}
+        {% call index_field() %}
+          <span>
+            {{ item.index + 2 }}
+          </span>
+        {% endcall %}
+        {% for column in recipients.column_headers %}
+          {% if item['columns'][column].error and not recipients.missing_column_headers %}
+            {% call field() %}
+              <span>
+                <span class="table-field-error-label">{{ item['columns'][column].error }}</span>
+                {{ item['columns'][column].data if item['columns'][column].data != None }}
+              </span>
+            {% endcall %}
+          {% elif item['columns'][column].ignore %}
+            {{ text_field(item['columns'][column].data or '', status='default') }}
+          {% else %}
+            {{ text_field(item['columns'][column].data or '') }}
+          {% endif %}
         {% endfor %}
-      {% endif %}
-    {% endcall %}
-
-  {% endif %}
-
+        {% if item['columns'].get(None) %}
+          {% for column in item['columns'][None].data %}
+            {{ text_field(column, status='default') }}
+          {% endfor %}
+        {% endif %}
+      {% endcall %}
+    {% endif %}
+  </div>
 
   {% if recipients.too_many_rows %}
     <p class="table-show-more-link">
diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html
index 4cd5d20f2..792f016d1 100644
--- a/app/templates/views/check/ok.html
+++ b/app/templates/views/check/ok.html
@@ -49,32 +49,34 @@
 
     <h2 class="heading-medium" id="{{ file_contents_header_id }}">{{ original_file_name }}</h2>
 
-    {% call(item, row_number) list_table(
-      recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
-      caption=original_file_name,
-      caption_visible=False,
-      field_headings=[
-        '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
-      ] + recipients.column_headers
-    ) %}
-      {% call index_field() %}
-        <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
-          {{ item.index + 2 }}
-        </span>
-      {% endcall %}
-      {% for column in recipients.column_headers %}
-        {% if item['columns'][column].ignore %}
-          {{ text_field(item['columns'][column].data or '', status='default') }}
-        {% else %}
-          {{ text_field(item['columns'][column].data or '') }}
-        {% endif %}
-      {% endfor %}
-      {% if item['columns'].get(None) %}
-        {% for column in item['columns'][None].data %}
-          {{ text_field(column, status='default') }}
+    <div class="fullscreen-content" data-module="fullscreen-table">
+      {% call(item, row_number) list_table(
+        recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
+        caption=original_file_name,
+        caption_visible=False,
+        field_headings=[
+          '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
+        ] + recipients.column_headers
+      ) %}
+        {% call index_field() %}
+          <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
+            {{ item.index + 2 }}
+          </span>
+        {% endcall %}
+        {% for column in recipients.column_headers %}
+          {% if item['columns'][column].ignore %}
+            {{ text_field(item['columns'][column].data or '', status='default') }}
+          {% else %}
+            {{ text_field(item['columns'][column].data or '') }}
+          {% endif %}
         {% endfor %}
-      {% endif %}
-    {% endcall %}
+        {% if item['columns'].get(None) %}
+          {% for column in item['columns'][None].data %}
+            {{ text_field(column, status='default') }}
+          {% endfor %}
+        {% endif %}
+      {% endcall %}
+    </div>
 
   {% endif %}
 
diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html
index 1ea7f870d..e92647886 100644
--- a/app/templates/views/check/row-errors.html
+++ b/app/templates/views/check/row-errors.html
@@ -1,4 +1,4 @@
-{% extends "fullscreen_template.html" %}
+{% extends "withnav_template.html" %}
 {% from "components/banner.html" import banner_wrapper %}
 {% from "components/radios.html" import radio_select %}
 {% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %}
@@ -13,11 +13,11 @@
   </p>
 {% endmacro %}
 
-{% block per_page_title %}
+{% block service_page_title %}
   Error
 {% endblock %}
 
-{% block fullscreen_pre_title %}
+{% block maincolumn_content %}
 
   <div class="bottom-gutter-1-2">
     {% call banner_wrapper(type='dangerous') %}
@@ -45,50 +45,48 @@
     {% endcall %}
   </div>
 
-  {% endblock %}
-  {% block fullscreen_title %}
-  <div class="bottom-gutter-2-3">
-    {{ file_upload(form.file, button_text='Re-upload your file') }}
-    <a href="{{ back_link }}" class="page-footer-back-link">Go back</a>
+  <div class="js-stick-at-top-when-scrolling">
+    <div class="form-group">
+      {{ file_upload(form.file, button_text='Re-upload your file') }}
+    </div>
+    <a href="#content" class="back-to-top-link">Back to top</a>
   </div>
 
-  {% endblock %}
-  {% block fullscreen_content %}
-
-  {% call(item, row_number) list_table(
-    recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
-    caption=original_file_name,
-    caption_visible=False,
-    field_headings=[
-      '<span class="visually-hidden">Row in file</span><span aria-hidden="true" class="table-field-invisible-error">1</span>'|safe
-    ] + recipients.column_headers
-  ) %}
-    {% call index_field() %}
-      <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
-        {{ item.index + 2 }}
-      </span>
-    {% endcall %}
-    {% for column in recipients.column_headers %}
-      {% if item['columns'][column].error and not recipients.missing_column_headers %}
-        {% call field() %}
-          <span>
-            <span class="table-field-error-label">{{ item['columns'][column].error }}</span>
-            {{ item['columns'][column].data if item['columns'][column].data != None }}
-          </span>
-        {% endcall %}
-      {% elif item['columns'][column].ignore %}
-        {{ text_field(item['columns'][column].data or '', status='default') }}
-      {% else %}
-        {{ text_field(item['columns'][column].data or '') }}
-      {% endif %}
-    {% endfor %}
-    {% if item['columns'].get(None) %}
-      {% for column in item['columns'][None].data %}
-        {{ text_field(column, status='default') }}
+  <div class="fullscreen-content" data-module="fullscreen-table">
+    {% call(item, row_number) list_table(
+      recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
+      caption=original_file_name,
+      caption_visible=False,
+      field_headings=[
+        '<span class="visually-hidden">Row in file</span><span aria-hidden="true" class="table-field-invisible-error">1</span>'|safe
+      ] + recipients.column_headers
+    ) %}
+      {% call index_field() %}
+        <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
+          {{ item.index + 2 }}
+        </span>
+      {% endcall %}
+      {% for column in recipients.column_headers %}
+        {% if item['columns'][column].error and not recipients.missing_column_headers %}
+          {% call field() %}
+            <span>
+              <span class="table-field-error-label">{{ item['columns'][column].error }}</span>
+              {{ item['columns'][column].data if item['columns'][column].data != None }}
+            </span>
+          {% endcall %}
+        {% elif item['columns'][column].ignore %}
+          {{ text_field(item['columns'][column].data or '', status='default') }}
+        {% else %}
+          {{ text_field(item['columns'][column].data or '') }}
+        {% endif %}
       {% endfor %}
-    {% endif %}
-  {% endcall %}
-
+      {% if item['columns'].get(None) %}
+        {% for column in item['columns'][None].data %}
+          {{ text_field(column, status='default') }}
+        {% endfor %}
+      {% endif %}
+    {% endcall %}
+  </div>
   {% if count_of_displayed_recipients < count_of_recipients %}
     <p class="table-show-more-link">
       {% if row_errors and not recipients.missing_column_headers %}