From 6092dc1a83f1b940f87d0f479b313264a0b9d2dd Mon Sep 17 00:00:00 2001
From: Ynda Jas <yndajas@gmail.com>
Date: Thu, 27 Jun 2024 14:44:59 +0100
Subject: [PATCH] Fix applications tables column alignment

In #2341, the table component from `govuk_publishing_components` was
copied over and customised

In #2664, we migrated the applications tables to use this. In the
process, we lost the custom classes on the first two columns, which set
the widths to 1/4 and 1/3 respectively. This was noted as "acceptable"
in the PR body in exchange for improved consistency and other benefits
from using the component

However, since we have a custom version of the table component, it's
very easy for us to support custom classes. This was in fact done in the
same PR that copies the component into the Signon codebase, adding
custom class support for the outer table element in 4f09fbc

This adds support for custom classes in the headers, which is then used
to restore the old classes and therefore consistent column widths
between tables (irrespective of content width)

In #2341, it was suggested that we might try to get the customisations
merged upstream - it might be worth exploring that with both this and
earlier changes as a separate piece of work

#2341: https://github.com/alphagov/signon/pull/2341
#2664: https://github.com/alphagov/signon/pull/2664
---
 app/helpers/components/table_helper.rb          | 1 +
 app/views/account/applications/index.html.erb   | 8 ++++----
 app/views/api_users/applications/index.html.erb | 4 ++--
 app/views/components/_table.html.erb            | 1 +
 app/views/users/applications/index.html.erb     | 8 ++++----
 5 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/app/helpers/components/table_helper.rb b/app/helpers/components/table_helper.rb
index 982601420..fe47237ec 100644
--- a/app/helpers/components/table_helper.rb
+++ b/app/helpers/components/table_helper.rb
@@ -50,6 +50,7 @@ def row
 
       def header(str, opt = {})
         classes = %w[govuk-table__header]
+        classes << opt[:classes] if opt[:classes]
         classes << "govuk-table__header--#{opt[:format]}" if opt[:format]
         classes << "govuk-table__header--active" if opt[:sort_direction]
         link_classes = %w[app-table__sort-link]
diff --git a/app/views/account/applications/index.html.erb b/app/views/account/applications/index.html.erb
index 46e275894..e851107cb 100644
--- a/app/views/account/applications/index.html.erb
+++ b/app/views/account/applications/index.html.erb
@@ -29,8 +29,8 @@
 <%= render "components/table", {
     caption: "Apps you have access to",
     head: [
-      { text: "Name" },
-      { text: "Description" },
+      { text: "Name", classes: "govuk-!-width-one-quarter" },
+      { text: "Description", classes: "govuk-!-width-one-third" },
       { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") },
     ],
     rows: @applications_with_signin.map do |application|
@@ -47,8 +47,8 @@
 <%= render "components/table", {
     caption: "Apps you don't have access to",
     head: [
-      { text: "Name" },
-      { text: "Description" },
+      { text: "Name", classes: "govuk-!-width-one-quarter" },
+      { text: "Description", classes: "govuk-!-width-one-third" },
       { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") }
     ],
     rows: @applications_without_signin.map do |application|
diff --git a/app/views/api_users/applications/index.html.erb b/app/views/api_users/applications/index.html.erb
index e15f35476..0b59d98ce 100644
--- a/app/views/api_users/applications/index.html.erb
+++ b/app/views/api_users/applications/index.html.erb
@@ -34,8 +34,8 @@
 <%= render "components/table", {
     caption: "Apps #{@api_user.name} has access to",
     head: [
-      { text: "Name" },
-      { text: "Description" },
+      { text: "Name", classes: "govuk-!-width-one-quarter" },
+      { text: "Description", classes: "govuk-!-width-one-third" },
       { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") },
     ],
     rows: @applications.map do |application|
diff --git a/app/views/components/_table.html.erb b/app/views/components/_table.html.erb
index 5ec626729..930abb54a 100644
--- a/app/views/components/_table.html.erb
+++ b/app/views/components/_table.html.erb
@@ -27,6 +27,7 @@
       <%= t.head do %>
         <% head.each_with_index do |item, cellindex| %>
           <%= t.header item[:text], {
+            classes: item[:classes],
             format: item[:format],
             href: item[:href],
             data_attributes: item[:data_attributes],
diff --git a/app/views/users/applications/index.html.erb b/app/views/users/applications/index.html.erb
index 854d7431e..33bb21726 100644
--- a/app/views/users/applications/index.html.erb
+++ b/app/views/users/applications/index.html.erb
@@ -34,8 +34,8 @@
 <%= render "components/table", {
     caption: "Apps #{@user.name} has access to",
     head: [
-      { text: "Name" },
-      { text: "Description" },
+      { text: "Name", classes: "govuk-!-width-one-quarter" },
+      { text: "Description", classes: "govuk-!-width-one-third" },
       { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") },
     ],
     rows: @applications_with_signin.map do |application|
@@ -52,8 +52,8 @@
 <%= render "components/table", {
     caption: "Apps #{@user.name} does not have access to",
     head: [
-      { text: "Name" },
-      { text: "Description" },
+      { text: "Name", classes: "govuk-!-width-one-quarter" },
+      { text: "Description", classes: "govuk-!-width-one-third" },
       { text: content_tag(:span, "Actions", class: "govuk-visually-hidden") }
     ],
     rows: @applications_without_signin.map do |application|