Make applications table vertical on small screens

Since #2341, we've had support for making tables use a vertical layout on smaller viewports, similar to how the summary list reflows on smaller viewports (the end result is something of a hybrid of table and summary list layouts) We already use this on the users screen. This feels like something that would be useful on the applications screen too I've added an optional `visually_hidden` flag on table head cells here, which when set to `true` will result in a class that removes padding from the header cell. The current actions column on the applications page has a visually hidden header, so this allows us to remove the padding from visually hidden header cells while retaining the content Per f0a8f53, we switch to regex-based matching for some tests. The vertical layout implementation adds some hidden text in table cells that would otherwise break these `assert_select`s
alphagov · Jun 27, 2024 · 853cee6 · 853cee6
1 parent cdb506e
commit 853cee6
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 33 deletions.
diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
@@ -8,6 +8,8 @@ $govuk-page-width: 1140px;
 @import "components/contact-details";
 @import "components/table";
 
+$applications-table-actions-gap: 10px;
+
 // TODO: move into component
 .gem-c-success-alert,
 .gem-c-error-alert {
@@ -88,15 +90,22 @@ $govuk-page-width: 1140px;
   }
 }
 
-.govuk-table__actions {
-  display: flex;
-  flex-direction: column;
-  align-items: end;
-  gap: 10px;
-
-  @media (min-width: 40.0625em) {
+.govuk-table__actions { 
+  @include govuk-media-query($until: $vertical-on-smallscreen-breakpoint) {
+    .govuk-button, .govuk-link {
+      display: block;
+
+      &:not(:last-child) {
+        margin-bottom: $applications-table-actions-gap;
+      }
+    }
+  }
+
+  @include govuk-media-query($from: $vertical-on-smallscreen-breakpoint) {
+    display: flex;
     flex-direction: row;
     align-items: center;
     justify-content: flex-end;
+    gap: $applications-table-actions-gap;
   }
 }
diff --git a/app/assets/stylesheets/components/_table.scss b/app/assets/stylesheets/components/_table.scss
@@ -176,6 +176,10 @@ $table-row-even-background-colour: govuk-colour("light-grey", $legacy: "grey-4")
     padding-right: 1em;
     text-align: left;
     word-break: initial;
+
+    &--visually-hidden {
+      padding-right: 0;
+    }
   }
 
   @include govuk-media-query($from: $vertical-on-smallscreen-breakpoint) {

diff --git a/app/views/account/applications/index.html.erb b/app/views/account/applications/index.html.erb
@@ -30,7 +30,7 @@
     head: [
       { text: "Name" },
       { text: "Description" },
-      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") },
+      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true },
     ],
     rows: @applications_with_signin.map do |application|
     [
@@ -43,6 +43,7 @@
       },
     ]
     end,
+    vertical_on_small_screen: true,
 } %>
 
 <div class="govuk-table--with-actions">
@@ -51,7 +52,7 @@
     head: [
       { text: "Name" },
       { text: "Description" },
-      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") }
+      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true }
     ],
     rows: @applications_without_signin.map do |application|
     [
@@ -60,5 +61,6 @@
       { text: wrap_links_in_actions_markup([account_applications_grant_access_link(application)]) }
     ]
     end,
+    vertical_on_small_screen: true,
 } %>
 </div>
diff --git a/app/views/api_users/applications/index.html.erb b/app/views/api_users/applications/index.html.erb
@@ -35,7 +35,7 @@
     head: [
       { text: "Name" },
       { text: "Description" },
-      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") },
+      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true },
     ],
     rows: @applications.map do |application|
     [
@@ -44,4 +44,5 @@
       { text: wrap_links_in_actions_markup([api_users_applications_permissions_link(application, @api_user)]) }
     ]
     end,
+    vertical_on_small_screen: true,
 } %>
diff --git a/app/views/components/_table.html.erb b/app/views/components/_table.html.erb
@@ -30,7 +30,8 @@
             format: item[:format],
             href: item[:href],
             data_attributes: item[:data_attributes],
-            sort_direction: item[:sort_direction]
+            sort_direction: item[:sort_direction],
+            visually_hidden: item[:visually_hidden]
           } %>
         <% end %>
       <% end %>
@@ -47,7 +48,7 @@
               } %>
             <% elsif vertical_on_small_screen && head.any? %>
               <%= t.cell nil, { format: cell[:format] } do %>
-                <span class="app-c-table__duplicate-heading" aria-hidden="true">
+                <span class="app-c-table__duplicate-heading<%= head[cellindex][:visually_hidden] ? " app-c-table__duplicate-heading--visually-hidden" : "" %>" aria-hidden="true">
                   <%= head[cellindex][:text] %>
                 </span>
                 <%= cell[:text] %>

diff --git a/app/views/users/applications/index.html.erb b/app/views/users/applications/index.html.erb
@@ -35,7 +35,7 @@
     head: [
       { text: "Name" },
       { text: "Description" },
-      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") },
+      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true },
     ],
     rows: @applications_with_signin.map do |application|
     [
@@ -48,14 +48,15 @@
       },
     ]
     end,
+    vertical_on_small_screen: true,
 } %>
 
 <%= render "components/table", {
     caption: "Apps #{@user.name} does not have access to",
     head: [
       { text: "Name" },
       { text: "Description" },
-      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") }
+      { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true }
     ],
     rows: @applications_without_signin.map do |application|
     [
@@ -64,4 +65,5 @@
       { text: wrap_links_in_actions_markup([users_applications_grant_access_link(application, @user)]) }
     ]
     end,
+    vertical_on_small_screen: true,
 } %>
diff --git a/test/controllers/account/applications_controller_test.rb b/test/controllers/account/applications_controller_test.rb
@@ -35,7 +35,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
         assert_select "form[action='#{account_application_signin_permission_path(application)}']"
       end
 
@@ -46,7 +46,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
         assert_select "a[href='#{delete_account_application_signin_permission_path(application)}']"
       end
 
@@ -57,7 +57,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
         assert_select "a[href='#{edit_account_application_permissions_path(application)}']"
       end
 
@@ -68,7 +68,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
         assert_select "a[href='#{edit_account_application_permissions_path(application)}']", count: 0
       end
 
@@ -78,7 +78,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "retired-app-name", count: 0
+        assert_select "tr td", text: /retired-app-name/, count: 0
       end
 
       should "not display an API-only application" do
@@ -87,7 +87,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "api-only-app-name", count: 0
+        assert_select "tr td", text: /api-only-app-name/, count: 0
       end
     end
 
@@ -102,7 +102,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
         get :index
 
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
         assert_select "form[action='#{account_application_signin_permission_path(@application)}']", count: 0
       end
 
@@ -116,7 +116,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
           get :index
 
-          assert_select "tr td", text: "app-name"
+          assert_select "tr td", text: /app-name/
           assert_select "a[href='#{delete_account_application_signin_permission_path(@application)}']"
         end
 
@@ -127,7 +127,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
           get :index
 
-          assert_select "tr td", text: "app-name"
+          assert_select "tr td", text: /app-name/
           assert_select "a[href='#{edit_account_application_permissions_path(@application)}']"
         end
 
@@ -136,7 +136,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
           get :index
 
-          assert_select "tr td", text: "app-name"
+          assert_select "tr td", text: /app-name/
           assert_select "a[href='#{edit_account_application_permissions_path(@application)}']", count: 0
         end
 
@@ -150,7 +150,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
             get :index
 
-            assert_select "tr td", text: "app-name"
+            assert_select "tr td", text: /app-name/
             assert_select "a[href='#{delete_account_application_signin_permission_path(@application)}']", count: 0
           end
 
@@ -159,7 +159,7 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
 
             get :index
 
-            assert_select "tr td", text: "app-name"
+            assert_select "tr td", text: /app-name/
             assert_select "a[href='#{account_application_permissions_path(@application)}']"
           end
         end

diff --git a/test/controllers/api_users/applications_controller_test.rb b/test/controllers/api_users/applications_controller_test.rb
@@ -37,7 +37,7 @@ class ApiUsers::ApplicationsControllerTest < ActionController::TestCase
       get :index, params: { api_user_id: api_user }
 
       assert_select "table:has( > caption[text()='Apps #{api_user.name} has access to'])" do
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
       end
     end
 
@@ -54,7 +54,7 @@ class ApiUsers::ApplicationsControllerTest < ActionController::TestCase
 
       get :index, params: { api_user_id: api_user }
 
-      assert_select "tr td", text: "revoked-app-name", count: 0
+      assert_select "tr td", text: /revoked-app-name/, count: 0
     end
 
     should "not display a retired application" do
@@ -70,7 +70,7 @@ class ApiUsers::ApplicationsControllerTest < ActionController::TestCase
 
       get :index, params: { api_user_id: api_user }
 
-      assert_select "tr td", text: "retired-app-name", count: 0
+      assert_select "tr td", text: /retired-app-name/, count: 0
     end
 
     should "display a flash message showing the permissions the user has" do

diff --git a/test/controllers/users/applications_controller_test.rb b/test/controllers/users/applications_controller_test.rb
@@ -77,7 +77,7 @@ class Users::ApplicationsControllerTest < ActionController::TestCase
       get :index, params: { user_id: user }
 
       assert_select "table:has( > caption[text()='Apps #{user.name} has access to'])" do
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
       end
     end
 
@@ -94,7 +94,7 @@ class Users::ApplicationsControllerTest < ActionController::TestCase
       get :index, params: { user_id: user }
 
       assert_select "table:has( > caption[text()='Apps #{user.name} does not have access to'])" do
-        assert_select "tr td", text: "app-name"
+        assert_select "tr td", text: /app-name/
       end
     end
 
@@ -232,7 +232,7 @@ class Users::ApplicationsControllerTest < ActionController::TestCase
 
       get :index, params: { user_id: user }
 
-      assert_select "tr td", text: "retired-app-name", count: 0
+      assert_select "tr td", text: /retired-app-name/, count: 0
     end
 
     should "not display an API-only application" do
@@ -247,7 +247,7 @@ class Users::ApplicationsControllerTest < ActionController::TestCase
 
       get :index, params: { user_id: user }
 
-      assert_select "tr td", text: "api-only-app-name", count: 0
+      assert_select "tr td", text: /api-only-app-name/, count: 0
     end
   end