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|