Update applications table layout

At present, on viewports of between 940px and 1199px, the actions
links/buttons are various shades of messy, owing to the fact that they
share a single column, use `flex` alignment, and are of different widths

This updates the layout so that the actions links/buttons are nicely
aligned and consistently positioned at all viewport sizes and regardless
of which of the four links/buttons appear in the given tables

- The visible text is simplified, favouring a combination of a visible
  column heading of "Access" or "Permissions" combined with single-word
  actions. The text read by a screen reader in each column should remain
  the same: the part no longer visible in individual links/buttons is
  simply moved to the visually-hidden `span`
- The access column is moved before the permissions column to make the
  column order more in sync between the two tables
- The `description` column width is increased, retaining a good
  whitespace distribution across the table and potentially reducing the
  line count for apps with longer descriptions

app/assets/stylesheets/application.scss

@@ -121,3 +121,16 @@ $show-button-space: calc($show-button-width + $show-button-left-margin);
     padding-right: $show-button-space;
   }
 }
+
+.applications-table__remove_access_link {
+  width: auto;
+  min-width: 90px;
+}
+
+.applications-table__permissions-action-links a {
+  display: inline-block;
+
+  &:not(:last-child) {
+    margin-right: 1rem;
+  }
+}

app/helpers/application_table_helper.rb

@@ -1,10 +1,6 @@
 module ApplicationTableHelper
   include Pundit::Authorization
 
-  def wrap_links_in_actions_markup(links)
-    "<div class=\"govuk-table__actions\">#{links.join}</div>".html_safe
-  end
-
   def account_applications_grant_access_link(application)
     if policy([:account, Doorkeeper::Application]).grant_signin_permission?
       grant_access_link(application)
@@ -73,7 +69,7 @@ def grant_access_link(application, user = nil)
       path,
       class: "govuk-button govuk-!-margin-0",
       data: { module: "govuk-button" },
-    ) { button_or_link_content("Grant access", "to", application.name) }
+    ) { button_or_link_content("Grant", "access to", application.name) }
   end
 
   def remove_access_link(application, user = nil)
@@ -85,9 +81,9 @@ def remove_access_link(application, user = nil)
 
     link_to(
       path,
-      class: "govuk-button govuk-button--warning govuk-!-margin-0",
+      class: "govuk-button govuk-button--warning govuk-!-margin-0 applications-table__remove_access_link",
       data: { module: "govuk-button" },
-    ) { button_or_link_content("Remove access", "to", application.name) }
+    ) { button_or_link_content("Remove", "access to", application.name) }
   end
 
   def view_permissions_link(application, user = nil)
@@ -97,7 +93,7 @@ def view_permissions_link(application, user = nil)
              account_application_permissions_path(application)
            end
 
-    link_to(path, class: "govuk-link") { button_or_link_content("View permissions", "for", application.name) }
+    link_to(path, class: "govuk-link") { button_or_link_content("View", "permissions for", application.name) }
   end
 
   def update_permissions_link(application, user = nil)
@@ -115,7 +111,7 @@ def update_permissions_link(application, user = nil)
              edit_user_application_permissions_path(user, application)
            end
 
-    link_to(path, class: "govuk-link") { button_or_link_content("Update permissions", "for", application.name) }
+    link_to(path, class: "govuk-link") { button_or_link_content("Update", "permissions for", application.name) }
   end
 
   def button_or_link_content(visible_text, visually_hidden_join_word, application_name)

app/views/account/applications/index.html.erb

@@ -29,18 +29,16 @@
     caption: "Apps you 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"), visually_hidden: true },
+      { text: "Description", classes: "govuk-!-width-one-half" },
+      { text: "Access" },
+      { text: "Permissions" }
     ],
     rows: @applications_with_signin.map do |application|
     [
       { text: application.name },
       { text: application.description },
-      { text: wrap_links_in_actions_markup([
-        account_applications_permissions_links(application), 
-        account_applications_remove_access_link(application)
-        ])
-      },
+      { text: account_applications_remove_access_link(application) },
+      { text: account_applications_permissions_links(application), classes: "applications-table__permissions-action-links" }
     ]
     end,
     vertical_on_small_screen: true,
@@ -51,14 +49,14 @@
     caption: "Apps you don't 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"), visually_hidden: true }
+      { text: "Description", classes: "govuk-!-width-one-half" },
+      { text: "Access" }
     ],
     rows: @applications_without_signin.map do |application|
     [
       { text: application.name },
       { text: application.description },
-      { text: wrap_links_in_actions_markup([account_applications_grant_access_link(application)]) }
+      { text: account_applications_grant_access_link(application) }
     ]
     end,
     vertical_on_small_screen: true,

app/views/api_users/applications/index.html.erb

@@ -34,14 +34,14 @@
     caption: "Apps #{@api_user.name} has 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"), visually_hidden: true },
+      { text: "Description", classes: "govuk-!-width-one-half" },
+      { text: "Permissions" }
     ],
     rows: @applications.map do |application|
     [
       { text: application.name },
       { text: application.description },
-      { text: wrap_links_in_actions_markup([api_users_applications_permissions_link(application, @api_user)]) }
+      { text: api_users_applications_permissions_link(application, @api_user) }
     ]
     end,
     vertical_on_small_screen: true,

app/views/users/applications/index.html.erb

@@ -34,18 +34,16 @@
     caption: "Apps #{@user.name} has 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"), visually_hidden: true },
+      { text: "Description", classes: "govuk-!-width-one-half" },
+      { text: "Access" },
+      { text: "Permissions" }
     ],
     rows: @applications_with_signin.map do |application|
     [
       { text: application.name },
       { text: application.description },
-      { text: wrap_links_in_actions_markup([
-          users_applications_permissions_links(application, @user),
-          users_applications_remove_access_link(application, @user)
-        ])
-      },
+      { text: users_applications_remove_access_link(application, @user) },
+      { text: users_applications_permissions_links(application, @user), classes: "applications-table__permissions-action-links" }
     ]
     end,
     vertical_on_small_screen: true,
@@ -55,14 +53,14 @@
     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"), visually_hidden: true }
+      { text: "Description", classes: "govuk-!-width-one-half" },
+      { text: "Access" }
     ],
     rows: @applications_without_signin.map do |application|
     [
       { text: application.name },
       { text: application.description },
-      { text: wrap_links_in_actions_markup([users_applications_grant_access_link(application, @user)]) }
+      { text: users_applications_grant_access_link(application, @user) }
     ]
     end,
     vertical_on_small_screen: true,

docs/access_and_permissions.md

@@ -68,14 +68,13 @@ These dependencies determine whether a user can:
 ```mermaid
 flowchart TD
     A(Account::ApplicationsController#index) --authorize [:account, Doorkeeper::Application]--> B(Account::ApplicationPolicy#index?)
-    C(app/views/account/applications/index.html.erb) --wrap_links_in_actions_markup--> D(ApplicationTableHelper#wrap_links_in_actions_markup)
-    C --account_applications_permissions_links--> E(ApplicationTableHelper#account_applications_permissions_links)
-    E --policy([:account, application]).view_permissions?--> F(Account::ApplicationPolicy#view_permissions?)
-    E --policy([:account, application]).edit_permissions?--> G(Account::ApplicationPolicy#edit_permissions?)
-    C --account_applications_remove_access_link--> H(ApplicationTableHelper#account_applications_remove_access_link)
-    H --policy([:account, application]).remove_signin_permission?--> I(Account::ApplicationPolicy#remove_signin_permission?)
-    C --account_applications_grant_access_link--> J(ApplicationTableHelper#account_applications_grant_access_link)
-    J --policy([:account, Doorkeeper::Application]).grant_signin_permission?--> K(Account::ApplicationPolicy#grant_signin_permission?)
+    C(app/views/account/applications/index.html.erb) --account_applications_permissions_links--> D(ApplicationTableHelper#account_applications_permissions_links)
+    D --policy([:account, application]).view_permissions?--> E(Account::ApplicationPolicy#view_permissions?)
+    D --policy([:account, application]).edit_permissions?--> F(Account::ApplicationPolicy#edit_permissions?)
+    C --account_applications_remove_access_link--> G(ApplicationTableHelper#account_applications_remove_access_link)
+    G --policy([:account, application]).remove_signin_permission?--> H(Account::ApplicationPolicy#remove_signin_permission?)
+    C --account_applications_grant_access_link--> I(ApplicationTableHelper#account_applications_grant_access_link)
+    I --policy([:account, Doorkeeper::Application]).grant_signin_permission?--> J(Account::ApplicationPolicy#grant_signin_permission?)
 ```
 
 #### Account permissions show
@@ -239,18 +238,17 @@ These dependencies determine whether a user can:
 flowchart TD
     A(Users::ApplicationsController#index) --authorize [{ user: }], policy_class: Users::ApplicationPolicy--> B(Users::ApplicationPolicy#index?)
     B --Pundit.policy(current_user, user).edit?--> C(UserPolicy#edit?)
-    D(app/views/users/applications/index.html.erb) --wrap_links_in_actions_markup--> E(ApplicationTableHelper#wrap_links_in_actions_markup)
-    D --users_applications_permissions_links--> F(ApplicationTableHelper#users_applications_permissions_links)
-    F --Users::ApplicationPolicy.new(current_user, { application:, user: }).view_permissions?--> G(Users::ApplicationPolicy#view_permissions?)
-    G --Pundit.policy(current_user, user).edit?--> H(UserPolicy#edit?)
-    F --Users::ApplicationPolicy.new(current_user, { application:, user: }).edit_permissions?--> I(Users::ApplicationPolicy#edit_permissions?)
-    I --Pundit.policy(current_user, user).edit?--> H
-    D --users_applications_remove_access_link--> J(ApplicationTableHelper#users_applications_remove_access_link)
-    J --Users::ApplicationPolicy.new(current_user, { application:, user: }).remove_signin_permission?--> K(Users::ApplicationPolicy#remove_signin_permission?)
-    K --Pundit.policy(current_user, user).edit?--> H
-    D --users_applications_grant_access_link--> L(ApplicationTableHelper#users_applications_grant_access_link)
-    L --Users::ApplicationPolicy.new(current_user, { application:, user: }).grant_signin_permission?--> M(Users::ApplicationPolicy#grant_signin_permission?)
-    M --Pundit.policy(current_user, user).edit?--> H
+    D(app/views/users/applications/index.html.erb) --users_applications_permissions_links--> E(ApplicationTableHelper#users_applications_permissions_links)
+    E --Users::ApplicationPolicy.new(current_user, { application:, user: }).view_permissions?--> F(Users::ApplicationPolicy#view_permissions?)
+    F --Pundit.policy(current_user, user).edit?--> G(UserPolicy#edit?)
+    E --Users::ApplicationPolicy.new(current_user, { application:, user: }).edit_permissions?--> H(Users::ApplicationPolicy#edit_permissions?)
+    H --Pundit.policy(current_user, user).edit?--> G
+    D --users_applications_remove_access_link--> I(ApplicationTableHelper#users_applications_remove_access_link)
+    I --Users::ApplicationPolicy.new(current_user, { application:, user: }).remove_signin_permission?--> J(Users::ApplicationPolicy#remove_signin_permission?)
+    J --Pundit.policy(current_user, user).edit?--> G
+    D --users_applications_grant_access_link--> K(ApplicationTableHelper#users_applications_grant_access_link)
+    K --Users::ApplicationPolicy.new(current_user, { application:, user: }).grant_signin_permission?--> L(Users::ApplicationPolicy#grant_signin_permission?)
+    L --Pundit.policy(current_user, user).edit?--> G
 ```
 
 #### Users permissions show

test/helpers/application_table_helper_test.rb

@@ -3,22 +3,6 @@
 class ApplicationTableHelperTest < ActionView::TestCase
   include PunditHelpers
 
-  context "#wrap_links_in_actions_markup" do
-    should "wrap an array of links in a container with a class to apply actions styles" do
-      links = [
-        "<a class=\"govuk-link\" href=\"https://www.gov.uk/destination-one\">Destination one</a>",
-        "<a class=\"govuk-link\" href=\"https://www.gov.uk/destination-two\">Destination two</a>",
-      ]
-
-      expected_output = "<div class=\"govuk-table__actions\">
-        <a class=\"govuk-link\" href=\"https://www.gov.uk/destination-one\">Destination one</a>
-        <a class=\"govuk-link\" href=\"https://www.gov.uk/destination-two\">Destination two</a>
-      </div>".gsub(/>\s+</, "><")
-
-      assert_equal expected_output, wrap_links_in_actions_markup(links)
-    end
-  end
-
   context "#account_applications_grant_access_link" do
     setup do
       @user = build(:user)
@@ -28,7 +12,7 @@ class ApplicationTableHelperTest < ActionView::TestCase
 
     should "generate a grant access button when the current user can grant the signin permission" do
       stub_policy @user, [:account, Doorkeeper::Application], grant_signin_permission?: true
-      assert_includes account_applications_grant_access_link(@application), "Grant access"
+      assert_includes account_applications_grant_access_link(@application), "Grant"
     end
 
     should "return an empty string when the current user cannot grant the signin permission" do
@@ -54,7 +38,7 @@ class ApplicationTableHelperTest < ActionView::TestCase
         grant_signin_permission?: true,
       )
 
-      assert_includes users_applications_grant_access_link(@application, @grantee), "Grant access"
+      assert_includes users_applications_grant_access_link(@application, @grantee), "Grant"
     end
 
     should "return an empty string when the current user cannot grant the signin permission" do
@@ -78,7 +62,7 @@ class ApplicationTableHelperTest < ActionView::TestCase
 
     should "generate a remove access link when the current user can remove the signin permission" do
       stub_policy @user, [:account, @application], remove_signin_permission?: true
-      assert_includes account_applications_remove_access_link(@application), "Remove access"
+      assert_includes account_applications_remove_access_link(@application), "Remove"
     end
 
     should "return an empty string when the current user cannot remove the signin permission" do
@@ -104,7 +88,7 @@ class ApplicationTableHelperTest < ActionView::TestCase
         remove_signin_permission?: true,
       )
 
-      assert_includes users_applications_remove_access_link(@application, @grantee), "Remove access"
+      assert_includes users_applications_remove_access_link(@application, @grantee), "Remove"
     end
 
     should "return an empty string when the current user cannot remove the signin permission" do
@@ -130,26 +114,26 @@ class ApplicationTableHelperTest < ActionView::TestCase
 
       result = account_applications_permissions_links(@application)
 
-      assert_includes result, "View permissions"
-      assert_includes result, "Update permissions"
+      assert_includes result, "View"
+      assert_includes result, "Update"
     end
 
     should "only generate a view link when the user can only view permissions" do
       stub_policy @user, [:account, @application], view_permissions?: true
 
       result = account_applications_permissions_links(@application)
 
-      assert_includes result, "View permissions"
-      assert_not_includes result, "Update permissions"
+      assert_includes result, "View"
+      assert_not_includes result, "Update"
     end
 
     should "only generate an update link when the user can only edit permissions" do
       stub_policy @user, [:account, @application], edit_permissions?: true
 
       result = account_applications_permissions_links(@application)
 
-      assert_not_includes result, "View permissions"
-      assert_includes result, "Update permissions"
+      assert_not_includes result, "View"
+      assert_includes result, "Update"
     end
 
     should "return an empty string when the user can do neither" do
@@ -178,8 +162,8 @@ class ApplicationTableHelperTest < ActionView::TestCase
 
       result = users_applications_permissions_links(@application, @grantee)
 
-      assert_includes result, "View permissions"
-      assert_includes result, "Update permissions"
+      assert_includes result, "View"
+      assert_includes result, "Update"
     end
 
     should "only generate a view link when the user can only view permissions" do
@@ -193,8 +177,8 @@ class ApplicationTableHelperTest < ActionView::TestCase
 
       result = users_applications_permissions_links(@application, @grantee)
 
-      assert_includes result, "View permissions"
-      assert_not_includes result, "Update permissions"
+      assert_includes result, "View"
+      assert_not_includes result, "Update"
     end
 
     should "only generate an edit link when the user can only edit permissions" do
@@ -208,8 +192,8 @@ class ApplicationTableHelperTest < ActionView::TestCase
 
       result = users_applications_permissions_links(@application, @grantee)
 
-      assert_not_includes result, "View permissions"
-      assert_includes result, "Update permissions"
+      assert_not_includes result, "View"
+      assert_includes result, "Update"
     end
 
     should "return an empty string when the user can do neither" do
@@ -234,7 +218,7 @@ class ApplicationTableHelperTest < ActionView::TestCase
       grantee = create(:api_user)
       stubs(:current_user).returns(granter)
 
-      assert_includes api_users_applications_permissions_link(application, grantee), "Update permissions"
+      assert_includes api_users_applications_permissions_link(application, grantee), "Update"
     end
   end