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 1/5] 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 From 7d590031f5856b7d570ef680a1cf37265845eb92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 30 May 2021 15:01:53 +0200 Subject: [PATCH 2/5] Remove redundant words in edit and destroy links When we see a list of, let's say, banners, and each one has a link to edit them, the word "banner" in the text "edit banner" is redundant and adds noise; even for users with cognitive disabilities, it's obvious that the "edit" link refers to the banner. --- .../admin/budget_phases/phases_component.html.erb | 3 +-- .../admin/budgets/table_actions_component.html.erb | 1 - app/views/admin/banners/index.html.erb | 7 +------ app/views/admin/milestones/_milestones.html.erb | 6 +----- app/views/admin/officials/index.html.erb | 3 +-- app/views/admin/officials/search.html.erb | 2 +- app/views/admin/poll/booths/_booth.html.erb | 5 +---- .../site_customization/content_blocks/index.html.erb | 6 +----- app/views/admin/tags/index.html.erb | 2 +- config/locales/en/admin.yml | 8 -------- config/locales/es/admin.yml | 8 -------- .../admin/budgets/table_actions_component_spec.rb | 2 +- spec/shared/system/admin_milestoneable.rb | 2 +- spec/system/admin/banners_spec.rb | 6 +++--- spec/system/admin/budget_phases_spec.rb | 2 +- spec/system/admin/budgets_spec.rb | 2 +- spec/system/admin/budgets_wizard/budgets_spec.rb | 2 +- spec/system/admin/budgets_wizard/phases_spec.rb | 4 ++-- spec/system/admin/budgets_wizard/wizard_spec.rb | 2 +- spec/system/admin/officials_spec.rb | 2 +- spec/system/admin/poll/booths_spec.rb | 4 ++-- .../admin/site_customization/content_blocks_spec.rb | 2 +- spec/system/admin/tags_spec.rb | 4 ++-- 23 files changed, 25 insertions(+), 60 deletions(-) diff --git a/app/components/admin/budget_phases/phases_component.html.erb b/app/components/admin/budget_phases/phases_component.html.erb index 836ca925e..918dc6484 100644 --- a/app/components/admin/budget_phases/phases_component.html.erb +++ b/app/components/admin/budget_phases/phases_component.html.erb @@ -31,8 +31,7 @@ <%= render Admin::TableActionsComponent.new(phase, actions: [:edit], - edit_path: edit_path(phase), - edit_text: t("admin.budgets.edit.edit_phase") + edit_path: edit_path(phase) ) %> diff --git a/app/components/admin/budgets/table_actions_component.html.erb b/app/components/admin/budgets/table_actions_component.html.erb index 9eba6926a..90677909e 100644 --- a/app/components/admin/budgets/table_actions_component.html.erb +++ b/app/components/admin/budgets/table_actions_component.html.erb @@ -1,5 +1,4 @@ <%= render Admin::TableActionsComponent.new(budget, - edit_text: t("admin.budgets.index.edit_budget"), destroy_confirmation: t("admin.actions.confirm_delete", resource_name: t("admin.budgets.shared.resource_name"), name: budget.name) ) do %> diff --git a/app/views/admin/banners/index.html.erb b/app/views/admin/banners/index.html.erb index bd71afb5f..a81d0671d 100644 --- a/app/views/admin/banners/index.html.erb +++ b/app/views/admin/banners/index.html.erb @@ -20,12 +20,7 @@ <%= banner.post_started_at %> <%= banner.post_ended_at %> - - <%= render Admin::TableActionsComponent.new(banner, - edit_text: t("admin.banners.index.edit"), - destroy_text: t("admin.banners.index.delete") - ) %> - + <%= render Admin::TableActionsComponent.new(banner) %> diff --git a/app/views/admin/milestones/_milestones.html.erb b/app/views/admin/milestones/_milestones.html.erb index ae80be88d..14e99f1b2 100644 --- a/app/views/admin/milestones/_milestones.html.erb +++ b/app/views/admin/milestones/_milestones.html.erb @@ -53,11 +53,7 @@ <% end %> <% end %> - - <%= render Admin::TableActionsComponent.new(milestone, - destroy_text: t("admin.milestones.index.delete") - ) %> - + <%= render Admin::TableActionsComponent.new(milestone) %> <% end %> diff --git a/app/views/admin/officials/index.html.erb b/app/views/admin/officials/index.html.erb index f3c930434..6212778cc 100644 --- a/app/views/admin/officials/index.html.erb +++ b/app/views/admin/officials/index.html.erb @@ -28,8 +28,7 @@ <%= render Admin::TableActionsComponent.new( actions: [:edit], - edit_path: edit_admin_official_path(official), - edit_text: t("admin.officials.search.edit_official") + edit_path: edit_admin_official_path(official) ) %> diff --git a/app/views/admin/officials/search.html.erb b/app/views/admin/officials/search.html.erb index 9aaf8c847..bf9706207 100644 --- a/app/views/admin/officials/search.html.erb +++ b/app/views/admin/officials/search.html.erb @@ -35,7 +35,7 @@ <%= render Admin::TableActionsComponent.new( actions: [:edit], edit_path: edit_admin_official_path(user), - edit_text: user.official? ? t("admin.officials.search.edit_official") : t("admin.officials.search.make_official") + edit_text: (t("admin.officials.search.make_official") unless user.official?) ) %> diff --git a/app/views/admin/poll/booths/_booth.html.erb b/app/views/admin/poll/booths/_booth.html.erb index 5c9c5d839..efa5a172c 100644 --- a/app/views/admin/poll/booths/_booth.html.erb +++ b/app/views/admin/poll/booths/_booth.html.erb @@ -13,10 +13,7 @@ class: "shifts-link" %> <% end %> <% else %> - <%= render Admin::TableActionsComponent.new(booth, - actions: [:edit], - edit_text: t("admin.booths.booth.edit") - ) %> + <%= render Admin::TableActionsComponent.new(booth, actions: [:edit]) %> <% end %> diff --git a/app/views/admin/site_customization/content_blocks/index.html.erb b/app/views/admin/site_customization/content_blocks/index.html.erb index d7a632709..79a6f44f2 100644 --- a/app/views/admin/site_customization/content_blocks/index.html.erb +++ b/app/views/admin/site_customization/content_blocks/index.html.erb @@ -34,10 +34,7 @@ <%= link_to "#{content_block.name} (#{content_block.locale})", edit_admin_site_customization_content_block_path(content_block) %> <%= raw content_block.body %> - <%= render Admin::TableActionsComponent.new(content_block, - actions: [:destroy], - destroy_text: t("admin.site_customization.content_blocks.index.delete") - ) %> + <%= render Admin::TableActionsComponent.new(content_block, actions: [:destroy]) %> <% end %> @@ -48,7 +45,6 @@ <%= render Admin::TableActionsComponent.new( actions: [:destroy], - destroy_text: t("admin.site_customization.content_blocks.index.delete"), destroy_path: admin_site_customization_delete_heading_content_block_path(content_block) ) %> diff --git a/app/views/admin/tags/index.html.erb b/app/views/admin/tags/index.html.erb index e2bfaeac3..39583f933 100644 --- a/app/views/admin/tags/index.html.erb +++ b/app/views/admin/tags/index.html.erb @@ -30,7 +30,7 @@ <% end %> - <%= render Admin::TableActionsComponent.new(tag, actions: [:destroy], destroy_text: t("admin.tags.destroy")) %> + <%= render Admin::TableActionsComponent.new(tag, actions: [:destroy]) %> <% end %> diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 315641017..380cd2e09 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -21,8 +21,6 @@ en: index: title: Banners create: Create banner - edit: Edit banner - delete: Delete banner filters: all: All with_active: Active @@ -90,7 +88,6 @@ en: type_pending: "Pending: No headings yet" type_single: "Single heading" edit_groups: Edit headings groups - edit_budget: Edit budget admin_ballots: Admin ballots no_budgets: "There are no budgets." create: @@ -109,7 +106,6 @@ en: duration: "Duration" enabled: Enabled actions: Actions - edit_phase: Edit phase enable_phase: "Enable %{phase} phase" active: Active blank_dates: Dates are blank @@ -354,7 +350,6 @@ en: table_publication_date: "Publication date" table_status: Status table_actions: "Actions" - delete: "Delete milestone" no_milestones: "Don't have defined milestones" image: "Image" show_image: "Show image" @@ -1207,7 +1202,6 @@ en: location: "Location" booth: shifts: "Manage shifts" - edit: "Edit booth" officials: edit: destroy: Remove "Official" status @@ -1229,7 +1223,6 @@ en: level_4: Level 4 level_5: Level 5 search: - edit_official: Edit official make_official: Make official title: "Official positions: User search" no_results: Official positions not found. @@ -1513,7 +1506,6 @@ en: title: "Sustainable Development Goals - Stats" tags: create: Create topic - destroy: Delete topic index: add_tag: Add a new proposal topic title: Proposal topics diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index bd53a4758..ad62f9be0 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -21,8 +21,6 @@ es: index: title: Banners create: Crear un banner - edit: Editar banner - delete: Eliminar banner filters: all: Todos with_active: Activos @@ -90,7 +88,6 @@ es: type_pending: "Pendiente: Aún no hay partidas" type_single: "Partida única" edit_groups: Editar grupos de partidas - edit_budget: Editar presupuesto admin_ballots: Gestionar urnas no_budgets: "No hay presupuestos participativos." create: @@ -109,7 +106,6 @@ es: duration: "Duración" enabled: Habilitada actions: Acciones - edit_phase: Editar fase enable_phase: "Habilitar fase de %{phase}" active: Activa blank_dates: Sin fechas @@ -354,7 +350,6 @@ es: table_publication_date: "Fecha de publicación" table_status: Estado table_actions: "Acciones" - delete: "Eliminar hito" no_milestones: "No hay hitos definidos" image: "Imagen" show_image: "Mostrar imagen" @@ -1206,7 +1201,6 @@ es: location: "Ubicación" booth: shifts: "Asignar turnos" - edit: "Editar urna" officials: edit: destroy: Eliminar condición de 'Cargo Público' @@ -1228,7 +1222,6 @@ es: level_4: Nivel 4 level_5: Nivel 5 search: - edit_official: Editar cargo público make_official: Convertir en cargo público title: "Cargos Públicos: Búsqueda de usuarios" no_results: No se han encontrado cargos públicos. @@ -1512,7 +1505,6 @@ es: title: "Objetivos de Desarrollo Sostenible - Estadísticas" tags: create: Crear tema - destroy: Eliminar tema index: add_tag: Añade un nuevo tema de propuesta title: Temas de propuesta diff --git a/spec/components/admin/budgets/table_actions_component_spec.rb b/spec/components/admin/budgets/table_actions_component_spec.rb index 119fdffd4..49d84a9af 100644 --- a/spec/components/admin/budgets/table_actions_component_spec.rb +++ b/spec/components/admin/budgets/table_actions_component_spec.rb @@ -14,7 +14,7 @@ describe Admin::Budgets::TableActionsComponent, type: :component do expect(page).to have_css "a", count: 6 expect(page).to have_link "Manage projects", href: /investments/ expect(page).to have_link "Edit headings groups", href: /groups/ - expect(page).to have_link "Edit budget", href: /edit/ + expect(page).to have_link "Edit", href: /edit/ expect(page).to have_link "Admin ballots" expect(page).to have_link "Preview budget", href: /budgets/ expect(page).to have_link "Delete", href: /budgets/ diff --git a/spec/shared/system/admin_milestoneable.rb b/spec/shared/system/admin_milestoneable.rb index a5b41284b..51996fa18 100644 --- a/spec/shared/system/admin_milestoneable.rb +++ b/spec/shared/system/admin_milestoneable.rb @@ -113,7 +113,7 @@ shared_examples "admin_milestoneable" do |factory_name, path_name| visit path - accept_confirm { click_link "Delete milestone" } + accept_confirm { click_link "Delete" } expect(page).not_to have_content "Title will it remove" end diff --git a/spec/system/admin/banners_spec.rb b/spec/system/admin/banners_spec.rb index 29f0fcb12..c066e915f 100644 --- a/spec/system/admin/banners_spec.rb +++ b/spec/system/admin/banners_spec.rb @@ -104,7 +104,7 @@ describe "Admin banners magement", :admin do fill_in "Post ended at", with: Date.current + 1.week click_button "Save changes" - click_link "Edit banner" + click_link "Edit" expect_to_have_language_selected "Français" expect(page).to have_field "Title", with: "En Français" @@ -137,7 +137,7 @@ describe "Admin banners magement", :admin do click_link "Manage banners" end - click_link "Edit banner" + click_link "Edit" fill_in "Title", with: "Modified title" fill_in "Description", with: "Edited text" @@ -173,7 +173,7 @@ describe "Admin banners magement", :admin do expect(page).to have_content "Ugly banner" - accept_confirm { click_link "Delete banner" } + accept_confirm { click_link "Delete" } visit admin_root_path expect(page).not_to have_content "Ugly banner" diff --git a/spec/system/admin/budget_phases_spec.rb b/spec/system/admin/budget_phases_spec.rb index 5ecd061ca..b4b7f4a7a 100644 --- a/spec/system/admin/budget_phases_spec.rb +++ b/spec/system/admin/budget_phases_spec.rb @@ -29,7 +29,7 @@ describe "Admin budget phases" do expect(page).to have_content "Accepting projects" expect(page).not_to have_content "My phase custom name" - within("tr", text: "Accepting projects") { click_link "Edit phase" } + within("tr", text: "Accepting projects") { click_link "Edit" } end expect(page).to have_css "h2", exact_text: "Edit phase - Accepting projects" diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb index b7c2b9395..5dfa2315b 100644 --- a/spec/system/admin/budgets_spec.rb +++ b/spec/system/admin/budgets_spec.rb @@ -227,7 +227,7 @@ describe "Admin budgets", :admin do within_table "Phases" do within "tr", text: "Information" do - expect(page).to have_link "Edit phase" + expect(page).to have_link "Edit" end end end diff --git a/spec/system/admin/budgets_wizard/budgets_spec.rb b/spec/system/admin/budgets_wizard/budgets_spec.rb index 71bd953af..6c509b4c6 100644 --- a/spec/system/admin/budgets_wizard/budgets_spec.rb +++ b/spec/system/admin/budgets_wizard/budgets_spec.rb @@ -88,7 +88,7 @@ describe "Budgets wizard, first step", :admin do expect(page).to have_content "New participatory budget created successfully!" within("#side_menu") { click_link "Participatory budgets" } - within("tr", text: "M30 - Summer campaign") { click_link "Edit budget" } + within("tr", text: "M30 - Summer campaign") { click_link "Edit" } expect(page).to have_content "This participatory budget is in draft mode" expect(page).to have_link "Preview budget" diff --git a/spec/system/admin/budgets_wizard/phases_spec.rb b/spec/system/admin/budgets_wizard/phases_spec.rb index e1f8e7be2..72edc9e6c 100644 --- a/spec/system/admin/budgets_wizard/phases_spec.rb +++ b/spec/system/admin/budgets_wizard/phases_spec.rb @@ -56,7 +56,7 @@ describe "Budgets wizard, phases step", :admin do expect(page).to have_css ".creation-timeline" - within("tr", text: "Selecting projects") { click_link "Edit phase" } + within("tr", text: "Selecting projects") { click_link "Edit" } fill_in "Name", with: "Choosing projects" click_button "Save changes" @@ -86,7 +86,7 @@ describe "Budgets wizard, phases step", :admin do scenario "update phase in single heading budget" do visit admin_budgets_wizard_budget_budget_phases_path(budget, mode: "single") - within("tr", text: "Selecting projects") { click_link "Edit phase" } + within("tr", text: "Selecting projects") { click_link "Edit" } fill_in "Name", with: "Choosing projects" click_button "Save changes" diff --git a/spec/system/admin/budgets_wizard/wizard_spec.rb b/spec/system/admin/budgets_wizard/wizard_spec.rb index 2cd89198e..75a686553 100644 --- a/spec/system/admin/budgets_wizard/wizard_spec.rb +++ b/spec/system/admin/budgets_wizard/wizard_spec.rb @@ -107,7 +107,7 @@ describe "Budgets creation wizard", :admin do expect(page).to have_css ".budget-phases-table" - within("tr", text: "Voting projects") { click_link "Edit phase" } + within("tr", text: "Voting projects") { click_link "Edit" } fill_in "Name", with: "Custom phase name" uncheck "Phase enabled" click_button "Save changes" diff --git a/spec/system/admin/officials_spec.rb b/spec/system/admin/officials_spec.rb index 2cd86c085..c71dfe928 100644 --- a/spec/system/admin/officials_spec.rb +++ b/spec/system/admin/officials_spec.rb @@ -15,7 +15,7 @@ describe "Admin officials", :admin do scenario "Edit an official" do visit admin_officials_path - click_link "Edit official" + click_link "Edit" expect(page).to have_current_path(edit_admin_official_path(official)) diff --git a/spec/system/admin/poll/booths_spec.rb b/spec/system/admin/poll/booths_spec.rb index 19051b9fa..eb6cead7e 100644 --- a/spec/system/admin/poll/booths_spec.rb +++ b/spec/system/admin/poll/booths_spec.rb @@ -47,7 +47,7 @@ describe "Admin booths", :admin do expect(page).to have_content booth_for_current_poll.name expect(page).not_to have_content booth_for_expired_poll.name - expect(page).not_to have_link "Edit booth" + expect(page).not_to have_link "Edit" end scenario "Show" do @@ -82,7 +82,7 @@ describe "Admin booths", :admin do within("#booth_#{booth.id}") do expect(page).not_to have_link "Manage shifts" - click_link "Edit booth" + click_link "Edit" end fill_in "poll_booth_name", with: "Next booth" diff --git a/spec/system/admin/site_customization/content_blocks_spec.rb b/spec/system/admin/site_customization/content_blocks_spec.rb index 7b9bceadd..599de43d5 100644 --- a/spec/system/admin/site_customization/content_blocks_spec.rb +++ b/spec/system/admin/site_customization/content_blocks_spec.rb @@ -91,7 +91,7 @@ describe "Admin custom content blocks", :admin do expect(page).to have_content("#{block.name} (#{block.locale})") expect(page).to have_content(block.body) - accept_confirm { click_link "Delete block" } + accept_confirm { click_link "Delete" } expect(page).not_to have_content("#{block.name} (#{block.locale})") expect(page).not_to have_content(block.body) diff --git a/spec/system/admin/tags_spec.rb b/spec/system/admin/tags_spec.rb index 45d17fad5..7d6d2ee78 100644 --- a/spec/system/admin/tags_spec.rb +++ b/spec/system/admin/tags_spec.rb @@ -39,7 +39,7 @@ describe "Admin tags", :admin do expect(page).to have_content "bad tag" within("#tag_#{tag2.id}") do - accept_confirm { click_link "Delete topic" } + accept_confirm { click_link "Delete" } end expect(page).not_to have_content "bad tag" @@ -57,7 +57,7 @@ describe "Admin tags", :admin do expect(page).to have_content "bad tag" within("#tag_#{tag2.id}") do - accept_confirm { click_link "Delete topic" } + accept_confirm { click_link "Delete" } end expect(page).not_to have_content "bad tag" From 6f04c8f0577f24732dc505ac9845a645fa8c76dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 30 May 2021 15:39:43 +0200 Subject: [PATCH 3/5] Use conventions in page actions texts We use the words "Manage" and "View" in other places; we don't use the word "See" as often. --- config/locales/en/admin.yml | 4 ++-- config/locales/es/admin.yml | 4 ++-- spec/system/admin/widgets/cards_spec.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 380cd2e09..c00aa7ab2 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1593,7 +1593,7 @@ en: create: Create new page delete: Delete page title: Custom Pages - see_page: See page + see_page: View new: title: Create new custom page slug_help: "Text to identify this page on URL, for example https://consulproject.org/page-slug" @@ -1603,7 +1603,7 @@ en: updated_at: Updated at title: Title slug: Slug - see_cards: See Cards + see_cards: Manage cards cards: cards_title: cards create_card: Create card diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index ad62f9be0..80c85fe29 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1592,7 +1592,7 @@ es: create: Crear nueva página delete: Borrar página title: Páginas - see_page: Ver página + see_page: Ver new: title: Página nueva slug_help: "Texto que identifica esta página en la URL, por ejemplo https://consulproject.org/slug-de-pagina" @@ -1602,7 +1602,7 @@ es: updated_at: Última actualización title: Título slug: Slug - see_cards: Ver tarjetas + see_cards: Tarjetas cards: cards_title: tarjetas create_card: Crear tarjeta diff --git a/spec/system/admin/widgets/cards_spec.rb b/spec/system/admin/widgets/cards_spec.rb index 4f7477142..dac6a03a7 100644 --- a/spec/system/admin/widgets/cards_spec.rb +++ b/spec/system/admin/widgets/cards_spec.rb @@ -163,7 +163,7 @@ describe "Cards", :admin do visit admin_site_customization_pages_path within "#site_customization_page_#{custom_page.id}" do - click_link "See Cards" + click_link "Manage cards" end click_link "Create card" From 5531a0b2bca9e67bbccd402e9ec8e31c40564c53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 6 Jun 2021 03:03:21 +0200 Subject: [PATCH 4/5] Simplify action links in budgets table The word "budget" in the "Preview budget" link is redundant. On the other hand, the words "Manage", Edit" and "Admin" are not really necessary in my humble opinion. Just like in the admin navigation menu we use "Participatory budgets" instead of "Manage Participatory budgets", the fact that we're going to manage or admin or edit something can be deduced from the fact that we're in the admin section. Besides, it isn't clear to me why we use "Manage" for projects, "Edit" for heading groups and "Admin" for ballots. The differences between these three concepts might be too subtle for me. The previous paragraphs haven't been corroborated with real users, though, so I might be mistaken and we might need to revisit these links in the future. These actions still take quite a lot of space. Maybe in the future we could remove the "delete" icon, at least on budgets which cannot be deleted. --- config/locales/en/admin.yml | 10 +++++----- config/locales/es/admin.yml | 10 +++++----- .../admin/budgets/table_actions_component_spec.rb | 12 ++++++------ spec/system/admin/budget_groups_spec.rb | 6 +++--- spec/system/admin/budgets_spec.rb | 4 ++-- spec/system/admin/budgets_wizard/budgets_spec.rb | 2 +- spec/system/admin/budgets_wizard/wizard_spec.rb | 8 ++++---- spec/system/budget_polls/budgets_spec.rb | 4 ++-- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index c00aa7ab2..3cdebaab7 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -67,7 +67,7 @@ en: no_activity: There are no moderators activity. budgets: actions: - preview: "Preview budget" + preview: "Preview" index: title: Participatory budgets new_link: Create new budget @@ -76,7 +76,7 @@ en: all: All open: Open finished: Finished - budget_investments: Manage projects + budget_investments: Investment projects table_budget_type: "Type" table_completed: Completed table_draft: "Draft" @@ -87,8 +87,8 @@ en: type_multiple: "Multiple headings" type_pending: "Pending: No headings yet" type_single: "Single heading" - edit_groups: Edit headings groups - admin_ballots: Admin ballots + edit_groups: Heading groups + admin_ballots: Ballots no_budgets: "There are no budgets." create: notice: New participatory budget created successfully! @@ -152,7 +152,7 @@ en: budget_groups: name: "Name" headings_name: "Headings" - headings_manage: "Manage headings" + headings_manage: "Headings" no_groups: "There are no groups." amount: one: "There is 1 group" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 80c85fe29..f71ae9eaa 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -67,7 +67,7 @@ es: no_activity: No hay actividad de moderadores. budgets: actions: - preview: "Previsualizar presupuesto" + preview: "Previsualizar" index: title: Presupuestos participativos new_link: Crear nuevo presupuesto @@ -76,7 +76,7 @@ es: all: Todos open: Abiertos finished: Terminados - budget_investments: Gestionar proyectos de gasto + budget_investments: Proyectos de gasto table_budget_type: "Tipo" table_completed: Completado table_draft: "Borrador" @@ -87,8 +87,8 @@ es: type_multiple: "Múltiples partidas" type_pending: "Pendiente: Aún no hay partidas" type_single: "Partida única" - edit_groups: Editar grupos de partidas - admin_ballots: Gestionar urnas + edit_groups: Grupos de partidas + admin_ballots: Urnas no_budgets: "No hay presupuestos participativos." create: notice: '¡Presupuestos participativos creados con éxito!' @@ -152,7 +152,7 @@ es: budget_groups: name: "Nombre" headings_name: "Partidas" - headings_manage: "Gestionar partidas" + headings_manage: "Partidas" no_groups: "No hay grupos." amount: one: "Hay 1 grupo de partidas presupuestarias" diff --git a/spec/components/admin/budgets/table_actions_component_spec.rb b/spec/components/admin/budgets/table_actions_component_spec.rb index 49d84a9af..692dd3b64 100644 --- a/spec/components/admin/budgets/table_actions_component_spec.rb +++ b/spec/components/admin/budgets/table_actions_component_spec.rb @@ -12,18 +12,18 @@ describe Admin::Budgets::TableActionsComponent, type: :component do render_inline component expect(page).to have_css "a", count: 6 - expect(page).to have_link "Manage projects", href: /investments/ - expect(page).to have_link "Edit headings groups", href: /groups/ + expect(page).to have_link "Investment projects", href: /investments/ + expect(page).to have_link "Heading groups", href: /groups/ expect(page).to have_link "Edit", href: /edit/ - expect(page).to have_link "Admin ballots" - expect(page).to have_link "Preview budget", href: /budgets/ + expect(page).to have_link "Ballots" + expect(page).to have_link "Preview", href: /budgets/ expect(page).to have_link "Delete", href: /budgets/ end it "renders link to create new poll for budgets without polls" do render_inline component - expect(page).to have_css "a[href*='polls'][data-method='post']", text: "Admin ballots" + expect(page).to have_css "a[href*='polls'][data-method='post']", text: "Ballots" end it "renders link to manage ballots for budgets with polls" do @@ -31,6 +31,6 @@ describe Admin::Budgets::TableActionsComponent, type: :component do render_inline component - expect(page).to have_link "Admin ballots", href: /booth_assignments/ + expect(page).to have_link "Ballots", href: /booth_assignments/ end end diff --git a/spec/system/admin/budget_groups_spec.rb b/spec/system/admin/budget_groups_spec.rb index 2dd148185..d502a45bb 100644 --- a/spec/system/admin/budget_groups_spec.rb +++ b/spec/system/admin/budget_groups_spec.rb @@ -37,21 +37,21 @@ describe "Admin budget groups", :admin do expect(page).to have_content(group1.name) expect(page).to have_content(group1.max_votable_headings) expect(page).to have_content(group1.headings.count) - expect(page).to have_link "Manage headings" + expect(page).to have_link "Headings" end within "#budget_group_#{group2.id}" do expect(page).to have_content(group2.name) expect(page).to have_content(group2.max_votable_headings) expect(page).to have_content(group2.headings.count) - expect(page).to have_link "Manage headings" + expect(page).to have_link "Headings" end within "#budget_group_#{group3.id}" do expect(page).to have_content(group3.name) expect(page).to have_content(group3.max_votable_headings) expect(page).to have_content(group3.headings.count) - expect(page).to have_link "Manage headings" + expect(page).to have_link "Headings" end end diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb index 5dfa2315b..ed4255094 100644 --- a/spec/system/admin/budgets_spec.rb +++ b/spec/system/admin/budgets_spec.rb @@ -116,7 +116,7 @@ describe "Admin budgets", :admin do scenario "Can preview budget before it is published" do visit edit_admin_budget_path(budget) - within_window(window_opened_by { click_link "Preview budget" }) do + within_window(window_opened_by { click_link "Preview" }) do expect(page).to have_current_path budget_path(budget) end end @@ -130,7 +130,7 @@ describe "Admin budgets", :admin do expect(page).not_to have_content "This participatory budget is in draft mode" expect(page).not_to have_link "Publish budget" - within_window(window_opened_by { click_link "Preview budget" }) do + within_window(window_opened_by { click_link "Preview" }) do expect(page).to have_current_path budget_path(budget) end end diff --git a/spec/system/admin/budgets_wizard/budgets_spec.rb b/spec/system/admin/budgets_wizard/budgets_spec.rb index 6c509b4c6..e97b361c3 100644 --- a/spec/system/admin/budgets_wizard/budgets_spec.rb +++ b/spec/system/admin/budgets_wizard/budgets_spec.rb @@ -91,7 +91,7 @@ describe "Budgets wizard, first step", :admin do within("tr", text: "M30 - Summer campaign") { click_link "Edit" } expect(page).to have_content "This participatory budget is in draft mode" - expect(page).to have_link "Preview budget" + expect(page).to have_link "Preview" expect(page).to have_link "Publish budget" end end diff --git a/spec/system/admin/budgets_wizard/wizard_spec.rb b/spec/system/admin/budgets_wizard/wizard_spec.rb index 75a686553..b388feac0 100644 --- a/spec/system/admin/budgets_wizard/wizard_spec.rb +++ b/spec/system/admin/budgets_wizard/wizard_spec.rb @@ -27,13 +27,13 @@ describe "Budgets creation wizard", :admin do expect(page).to have_content "Phases configured successfully" within "tr", text: "Single heading budget" do - click_link "Edit headings groups" + click_link "Heading groups" end expect(page).to have_content "There is 1 group" within "tr", text: "Single heading budget" do - click_link "Manage headings" + click_link "Headings" end expect(page).to have_content "There is 1 heading" @@ -124,7 +124,7 @@ describe "Budgets creation wizard", :admin do expect(page).to have_content "Phases configured successfully" within "tr", text: "Multiple headings budget" do - click_link "Edit headings groups" + click_link "Heading groups" end expect(page).to have_content "There are 2 groups" @@ -135,7 +135,7 @@ describe "Budgets creation wizard", :admin do end within "tr", text: "Districts" do - click_link "Manage headings" + click_link "Headings" end expect(page).to have_content "There are 2 headings" diff --git a/spec/system/budget_polls/budgets_spec.rb b/spec/system/budget_polls/budgets_spec.rb index ca97e317b..26af07349 100644 --- a/spec/system/budget_polls/budgets_spec.rb +++ b/spec/system/budget_polls/budgets_spec.rb @@ -8,7 +8,7 @@ describe "Admin Budgets", :admin do visit admin_budgets_path - click_link "Admin ballots" + click_link "Ballots" expect(page).to have_current_path(/admin\/polls\/\d+/) expect(page).to have_content(budget.name) @@ -37,7 +37,7 @@ describe "Admin Budgets", :admin do visit admin_budgets_path within "#budget_#{budget.id}" do - expect(page).to have_link "Admin ballots", href: admin_poll_booth_assignments_path(poll) + expect(page).to have_link "Ballots", href: admin_poll_booth_assignments_path(poll) end end end From 450d954526809bf422c3fda158a6a69d22d57824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 29 Jun 2021 16:18:47 +0200 Subject: [PATCH 5/5] Don't add default margin to font awesome icons We were removing the margin half of the time, and sometimes removing it made us use `!important` rules. --- app/assets/stylesheets/_consul_settings.scss | 1 + app/assets/stylesheets/admin/budgets/drafting.scss | 4 ++++ app/assets/stylesheets/admin/budgets/help.scss | 1 - app/assets/stylesheets/admin/table_actions.scss | 1 - app/assets/stylesheets/budgets/ballot/investment.scss | 4 ---- app/assets/stylesheets/budgets/investments-list.scss | 1 - app/assets/stylesheets/budgets/investments/ballot.scss | 4 ++++ app/assets/stylesheets/budgets/phases.scss | 1 - app/assets/stylesheets/layout.scss | 7 +++++++ app/assets/stylesheets/mixins/icons.scss | 1 - app/assets/stylesheets/notification_item.scss | 5 ++--- app/assets/stylesheets/participation.scss | 1 - 12 files changed, 18 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/_consul_settings.scss b/app/assets/stylesheets/_consul_settings.scss index f3dae6a48..034a19162 100644 --- a/app/assets/stylesheets/_consul_settings.scss +++ b/app/assets/stylesheets/_consul_settings.scss @@ -74,6 +74,7 @@ $outline-focus: 3px solid #ffbf47 !default; $input-height: $line-height * 2 !default; +$font-icon-margin: rem-calc(4) !default; $icon-width: $line-height * 2 !default; $off-screen-left: -1000rem !default; diff --git a/app/assets/stylesheets/admin/budgets/drafting.scss b/app/assets/stylesheets/admin/budgets/drafting.scss index aa7cde254..28819e763 100644 --- a/app/assets/stylesheets/admin/budgets/drafting.scss +++ b/app/assets/stylesheets/admin/budgets/drafting.scss @@ -23,6 +23,10 @@ .preview-link { @include has-fa-icon(eye, regular); @include hollow-button; + + &::before { + margin-right: $font-icon-margin; + } } .publish-link { diff --git a/app/assets/stylesheets/admin/budgets/help.scss b/app/assets/stylesheets/admin/budgets/help.scss index e3bf1d6c0..0a055801f 100644 --- a/app/assets/stylesheets/admin/budgets/help.scss +++ b/app/assets/stylesheets/admin/budgets/help.scss @@ -36,7 +36,6 @@ &::after { bottom: $padding / 2; color: $white; - margin-right: 0; position: absolute; right: calc(#{$padding} + #{$quote-padding}); } diff --git a/app/assets/stylesheets/admin/table_actions.scss b/app/assets/stylesheets/admin/table_actions.scss index ff9bf0683..4129b20f4 100644 --- a/app/assets/stylesheets/admin/table_actions.scss +++ b/app/assets/stylesheets/admin/table_actions.scss @@ -22,7 +22,6 @@ &::before { font-size: 1.6em; - margin-right: 0 !important; } } diff --git a/app/assets/stylesheets/budgets/ballot/investment.scss b/app/assets/stylesheets/budgets/ballot/investment.scss index a09be82ab..19c490bdf 100644 --- a/app/assets/stylesheets/budgets/ballot/investment.scss +++ b/app/assets/stylesheets/budgets/ballot/investment.scss @@ -33,10 +33,6 @@ position: absolute; right: $close-icon-margin; top: $close-icon-margin; - - &::before { - margin-right: 0; - } } &:hover { diff --git a/app/assets/stylesheets/budgets/investments-list.scss b/app/assets/stylesheets/budgets/investments-list.scss index 8846ff592..5d19f4a45 100644 --- a/app/assets/stylesheets/budgets/investments-list.scss +++ b/app/assets/stylesheets/budgets/investments-list.scss @@ -71,7 +71,6 @@ &::after { font-size: 2em; margin-left: auto; - margin-right: 0; transform: translateY(-25%); } } diff --git a/app/assets/stylesheets/budgets/investments/ballot.scss b/app/assets/stylesheets/budgets/investments/ballot.scss index e2d141cda..c98c861ec 100644 --- a/app/assets/stylesheets/budgets/investments/ballot.scss +++ b/app/assets/stylesheets/budgets/investments/ballot.scss @@ -6,5 +6,9 @@ color: $brand-secondary; font-weight: bold; margin-top: $line-height; + + &::before { + margin-right: $font-icon-margin; + } } } diff --git a/app/assets/stylesheets/budgets/phases.scss b/app/assets/stylesheets/budgets/phases.scss index 4f06a78ae..2cf816322 100644 --- a/app/assets/stylesheets/budgets/phases.scss +++ b/app/assets/stylesheets/budgets/phases.scss @@ -204,7 +204,6 @@ &::after { font-weight: bold; margin-left: 0.5em; - margin-right: 0; } } } diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 313d06903..9c9398e60 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1470,6 +1470,7 @@ table { &::before { font-size: rem-calc(24); + margin-right: $font-icon-margin; } } @@ -1863,6 +1864,7 @@ table { .show-children::before, .collapse-children::before { + margin-right: $font-icon-margin; transform: translateY(-1px); } @@ -1891,6 +1893,7 @@ table { } &::before { + margin-right: $font-icon-margin; transform: translateY(-1px); } } @@ -2456,6 +2459,10 @@ table { @include has-fa-icon(times, solid); color: $color-alert; } + + &::before { + margin-right: $font-icon-margin; + } } } diff --git a/app/assets/stylesheets/mixins/icons.scss b/app/assets/stylesheets/mixins/icons.scss index e7cf32e2c..c524f70ae 100644 --- a/app/assets/stylesheets/mixins/icons.scss +++ b/app/assets/stylesheets/mixins/icons.scss @@ -1,7 +1,6 @@ %font-icon { @extend %fa-icon; font-family: "Font Awesome 5 Free"; - margin-right: rem-calc(4); vertical-align: middle; } diff --git a/app/assets/stylesheets/notification_item.scss b/app/assets/stylesheets/notification_item.scss index a6d2ee1e5..d4bbc5aab 100644 --- a/app/assets/stylesheets/notification_item.scss +++ b/app/assets/stylesheets/notification_item.scss @@ -7,8 +7,8 @@ @include has-fa-icon(circle, solid, after); &::before { - @include breakpoint(medium) { - margin-right: 0; + @include breakpoint(small only) { + margin-right: $font-icon-margin; } } @@ -18,7 +18,6 @@ color: #ecf00b; font-size: $circle-icon-size; - margin-right: 0; position: absolute; left: $notification-icon-size - rem-calc(5); top: $menu-link-top-padding - $circle-icon-size / 2; diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 167ca4003..9cbebc07b 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -1215,7 +1215,6 @@ &::after { margin-left: $line-height / 4; - margin-right: 0; } } }