Merge pull request #2989 from alphagov/use-vertical-layout-on-applica…

…tions-table Make applications table vertical on small screens
alphagov · Jul 3, 2024 · 238e77c · 238e77c
2 parents c320a0b + 0404d75
commit 238e77c
Show file tree

Hide file tree

Showing 10 changed files with 54 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", classes: "govuk-!-width-one-quarter" },
       { text: "Description", classes: "govuk-!-width-one-third" },
-      { 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", classes: "govuk-!-width-one-quarter" },
       { text: "Description", classes: "govuk-!-width-one-third" },
-      { 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", classes: "govuk-!-width-one-quarter" },
       { text: "Description", classes: "govuk-!-width-one-third" },
-      { 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
@@ -31,7 +31,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 %>
@@ -48,7 +49,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", classes: "govuk-!-width-one-quarter" },
       { text: "Description", classes: "govuk-!-width-one-third" },
-      { 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", classes: "govuk-!-width-one-quarter" },
       { text: "Description", classes: "govuk-!-width-one-third" },
-      { 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/db/seeds.rb b/db/seeds.rb
@@ -129,6 +129,7 @@
 
 application = Doorkeeper::Application.create!(
   name: "Test Application 1",
+  description: "A concise description of what this app does",
   redirect_uri: "https://www.gov.uk",
 )
 
@@ -139,6 +140,7 @@
 
 application_with_9_permissions = Doorkeeper::Application.create!(
   name: "Test Application with 9 Permissions",
+  description: "A description which you could characterise as wordier and will most definitely take up considerably more space in the table of applications",
   redirect_uri: "https://www.gov.uk",
 )
 

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