Skip to content

Commit

Permalink
Fix row alignment in applications tables
Browse files Browse the repository at this point in the history
In 901963c, styles were added to right align the actions in the final
column of the applications tables. This was described as adopting a
Whitehall-style pattern, and it does follow the pattern of wrapping the
applications tables in a `div` with the class
`govuk-table--with-actions` and applying alignment styles to the last
column in the wrapped table, however the actual applied styles are quite
different. In Whitehall, it's a simple `text-align: right`, whereas in
Signon flex-based styling was introduced. The commit message explained
that this was done so that we could have a gap between elements, since
in Signon we have buttons/button links in addition to simple text links

However, since this change, we've had issues with the height of the
cells in the actions column. This has led to some broken/misaligned row
borders under certain conditions (variation in whether cells need to
wrap their text content)

Using flex on the actual table cell appears to be the issue here: we
lose the consistent height that table cells provide for free. In this
commit, we therefore pivot to wrapping the contents of actions cells in
a classed `div` on which the styles are now applied, which retains the
custom alignment and gap for the actions but restores the default table
styling on the cell itself. This fixes the height/border/row alignment
issues

The `flex-direction` is also switched to `column` at smaller viewport
sizes so that we don't end up with very narrow links/buttons
  • Loading branch information
yndajas committed Jun 28, 2024
1 parent e162e35 commit fa48f49
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 18 deletions.
14 changes: 9 additions & 5 deletions app/assets/stylesheets/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,15 @@ $govuk-page-width: 1140px;
}
}

.govuk-table--with-actions {
.govuk-table__cell:last-child {
display: flex;
justify-content: flex-end;
.govuk-table__actions {
display: flex;
flex-direction: column;
align-items: end;
gap: 10px;

@media (min-width: 40.0625em) {
flex-direction: row;
align-items: center;
gap: 10px;
justify-content: flex-end;
}
}
4 changes: 4 additions & 0 deletions app/helpers/application_table_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
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)
Expand Down
10 changes: 6 additions & 4 deletions app/views/account/applications/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
<% end %>
<% end %>

<div class="govuk-table--with-actions">
<%= render "components/table", {
caption: "Apps you have access to",
head: [
Expand All @@ -37,11 +36,14 @@
[
{ text: application.name },
{ text: application.description },
{ text: safe_join([account_applications_permissions_link(application), account_applications_remove_access_link(application)]) },
{ text: wrap_links_in_actions_markup([
account_applications_permissions_link(application),
account_applications_remove_access_link(application)
])
},
]
end,
} %>
</div>

<div class="govuk-table--with-actions">
<%= render "components/table", {
Expand All @@ -55,7 +57,7 @@
[
{ text: application.name },
{ text: application.description },
{ text: account_applications_grant_access_link(application) }
{ text: wrap_links_in_actions_markup([account_applications_grant_access_link(application)]) }
]
end,
} %>
Expand Down
4 changes: 1 addition & 3 deletions app/views/api_users/applications/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
<% end %>
<% end %>

<div class="govuk-table--with-actions">
<%= render "components/table", {
caption: "Apps #{@api_user.name} has access to",
head: [
Expand All @@ -42,8 +41,7 @@
[
{ text: application.name },
{ text: application.description },
{ text: api_users_applications_permissions_link(application, @api_user) }
{ text: wrap_links_in_actions_markup([api_users_applications_permissions_link(application, @api_user)]) }
]
end,
} %>
</div>
12 changes: 6 additions & 6 deletions app/views/users/applications/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
<% end %>
<% end %>

<div class="govuk-table--with-actions">
<%= render "components/table", {
caption: "Apps #{@user.name} has access to",
head: [
Expand All @@ -42,13 +41,15 @@
[
{ text: application.name },
{ text: application.description },
{ text: safe_join([users_applications_permissions_link(application, @user), users_applications_remove_access_link(application, @user)]) },
{ text: wrap_links_in_actions_markup([
users_applications_permissions_link(application, @user),
users_applications_remove_access_link(application, @user)
])
},
]
end,
} %>
</div>

<div class="govuk-table--with-actions">
<%= render "components/table", {
caption: "Apps #{@user.name} does not have access to",
head: [
Expand All @@ -60,8 +61,7 @@
[
{ text: application.name },
{ text: application.description },
{ text: users_applications_grant_access_link(application, @user) }
{ text: wrap_links_in_actions_markup([users_applications_grant_access_link(application, @user)]) }
]
end,
} %>
</div>
16 changes: 16 additions & 0 deletions test/helpers/application_table_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@
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)
Expand Down

0 comments on commit fa48f49

Please sign in to comment.