Skip to content

Commit

Permalink
Update applications table layout
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@yndajas
yndajas committed Oct 1, 2024
1 parent 1c48de6 commit 5fce1c2
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 42 deletions.
13 changes: 13 additions & 0 deletions app/assets/stylesheets/application.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
10 changes: 5 additions & 5 deletions app/helpers/application_table_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,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)
Expand All @@ -85,9 +85,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)
Expand All @@ -97,7 +97,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)
Expand All @@ -115,7 +115,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)
Expand Down
18 changes: 8 additions & 10 deletions app/views/account/applications/index.html.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
18 changes: 8 additions & 10 deletions app/views/users/applications/index.html.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
34 changes: 17 additions & 17 deletions test/helpers/application_table_helper_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,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
Expand All @@ -54,7 +54,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
Expand All @@ -78,7 +78,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
Expand All @@ -104,7 +104,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
Expand All @@ -130,26 +130,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
Expand Down Expand Up @@ -178,8 +178,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
Expand All @@ -193,8 +193,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
Expand All @@ -208,8 +208,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
Expand All @@ -234,7 +234,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

Expand Down

0 comments on commit 5fce1c2

Please sign in to comment.