From 25e9065913c59074562aebc2e3362d173a7b7f0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 21 May 2021 23:04:48 +0200 Subject: [PATCH] Use icons with text in admin table actions In commit 9794ffbbf, we replaced "buttons" with icons in order to make the admin interface consistent with the planned budget investments redesign. However, using icons has some issues. For once, icons like a trash for the "delete" action might be obvious, but other icons like "confirm moderation" or "send pending" might be a bit confusing. It's true that we were adding tooltips on hover. We tried two approaches: using Foundation's tooltips and using CSS tooltips. Foundation tooltips are not activated on focus (only on hover), while CSS tooltips always appear below the icon, which might be a problem when the icons are at the bottom of the screen (one of our milestone tests was failing because of that and we can now run it with JavaScript enabled). Both Foundation and CSS tooltips have other issues: * They force users to make an extra step and move the mouse over the link just to know what the link is about * They aren't available on touch screens, so these users will have to memorize what each icon does * They are not hoverable, and making them hoverable would cause a different issue because the tooltip might cover links below it, making it impossible to click these links without moving the mouse away first * They are not dismissable, which is considered an accessibility issue and a requirement in the Web Content Accessibility Guidelines [1] For all these reasons, we're using both texts and icons. As Thomas Byttebier said "The best icon is a text label [2]". Heydon Pickering also makes a point towards providing text alongside icons in his book "Inclusive Components" [3]. Note that, since we're now adding text and some of the colors we use for actions are hard to read against a white/gray background, we're making a few colors darker. With these changes, actions take more space in the admin table compared to the space they took in version 1.3, but they are more usable and accessible while they still take less space than they did in version 1.2. [1] https://www.w3.org/WAI/WCAG21/Understanding/content-on-hover-or-focus [2] https://thomasbyttebier.be/blog/the-best-icon-is-a-text-label [3] https://inclusive-components.design/tooltips-toggletips/ --- .../stylesheets/admin/table_actions.scss | 50 ++++++++++--------- .../admin/budgets/table_actions_component.rb | 1 - .../admin/hidden_table_actions_component.rb | 1 - .../organizations/table_actions_component.rb | 1 - .../admin/roles/table_actions_component.rb | 1 - .../admin/table_actions_component.rb | 1 - app/components/concerns/table_action_link.rb | 7 --- spec/shared/system/admin_milestoneable.rb | 4 +- 8 files changed, 28 insertions(+), 38 deletions(-) delete mode 100644 app/components/concerns/table_action_link.rb diff --git a/app/assets/stylesheets/admin/table_actions.scss b/app/assets/stylesheets/admin/table_actions.scss index b217105ba..ff9bf0683 100644 --- a/app/assets/stylesheets/admin/table_actions.scss +++ b/app/assets/stylesheets/admin/table_actions.scss @@ -6,33 +6,23 @@ } a { + align-items: center; + display: flex; + flex-direction: column; + font-size: 0.9em; + line-height: $global-lineheight; + margin-right: 1em; position: relative; - - > :first-child { - @include bottom-tooltip; - left: $off-screen-left; - opacity: 0; - transform: translateX(-50%); - transition: opacity 0.3s, left 0s 0.3s; - } + text-align: center; &:hover, &:focus { color: $link-hover; - - > :first-child { - left: 50%; - opacity: 1; - transition: opacity 0.4s 0.2s; - } - } - - &:not(:focus) > :first-child:hover { - left: $off-screen-left; } &::before { - font-size: rem-calc(18); + font-size: 1.6em; + margin-right: 0 !important; } } @@ -42,7 +32,14 @@ .destroy-link { @include has-fa-icon(trash-alt, regular); - color: $alert-color; + } + + .destroy-link, + .destroy-role-link, + .destroy-officer-link, + .reject-link, + .confirm-hide-link { + color: darken($alert-color, 5%); } .show-link, @@ -66,17 +63,23 @@ .destroy-officer-link, .reject-link { @include has-fa-icon(user-times, solid); - color: $alert-color; } .restore-link { @include has-fa-icon(undo, solid); - color: $warning-color; + } + + .restore-link, + .investments-link { + color: darken($warning-color, 20%); + + &::before { + color: $warning-color; + } } .confirm-hide-link { @include has-fa-icon(flag, regular); - color: $alert-color; } .verify-link { @@ -108,7 +111,6 @@ .investments-link { @include has-fa-icon(coins, solid); - color: $warning-color; } .groups-link, diff --git a/app/components/admin/budgets/table_actions_component.rb b/app/components/admin/budgets/table_actions_component.rb index 46c642a86..9a90a3799 100644 --- a/app/components/admin/budgets/table_actions_component.rb +++ b/app/components/admin/budgets/table_actions_component.rb @@ -1,5 +1,4 @@ class Admin::Budgets::TableActionsComponent < ApplicationComponent - include TableActionLink attr_reader :budget def initialize(budget) diff --git a/app/components/admin/hidden_table_actions_component.rb b/app/components/admin/hidden_table_actions_component.rb index 41788be0b..68647d130 100644 --- a/app/components/admin/hidden_table_actions_component.rb +++ b/app/components/admin/hidden_table_actions_component.rb @@ -1,5 +1,4 @@ class Admin::HiddenTableActionsComponent < ApplicationComponent - include TableActionLink attr_reader :record def initialize(record) diff --git a/app/components/admin/organizations/table_actions_component.rb b/app/components/admin/organizations/table_actions_component.rb index 786755b74..d0b29708f 100644 --- a/app/components/admin/organizations/table_actions_component.rb +++ b/app/components/admin/organizations/table_actions_component.rb @@ -1,5 +1,4 @@ class Admin::Organizations::TableActionsComponent < ApplicationComponent - include TableActionLink delegate :can?, to: :controller attr_reader :organization diff --git a/app/components/admin/roles/table_actions_component.rb b/app/components/admin/roles/table_actions_component.rb index d674321af..57d19f169 100644 --- a/app/components/admin/roles/table_actions_component.rb +++ b/app/components/admin/roles/table_actions_component.rb @@ -1,5 +1,4 @@ class Admin::Roles::TableActionsComponent < ApplicationComponent - include TableActionLink attr_reader :record, :actions def initialize(record, actions: [:destroy]) diff --git a/app/components/admin/table_actions_component.rb b/app/components/admin/table_actions_component.rb index 7cc9697c5..f920447bc 100644 --- a/app/components/admin/table_actions_component.rb +++ b/app/components/admin/table_actions_component.rb @@ -1,5 +1,4 @@ class Admin::TableActionsComponent < ApplicationComponent - include TableActionLink include Admin::Namespace attr_reader :record, :options diff --git a/app/components/concerns/table_action_link.rb b/app/components/concerns/table_action_link.rb deleted file mode 100644 index 34a146059..000000000 --- a/app/components/concerns/table_action_link.rb +++ /dev/null @@ -1,7 +0,0 @@ -module TableActionLink - extend ActiveSupport::Concern - - def link_to(text, url, **options) - super(tag.span(text), url, options) - end -end diff --git a/spec/shared/system/admin_milestoneable.rb b/spec/shared/system/admin_milestoneable.rb index 5c7e3f630..a5b41284b 100644 --- a/spec/shared/system/admin_milestoneable.rb +++ b/spec/shared/system/admin_milestoneable.rb @@ -108,12 +108,12 @@ shared_examples "admin_milestoneable" do |factory_name, path_name| end context "Delete" do - scenario "Remove milestone", :no_js do + scenario "Remove milestone" do create(:milestone, milestoneable: milestoneable, title: "Title will it remove") visit path - click_link "Delete milestone" + accept_confirm { click_link "Delete milestone" } expect(page).not_to have_content "Title will it remove" end