From 99dad7a7b695cced262d3cfa631164d7ecb2c36a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 6 Jul 2020 21:59:56 +0200 Subject: [PATCH 1/5] Don't mix links and actions in an admin table In some tables, we had "actions", and some columns were also links pointing to some places. Having both of them at the same time is confusing, particularly since traditionally the links in the columns pointed to the same place as some of the actions (although that's not the case since commit 48db31cd). We're still keeping links in tables which don't have an action column. For instance, the proposals table has a "select" button which would be harder to use if we had action buttons next to it. --- .../admin/legislation/processes/index.html.erb | 8 ++------ .../admin/milestones/_milestones.html.erb | 5 +---- .../admin/poll/questions/_questions.html.erb | 7 +++++-- .../questions/_successful_proposals.html.erb | 3 ++- .../site_customization/pages/index.html.erb | 18 ++++++------------ .../admin/valuators/_valuator_row.html.erb | 8 ++++++-- config/locales/en/admin.yml | 1 - config/locales/es/admin.yml | 1 - spec/shared/system/admin_milestoneable.rb | 2 +- .../admin/legislation/draft_versions_spec.rb | 9 +++------ .../system/admin/legislation/processes_spec.rb | 4 ++-- .../system/admin/legislation/questions_spec.rb | 9 +++------ .../admin/site_customization/pages_spec.rb | 2 +- 13 files changed, 32 insertions(+), 45 deletions(-) diff --git a/app/views/admin/legislation/processes/index.html.erb b/app/views/admin/legislation/processes/index.html.erb index ddef4a150..159f553c7 100644 --- a/app/views/admin/legislation/processes/index.html.erb +++ b/app/views/admin/legislation/processes/index.html.erb @@ -27,16 +27,12 @@ <% @processes.each do |process| %> - - <%= link_to process.title, edit_admin_legislation_process_path(process) %> - + <%= process.title %> <%= t("admin.legislation.processes.process.status_#{process.status}") %> <%= I18n.l process.start_date %> <%= I18n.l process.end_date %> <%= process.total_comments %> - - <%= render Admin::TableActionsComponent.new(process, actions: [:destroy]) %> - + <%= render Admin::TableActionsComponent.new(process) %> <% end %> diff --git a/app/views/admin/milestones/_milestones.html.erb b/app/views/admin/milestones/_milestones.html.erb index f3c1e1991..b36a7c545 100644 --- a/app/views/admin/milestones/_milestones.html.erb +++ b/app/views/admin/milestones/_milestones.html.erb @@ -30,9 +30,7 @@ <% milestoneable.milestones.order_by_publication_date.each do |milestone| %> <%= milestone.id %> - - <%= link_to milestone.title, admin_polymorphic_path(milestone, action: :edit) %> - + <%= milestone.title %> <%= milestone.description %> <%= l(milestone.publication_date.to_date) if milestone.publication_date.present? %> @@ -57,7 +55,6 @@ <%= render Admin::TableActionsComponent.new(milestone, - actions: [:destroy], destroy_text: t("admin.milestones.index.delete") ) %> diff --git a/app/views/admin/poll/questions/_questions.html.erb b/app/views/admin/poll/questions/_questions.html.erb index 784d3e73b..b8d411689 100644 --- a/app/views/admin/poll/questions/_questions.html.erb +++ b/app/views/admin/poll/questions/_questions.html.erb @@ -18,7 +18,7 @@ <% @questions.each do |question| %> - <%= link_to question.title, admin_question_path(question) %> + <%= question.title %> <% if question.poll.present? %> <%= question.poll.name %> @@ -27,7 +27,10 @@ <% end %> - <%= render Admin::TableActionsComponent.new(question) %> + <%= render Admin::TableActionsComponent.new(question) do |actions| %> + <%= actions.link_to t("admin.polls.show.edit_answers"), admin_question_path(question), + class: "button hollow" %> + <% end %> <% end %> diff --git a/app/views/admin/poll/questions/_successful_proposals.html.erb b/app/views/admin/poll/questions/_successful_proposals.html.erb index 88bd6e477..b0e91ebfa 100644 --- a/app/views/admin/poll/questions/_successful_proposals.html.erb +++ b/app/views/admin/poll/questions/_successful_proposals.html.erb @@ -9,13 +9,14 @@ <% @proposals.each do |proposal| %> - <%= link_to proposal.title, proposal_path(proposal) %> + <%= proposal.title %>

<%= proposal.summary %>

<%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> + <%= actions.link_to t("admin.shared.view"), proposal_path(proposal), class: "button hollow" %> <%= actions.link_to t("admin.questions.index.create_question"), new_admin_question_path(proposal_id: proposal.id), class: "button hollow" %> diff --git a/app/views/admin/site_customization/pages/index.html.erb b/app/views/admin/site_customization/pages/index.html.erb index 359ba932f..47f31da97 100644 --- a/app/views/admin/site_customization/pages/index.html.erb +++ b/app/views/admin/site_customization/pages/index.html.erb @@ -13,7 +13,6 @@ <%= t("admin.site_customization.pages.page.title") %> <%= t("admin.site_customization.pages.page.slug") %> - <%= t("admin.site_customization.pages.page.cards_title") %> <%= t("admin.site_customization.pages.page.created_at") %> <%= t("admin.site_customization.pages.page.updated_at") %> <%= t("admin.site_customization.pages.page.status") %> @@ -23,22 +22,17 @@ <% @pages.each do |page| %> - - <%= link_to page.title, edit_admin_site_customization_page_path(page) %> - + <%= page.title %> <%= page.slug %> - - <%= link_to t("admin.site_customization.pages.page.see_cards"), admin_site_customization_page_cards_path(page), - class: "button hollow expanded" %> - <%= I18n.l page.created_at, format: :short %> <%= I18n.l page.created_at, format: :short %> <%= t("admin.site_customization.pages.page.status_#{page.status}") %> - <%= render Admin::TableActionsComponent.new(page, - actions: [:destroy], - destroy_text: t("admin.site_customization.pages.index.delete") - ) do |actions| %> + <%= render Admin::TableActionsComponent.new(page) do |actions| %> + <%= actions.link_to t("admin.site_customization.pages.page.see_cards"), + admin_site_customization_page_cards_path(page), + class: "button hollow expanded" %> + <% if page.status == "published" %> <%= actions.link_to t("admin.site_customization.pages.index.see_page"), page.url, diff --git a/app/views/admin/valuators/_valuator_row.html.erb b/app/views/admin/valuators/_valuator_row.html.erb index a93d492d7..6a8f28968 100644 --- a/app/views/admin/valuators/_valuator_row.html.erb +++ b/app/views/admin/valuators/_valuator_row.html.erb @@ -1,5 +1,5 @@ - <%= link_to valuator.name, admin_valuator_path(valuator) %> + <%= valuator.name %> <%= valuator.email %> <% if valuator.description.present? %> @@ -19,6 +19,10 @@ <%= valuator_abilities(valuator) %> - <%= render Admin::TableActionsComponent.new(valuator) %> + <%= render Admin::TableActionsComponent.new(valuator) do |actions| %> + <%= actions.link_to t("admin.shared.view"), + admin_valuator_path(valuator), + class: "button hollow" %> + <% end %> diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index a7960eee4..346b59c09 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1512,7 +1512,6 @@ en: updated_at: Updated at title: Title slug: Slug - cards_title: Cards see_cards: See Cards cards: cards_title: cards diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 586cc1b70..2feeab0d6 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1511,7 +1511,6 @@ es: updated_at: Última actualización title: Título slug: Slug - cards_title: Tarjetas see_cards: Ver tarjetas cards: cards_title: tarjetas diff --git a/spec/shared/system/admin_milestoneable.rb b/spec/shared/system/admin_milestoneable.rb index 5374e0a4a..bdbd3e3bb 100644 --- a/spec/shared/system/admin_milestoneable.rb +++ b/spec/shared/system/admin_milestoneable.rb @@ -90,7 +90,7 @@ shared_examples "admin_milestoneable" do |factory_name, path_name| visit path expect(page).to have_link document.title - click_link milestone.title + within("tr", text: milestone.title) { click_link "Edit" } expect(page).to have_css("img[alt='#{milestone.image.title}']") diff --git a/spec/system/admin/legislation/draft_versions_spec.rb b/spec/system/admin/legislation/draft_versions_spec.rb index 05e41194c..9423fa55f 100644 --- a/spec/system/admin/legislation/draft_versions_spec.rb +++ b/spec/system/admin/legislation/draft_versions_spec.rb @@ -21,7 +21,7 @@ describe "Admin legislation draft versions" do visit admin_legislation_processes_path(filter: "all") - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } click_link "Drafting" click_link "Version 1" @@ -41,10 +41,7 @@ describe "Admin legislation draft versions" do end click_link "All" - - expect(page).to have_content "An example legislation process" - - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } click_link "Drafting" click_link "Create version" @@ -77,7 +74,7 @@ describe "Admin legislation draft versions" do expect(page).not_to have_link "All" - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } click_link "Drafting" click_link "Version 1" diff --git a/spec/system/admin/legislation/processes_spec.rb b/spec/system/admin/legislation/processes_spec.rb index e85fdc353..ce766faa0 100644 --- a/spec/system/admin/legislation/processes_spec.rb +++ b/spec/system/admin/legislation/processes_spec.rb @@ -201,7 +201,7 @@ describe "Admin collaborative legislation" do click_link "Collaborative Legislation" end - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } expect(page).to have_selector("h2", text: "An example legislation process") expect(find("#legislation_process_debate_phase_enabled")).to be_checked @@ -224,7 +224,7 @@ describe "Admin collaborative legislation" do click_link "Collaborative Legislation" end - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } expect(find("#legislation_process_draft_publication_enabled")).to be_checked diff --git a/spec/system/admin/legislation/questions_spec.rb b/spec/system/admin/legislation/questions_spec.rb index c2db6cdf8..3c70fed27 100644 --- a/spec/system/admin/legislation/questions_spec.rb +++ b/spec/system/admin/legislation/questions_spec.rb @@ -25,7 +25,7 @@ describe "Admin legislation questions" do visit admin_legislation_processes_path(filter: "all") - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } click_link "Debate" expect(page).to have_content("Question 1") @@ -43,9 +43,7 @@ describe "Admin legislation questions" do click_link "All" - expect(page).to have_content "An example legislation process" - - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } click_link "Debate" click_link "Create question" @@ -71,9 +69,8 @@ describe "Admin legislation questions" do expect(page).not_to have_link "All" - click_link "An example legislation process" + within("tr", text: "An example legislation process") { click_link "Edit" } click_link "Debate" - click_link "Question 2" fill_in "Question", with: "Question 2b" diff --git a/spec/system/admin/site_customization/pages_spec.rb b/spec/system/admin/site_customization/pages_spec.rb index 3671ba64a..066324e26 100644 --- a/spec/system/admin/site_customization/pages_spec.rb +++ b/spec/system/admin/site_customization/pages_spec.rb @@ -70,7 +70,7 @@ describe "Admin custom pages" do click_link "Custom pages" end - click_link "An example custom page" + within("tr", text: "An example custom page") { click_link "Edit" } expect(page).to have_selector("h2", text: "An example custom page") expect(page).to have_selector("input[value='custom-example-page']") From ccb7695056beb63c2fb86d4c85f8294539c2ed69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 30 Jun 2020 19:26:07 +0200 Subject: [PATCH 2/5] Use a custom link_to method for table actions This way we'll be able to change the behavior of these links without changing the view nor affecting the rest of the application. --- app/components/admin/budgets/table_actions_component.rb | 1 + app/components/admin/hidden_table_actions_component.rb | 1 + .../admin/organizations/table_actions_component.rb | 1 + app/components/admin/roles/table_actions_component.rb | 1 + app/components/admin/table_actions_component.rb | 1 + app/components/concerns/table_action_link.rb | 7 +++++++ 6 files changed, 12 insertions(+) create mode 100644 app/components/concerns/table_action_link.rb diff --git a/app/components/admin/budgets/table_actions_component.rb b/app/components/admin/budgets/table_actions_component.rb index 75dab1850..0250eb1d7 100644 --- a/app/components/admin/budgets/table_actions_component.rb +++ b/app/components/admin/budgets/table_actions_component.rb @@ -1,4 +1,5 @@ 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 68647d130..41788be0b 100644 --- a/app/components/admin/hidden_table_actions_component.rb +++ b/app/components/admin/hidden_table_actions_component.rb @@ -1,4 +1,5 @@ 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 d0b29708f..786755b74 100644 --- a/app/components/admin/organizations/table_actions_component.rb +++ b/app/components/admin/organizations/table_actions_component.rb @@ -1,4 +1,5 @@ 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 57d19f169..d674321af 100644 --- a/app/components/admin/roles/table_actions_component.rb +++ b/app/components/admin/roles/table_actions_component.rb @@ -1,4 +1,5 @@ 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 339dc4c6c..e43c5294d 100644 --- a/app/components/admin/table_actions_component.rb +++ b/app/components/admin/table_actions_component.rb @@ -1,4 +1,5 @@ class Admin::TableActionsComponent < ApplicationComponent + include TableActionLink attr_reader :record, :options def initialize(record = nil, **options) diff --git a/app/components/concerns/table_action_link.rb b/app/components/concerns/table_action_link.rb new file mode 100644 index 000000000..bca6a91ad --- /dev/null +++ b/app/components/concerns/table_action_link.rb @@ -0,0 +1,7 @@ +module TableActionLink + extend ActiveSupport::Concern + + def link_to(text, url, **options) + super(text, url, options) + end +end From 8c1140a1bf7a1a471dd21fcd794eed5723e12406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 1 Jul 2020 19:09:22 +0200 Subject: [PATCH 3/5] Use semantic HTML classes in table actions Note the CSS could probably be improved to avoid duplication with other button style definitions. However, that's fine because we're going to change the style of the links soon. For the same reason, I haven't bothered to style every single link the way it was until now. --- app/assets/stylesheets/admin/table_actions.scss | 13 +++++++++++++ .../admin/budgets/table_actions_component.html.erb | 10 +++++++--- .../admin/budgets/table_actions_component.rb | 10 ++++++---- .../admin/hidden_table_actions_component.html.erb | 4 ++-- .../organizations/table_actions_component.html.erb | 4 ++-- .../admin/poll/officers/officers_component.html.erb | 7 ++++--- .../admin/roles/table_actions_component.html.erb | 7 +++++-- app/components/admin/table_actions_component.rb | 4 ++-- app/views/admin/admin_notifications/index.html.erb | 6 +++--- app/views/admin/administrators/search.html.erb | 2 +- app/views/admin/audits/_audits.html.erb | 2 +- app/views/admin/banners/index.html.erb | 2 +- app/views/admin/budget_groups/index.html.erb | 3 ++- app/views/admin/dashboard/actions/index.html.erb | 2 +- app/views/admin/geozones/index.html.erb | 2 +- app/views/admin/managers/search.html.erb | 2 +- app/views/admin/milestones/_milestones.html.erb | 2 +- app/views/admin/moderators/search.html.erb | 2 +- app/views/admin/newsletters/index.html.erb | 4 ++-- .../booth_assignments/_booth_assignment.html.erb | 3 +-- app/views/admin/poll/booths/_booth.html.erb | 2 +- app/views/admin/poll/polls/_poll.html.erb | 2 +- app/views/admin/poll/polls/_questions.html.erb | 6 +++--- .../admin/poll/polls/booth_assignments.html.erb | 2 +- app/views/admin/poll/questions/_questions.html.erb | 6 +++--- .../poll/questions/_successful_proposals.html.erb | 6 +++--- .../admin/poll/questions/answers/documents.html.erb | 2 +- app/views/admin/poll/shifts/_shifts.html.erb | 2 +- .../admin/site_customization/pages/index.html.erb | 6 +++--- app/views/admin/system_emails/index.html.erb | 6 +++--- app/views/admin/valuators/_valuator_row.html.erb | 2 +- app/views/admin/valuators/_valuators.html.erb | 2 +- 32 files changed, 79 insertions(+), 56 deletions(-) diff --git a/app/assets/stylesheets/admin/table_actions.scss b/app/assets/stylesheets/admin/table_actions.scss index 9d782a4be..0409e43a9 100644 --- a/app/assets/stylesheets/admin/table_actions.scss +++ b/app/assets/stylesheets/admin/table_actions.scss @@ -9,4 +9,17 @@ > p { align-self: flex-start; } + + a { + @include button($style: hollow); + border-color: $link !important; + color: $link !important; + font-size: $base-font-size; + margin-bottom: 0; + + &.destroy-link { + border-color: $alert-color !important; + color: $alert-color !important; + } + } } diff --git a/app/components/admin/budgets/table_actions_component.html.erb b/app/components/admin/budgets/table_actions_component.html.erb index 9540163e0..8bc788c34 100644 --- a/app/components/admin/budgets/table_actions_component.html.erb +++ b/app/components/admin/budgets/table_actions_component.html.erb @@ -1,10 +1,14 @@ <%= render Admin::TableActionsComponent.new(budget, actions: [:edit], edit_text: t("admin.budgets.index.edit_budget")) do %> <%= link_to t("admin.budgets.index.budget_investments"), admin_budget_budget_investments_path(budget_id: budget.id), - class: "button hollow medium" %> - <%= link_to t("admin.budgets.index.edit_groups"), admin_budget_groups_path(budget) %> + class: "investments-link" %> + <%= link_to t("admin.budgets.index.edit_groups"), + admin_budget_groups_path(budget), + class: "groups-link" %> <% if budget.poll.present? %> - <%= link_to t("admin.budgets.index.admin_ballots"), admin_poll_booth_assignments_path(budget.poll) %> + <%= link_to t("admin.budgets.index.admin_ballots"), + admin_poll_booth_assignments_path(budget.poll), + class: "ballots-link" %> <% else %> <%= link_to_create_budget_poll %> <% end %> diff --git a/app/components/admin/budgets/table_actions_component.rb b/app/components/admin/budgets/table_actions_component.rb index 0250eb1d7..46c642a86 100644 --- a/app/components/admin/budgets/table_actions_component.rb +++ b/app/components/admin/budgets/table_actions_component.rb @@ -13,10 +13,12 @@ class Admin::Budgets::TableActionsComponent < ApplicationComponent link_to t("admin.budgets.index.admin_ballots"), admin_polls_path(poll: { - name: budget.name, - budget_id: budget.id, - starts_at: balloting_phase.starts_at, - ends_at: balloting_phase.ends_at }), + name: budget.name, + budget_id: budget.id, + starts_at: balloting_phase.starts_at, + ends_at: balloting_phase.ends_at + }), + class: "ballots-link", method: :post end end diff --git a/app/components/admin/hidden_table_actions_component.html.erb b/app/components/admin/hidden_table_actions_component.html.erb index 7cf58e888..dad308a78 100644 --- a/app/components/admin/hidden_table_actions_component.html.erb +++ b/app/components/admin/hidden_table_actions_component.html.erb @@ -2,11 +2,11 @@ <%= link_to restore_text, restore_path, method: :put, data: { confirm: t("admin.actions.confirm") }, - class: "button hollow warning" %> + class: "restore-link" %> <% unless record.confirmed_hide? %> <%= link_to confirm_hide_text, confirm_hide_path, method: :put, - class: "button" %> + class: "confirm-hide-link" %> <% end %> <% end %> diff --git a/app/components/admin/organizations/table_actions_component.html.erb b/app/components/admin/organizations/table_actions_component.html.erb index 5c0b565ab..dd9c521a1 100644 --- a/app/components/admin/organizations/table_actions_component.html.erb +++ b/app/components/admin/organizations/table_actions_component.html.erb @@ -2,12 +2,12 @@ <% if can_verify? %> <%= link_to t("admin.organizations.index.verify"), verify_admin_organization_path(organization, request.query_parameters), - method: :put, class: "button success small-5" %> + method: :put, class: "verify-link" %> <% end %> <% if can_reject? %> <%= link_to t("admin.organizations.index.reject"), reject_admin_organization_path(organization, request.query_parameters), - method: :put, class: "button hollow alert small-5" %> + method: :put, class: "reject-link" %> <% end %> <% end %> diff --git a/app/components/admin/poll/officers/officers_component.html.erb b/app/components/admin/poll/officers/officers_component.html.erb index baf93a50f..3ef36fd13 100644 --- a/app/components/admin/poll/officers/officers_component.html.erb +++ b/app/components/admin/poll/officers/officers_component.html.erb @@ -3,7 +3,7 @@ <%= t("admin.poll_officers.officer.name") %> <%= t("admin.poll_officers.officer.email") %> - <%= t("admin.actions.actions") %> + <%= t("admin.actions.actions") %> @@ -19,14 +19,15 @@ <% if officer.persisted? %> <%= render Admin::TableActionsComponent.new(officer, actions: [:destroy], - destroy_text: t("admin.poll_officers.officer.delete") + destroy_text: t("admin.poll_officers.officer.delete"), + destroy_options: { class: "destroy-officer-link" } ) %> <% else %> <%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> <%= actions.link_to t("admin.poll_officers.officer.add"), add_user_path(officer), method: :post, - class: "button success expanded" %> + class: "create-officer-link" %> <% end %> <% end %> diff --git a/app/components/admin/roles/table_actions_component.html.erb b/app/components/admin/roles/table_actions_component.html.erb index 0531450cc..59aed5763 100644 --- a/app/components/admin/roles/table_actions_component.html.erb +++ b/app/components/admin/roles/table_actions_component.html.erb @@ -1,7 +1,10 @@ <% if already_has_role? %> - <%= render Admin::TableActionsComponent.new(record, actions: actions) %> + <%= render Admin::TableActionsComponent.new(record, + actions: actions, + destroy_options: { class: "destroy-role-link" } + ) %> <% else %> <%= render Admin::TableActionsComponent.new(actions: []) do %> - <%= link_to add_user_text, add_user_path, method: :post, class: "button success expanded" %> + <%= link_to add_user_text, add_user_path, method: :post, class: "create-role-link" %> <% end %> <% end %> diff --git a/app/components/admin/table_actions_component.rb b/app/components/admin/table_actions_component.rb index e43c5294d..990f54c74 100644 --- a/app/components/admin/table_actions_component.rb +++ b/app/components/admin/table_actions_component.rb @@ -22,7 +22,7 @@ class Admin::TableActionsComponent < ApplicationComponent end def edit_options - { class: "button hollow" }.merge(options[:edit_options] || {}) + { class: "edit-link" }.merge(options[:edit_options] || {}) end def destroy_text @@ -36,7 +36,7 @@ class Admin::TableActionsComponent < ApplicationComponent def destroy_options { method: :delete, - class: "button hollow alert", + class: "destroy-link", data: { confirm: destroy_confirmation } }.merge(options[:destroy_options] || {}) end diff --git a/app/views/admin/admin_notifications/index.html.erb b/app/views/admin/admin_notifications/index.html.erb index ad284ce11..d51ac2c8b 100644 --- a/app/views/admin/admin_notifications/index.html.erb +++ b/app/views/admin/admin_notifications/index.html.erb @@ -9,7 +9,7 @@ <%= t("admin.admin_notifications.index.title") %> <%= t("admin.admin_notifications.index.segment_recipient") %> <%= t("admin.admin_notifications.index.sent") %> - <%= t("admin.admin_notifications.index.actions") %> + <%= t("admin.admin_notifications.index.actions") %> @@ -33,13 +33,13 @@ <%= render Admin::TableActionsComponent.new(admin_notification) do |actions| %> <%= actions.link_to t("admin.admin_notifications.index.preview"), admin_admin_notification_path(admin_notification), - class: "button" %> + class: "preview-link" %> <% end %> <% else %> <%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> <%= actions.link_to t("admin.admin_notifications.index.view"), admin_admin_notification_path(admin_notification), - class: "button" %> + class: "show-link" %> <% end %> <% end %> diff --git a/app/views/admin/administrators/search.html.erb b/app/views/admin/administrators/search.html.erb index d5ccddf38..e407ef4f5 100644 --- a/app/views/admin/administrators/search.html.erb +++ b/app/views/admin/administrators/search.html.erb @@ -10,7 +10,7 @@ <%= t("admin.administrators.index.name") %> <%= t("admin.administrators.index.email") %> - <%= t("admin.shared.actions") %> + <%= t("admin.shared.actions") %> <% @users.each do |user| %> diff --git a/app/views/admin/audits/_audits.html.erb b/app/views/admin/audits/_audits.html.erb index 684c246a3..aa72d36b8 100644 --- a/app/views/admin/audits/_audits.html.erb +++ b/app/views/admin/audits/_audits.html.erb @@ -37,7 +37,7 @@ <%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> <%= actions.link_to t("shared.show"), admin_polymorphic_path(audit), - class: "button hollow primary" %> + class: "show-link" %> <% end %> diff --git a/app/views/admin/banners/index.html.erb b/app/views/admin/banners/index.html.erb index 1a0783c2a..67b151d86 100644 --- a/app/views/admin/banners/index.html.erb +++ b/app/views/admin/banners/index.html.erb @@ -13,7 +13,7 @@ <%= Banner.human_attribute_name(:post_started_at) %> <%= Banner.human_attribute_name(:post_ended_at) %> - <%= t("admin.actions.actions") %> + <%= t("admin.actions.actions") %> diff --git a/app/views/admin/budget_groups/index.html.erb b/app/views/admin/budget_groups/index.html.erb index 293078dd3..195642d43 100644 --- a/app/views/admin/budget_groups/index.html.erb +++ b/app/views/admin/budget_groups/index.html.erb @@ -28,7 +28,8 @@ <%= render Admin::TableActionsComponent.new(group) do |actions| %> <%= actions.link_to t("admin.budget_groups.headings_manage"), - admin_budget_group_headings_path(@budget, group) %> + admin_budget_group_headings_path(@budget, group), + class: "headings-link" %> <% end %> diff --git a/app/views/admin/dashboard/actions/index.html.erb b/app/views/admin/dashboard/actions/index.html.erb index 2e32b97d6..38b4cba02 100644 --- a/app/views/admin/dashboard/actions/index.html.erb +++ b/app/views/admin/dashboard/actions/index.html.erb @@ -17,7 +17,7 @@ <%= t("admin.dashboard.actions.index.day_offset") %> <%= t("admin.dashboard.actions.index.required_supports") %> <%= t("admin.dashboard.actions.index.order") %> - <%= t("admin.actions.actions") %> + <%= t("admin.actions.actions") %> diff --git a/app/views/admin/geozones/index.html.erb b/app/views/admin/geozones/index.html.erb index 31bae3f73..c8dabf366 100644 --- a/app/views/admin/geozones/index.html.erb +++ b/app/views/admin/geozones/index.html.erb @@ -10,7 +10,7 @@ <%= t("admin.geozones.geozone.external_code") %> <%= t("admin.geozones.geozone.census_code") %> <%= t("admin.geozones.geozone.coordinates") %> - <%= t("admin.actions.actions") %> + <%= t("admin.actions.actions") %> diff --git a/app/views/admin/managers/search.html.erb b/app/views/admin/managers/search.html.erb index 442fe3194..60a5fcc54 100644 --- a/app/views/admin/managers/search.html.erb +++ b/app/views/admin/managers/search.html.erb @@ -10,7 +10,7 @@ <%= t("admin.managers.index.name") %> <%= t("admin.managers.index.email") %> - <%= t("admin.shared.actions") %> + <%= t("admin.shared.actions") %> <% @users.each do |user| %> diff --git a/app/views/admin/milestones/_milestones.html.erb b/app/views/admin/milestones/_milestones.html.erb index b36a7c545..ae80be88d 100644 --- a/app/views/admin/milestones/_milestones.html.erb +++ b/app/views/admin/milestones/_milestones.html.erb @@ -53,7 +53,7 @@ <% end %> <% end %> - + <%= render Admin::TableActionsComponent.new(milestone, destroy_text: t("admin.milestones.index.delete") ) %> diff --git a/app/views/admin/moderators/search.html.erb b/app/views/admin/moderators/search.html.erb index d3b1d69d1..2b4607947 100644 --- a/app/views/admin/moderators/search.html.erb +++ b/app/views/admin/moderators/search.html.erb @@ -10,7 +10,7 @@ <%= t("admin.moderators.index.name") %> <%= t("admin.moderators.index.email") %> - <%= t("admin.shared.actions") %> + <%= t("admin.shared.actions") %> <% @users.each do |user| %> diff --git a/app/views/admin/newsletters/index.html.erb b/app/views/admin/newsletters/index.html.erb index a86d704c3..e5c0e2427 100644 --- a/app/views/admin/newsletters/index.html.erb +++ b/app/views/admin/newsletters/index.html.erb @@ -9,7 +9,7 @@ <%= t("admin.newsletters.index.subject") %> <%= t("admin.newsletters.index.segment_recipient") %> <%= t("admin.newsletters.index.sent") %> - <%= t("admin.newsletters.index.actions") %> + <%= t("admin.newsletters.index.actions") %> @@ -32,7 +32,7 @@ <%= render Admin::TableActionsComponent.new(newsletter) do |actions| %> <%= actions.link_to t("admin.newsletters.index.preview"), admin_newsletter_path(newsletter), - class: "button" %> + class: "preview-link" %> <% end %> diff --git a/app/views/admin/poll/booth_assignments/_booth_assignment.html.erb b/app/views/admin/poll/booth_assignments/_booth_assignment.html.erb index acbddc7fa..fbf8664aa 100644 --- a/app/views/admin/poll/booth_assignments/_booth_assignment.html.erb +++ b/app/views/admin/poll/booth_assignments/_booth_assignment.html.erb @@ -31,8 +31,7 @@ admin_poll_booth_assignments_path(@poll, booth_id: booth.id), method: :post, remote: true, - title: t("admin.booth_assignments.manage.actions.assign"), - class: "button hollow expanded" %> + class: "assign-booth-link" %> <% end %> <% end %> diff --git a/app/views/admin/poll/booths/_booth.html.erb b/app/views/admin/poll/booths/_booth.html.erb index 8814016ed..5c9c5d839 100644 --- a/app/views/admin/poll/booths/_booth.html.erb +++ b/app/views/admin/poll/booths/_booth.html.erb @@ -10,7 +10,7 @@ <%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> <%= actions.link_to t("admin.booths.booth.shifts"), new_admin_booth_shift_path(booth), - class: "button hollow" %> + class: "shifts-link" %> <% end %> <% else %> <%= render Admin::TableActionsComponent.new(booth, diff --git a/app/views/admin/poll/polls/_poll.html.erb b/app/views/admin/poll/polls/_poll.html.erb index 2cad1f6cd..e367d7708 100644 --- a/app/views/admin/poll/polls/_poll.html.erb +++ b/app/views/admin/poll/polls/_poll.html.erb @@ -14,7 +14,7 @@ ) do |actions| %> <%= actions.link_to t("admin.actions.configure"), admin_poll_path(poll), - class: "button hollow " %> + class: "configure-link" %> <% end %> diff --git a/app/views/admin/poll/polls/_questions.html.erb b/app/views/admin/poll/polls/_questions.html.erb index 130f899c6..2cde94099 100644 --- a/app/views/admin/poll/polls/_questions.html.erb +++ b/app/views/admin/poll/polls/_questions.html.erb @@ -8,11 +8,11 @@ <%= t("admin.polls.show.no_questions") %> <% else %> - +
- + <% @poll.questions.each do |question| %> @@ -30,7 +30,7 @@ diff --git a/app/views/admin/poll/polls/booth_assignments.html.erb b/app/views/admin/poll/polls/booth_assignments.html.erb index 7fa9b2e01..ece854d29 100644 --- a/app/views/admin/poll/polls/booth_assignments.html.erb +++ b/app/views/admin/poll/polls/booth_assignments.html.erb @@ -18,7 +18,7 @@ <%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> <%= actions.link_to t("admin.booth_assignments.manage_assignments"), manage_admin_poll_booth_assignments_path(poll), - class: "button hollow" %> + class: "manage-link" %> <% end %> diff --git a/app/views/admin/poll/questions/_questions.html.erb b/app/views/admin/poll/questions/_questions.html.erb index b8d411689..d83428879 100644 --- a/app/views/admin/poll/questions/_questions.html.erb +++ b/app/views/admin/poll/questions/_questions.html.erb @@ -7,12 +7,12 @@ <%= t("admin.questions.index.no_questions") %> <% else %> -
<%= t("admin.polls.show.table_title") %><%= t("admin.actions.actions") %><%= t("admin.actions.actions") %>
<%= render Admin::TableActionsComponent.new(question) do |actions| %> <%= actions.link_to t("admin.polls.show.edit_answers"), admin_question_path(question), - class: "button hollow" %> + class: "answers-link" %> <% end %>
+
- + @@ -29,7 +29,7 @@ diff --git a/app/views/admin/poll/questions/_successful_proposals.html.erb b/app/views/admin/poll/questions/_successful_proposals.html.erb index b0e91ebfa..1a4b06aad 100644 --- a/app/views/admin/poll/questions/_successful_proposals.html.erb +++ b/app/views/admin/poll/questions/_successful_proposals.html.erb @@ -1,4 +1,4 @@ -
<%= t("admin.questions.index.table_question") %> <%= t("admin.questions.index.table_poll") %><%= t("admin.actions.actions") %><%= t("admin.actions.actions") %>
<%= render Admin::TableActionsComponent.new(question) do |actions| %> <%= actions.link_to t("admin.polls.show.edit_answers"), admin_question_path(question), - class: "button hollow" %> + class: "answers-link" %> <% end %>
+
@@ -16,10 +16,10 @@ diff --git a/app/views/admin/poll/questions/answers/documents.html.erb b/app/views/admin/poll/questions/answers/documents.html.erb index 0a1d66573..786c38cd9 100644 --- a/app/views/admin/poll/questions/answers/documents.html.erb +++ b/app/views/admin/poll/questions/answers/documents.html.erb @@ -44,7 +44,7 @@ document.attachment.url, target: "_blank", rel: "nofollow", - class: "button hollow" %> + class: "download-link" %> <% end %> diff --git a/app/views/admin/poll/shifts/_shifts.html.erb b/app/views/admin/poll/shifts/_shifts.html.erb index af3c0e662..d3cda31fb 100644 --- a/app/views/admin/poll/shifts/_shifts.html.erb +++ b/app/views/admin/poll/shifts/_shifts.html.erb @@ -1,5 +1,5 @@

<%= t("admin.poll_shifts.new.shifts") %>

-
<%= t("admin.questions.index.table_proposal") %> <%= render Admin::TableActionsComponent.new(actions: []) do |actions| %> - <%= actions.link_to t("admin.shared.view"), proposal_path(proposal), class: "button hollow" %> + <%= actions.link_to t("admin.shared.view"), proposal_path(proposal), class: "show-link" %> <%= actions.link_to t("admin.questions.index.create_question"), new_admin_question_path(proposal_id: proposal.id), - class: "button hollow" %> + class: "new-link" %> <% end %>
+
diff --git a/app/views/admin/site_customization/pages/index.html.erb b/app/views/admin/site_customization/pages/index.html.erb index 47f31da97..6d0d8f6e9 100644 --- a/app/views/admin/site_customization/pages/index.html.erb +++ b/app/views/admin/site_customization/pages/index.html.erb @@ -16,7 +16,7 @@ - + @@ -31,13 +31,13 @@ <%= render Admin::TableActionsComponent.new(page) do |actions| %> <%= actions.link_to t("admin.site_customization.pages.page.see_cards"), admin_site_customization_page_cards_path(page), - class: "button hollow expanded" %> + class: "cards-link" %> <% if page.status == "published" %> <%= actions.link_to t("admin.site_customization.pages.index.see_page"), page.url, target: "_blank", - class: "button hollow" %> + class: "show-link" %> <% end %> <% end %> diff --git a/app/views/admin/system_emails/index.html.erb b/app/views/admin/system_emails/index.html.erb index c70793954..bcbca17c2 100644 --- a/app/views/admin/system_emails/index.html.erb +++ b/app/views/admin/system_emails/index.html.erb @@ -22,16 +22,16 @@ <% if system_email_actions.include?("view") %> <%= actions.link_to t("admin.shared.view"), admin_system_email_view_path(system_email_title), - class: "button hollow" %> + class: "show-link" %> <% end %> <% if system_email_actions.include?("preview_pending") %> <%= actions.link_to t("admin.system_emails.preview_pending.action"), admin_system_email_preview_pending_path(system_email_title), - class: "button" %> + class: "preview-pending-link" %> <%= actions.link_to t("admin.system_emails.preview_pending.send_pending"), admin_system_email_send_pending_path(system_email_title), - class: "button success", + class: "send-pending-link", method: :put %> <% end %> diff --git a/app/views/admin/valuators/_valuator_row.html.erb b/app/views/admin/valuators/_valuator_row.html.erb index 6a8f28968..c85e159cd 100644 --- a/app/views/admin/valuators/_valuator_row.html.erb +++ b/app/views/admin/valuators/_valuator_row.html.erb @@ -22,7 +22,7 @@ <%= render Admin::TableActionsComponent.new(valuator) do |actions| %> <%= actions.link_to t("admin.shared.view"), admin_valuator_path(valuator), - class: "button hollow" %> + class: "show-link" %> <% end %> diff --git a/app/views/admin/valuators/_valuators.html.erb b/app/views/admin/valuators/_valuators.html.erb index cd80dc7f2..5f9abbc3e 100644 --- a/app/views/admin/valuators/_valuators.html.erb +++ b/app/views/admin/valuators/_valuators.html.erb @@ -5,7 +5,7 @@ - + <% valuators.each do |valuator| %> From 9794ffbbf8e5b79edb2ce0ac174eb3cd586a29eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 1 Jul 2020 23:14:24 +0200 Subject: [PATCH 4/5] Use icons in admin table actions The planned budget investments redesign includes using icons in some tables, so we might as well use them everywhere. The original design used Foundation to show the tooltips. We're using CSS in order to keep the ERB/HTML code simple. One advantage of using CSS is we can show the tooltip on focus as well, just like accessibility guidelines recommend [1]. On the other hand, Foundation tooltips appear on the sides when the link is at the bottom of the page, making sure they're visible in this case, while CSS tooltips do not. Neither CSS tooltips nor Foundation tooltips are dismissable, which might be an accessibility issue. Note we aren't changing any ERB files in order to replace links with icons; we're only changing CSS and one line of Ruby code. [1] https://www.w3.org/WAI/WCAG21/Understanding/content-on-hover-or-focus --- app/assets/stylesheets/_consul_settings.scss | 1 + .../stylesheets/admin/table_actions.scss | 131 ++++++++++++++++-- app/assets/stylesheets/mixins.scss | 14 ++ app/components/concerns/table_action_link.rb | 2 +- 4 files changed, 133 insertions(+), 15 deletions(-) diff --git a/app/assets/stylesheets/_consul_settings.scss b/app/assets/stylesheets/_consul_settings.scss index 1275efcfc..54cf561d1 100644 --- a/app/assets/stylesheets/_consul_settings.scss +++ b/app/assets/stylesheets/_consul_settings.scss @@ -122,3 +122,4 @@ $pagination-radius: $global-radius; $show-header-for-stacked: true; +$tooltip-background-color: $brand; diff --git a/app/assets/stylesheets/admin/table_actions.scss b/app/assets/stylesheets/admin/table_actions.scss index 0409e43a9..51f1a5ee5 100644 --- a/app/assets/stylesheets/admin/table_actions.scss +++ b/app/assets/stylesheets/admin/table_actions.scss @@ -1,25 +1,128 @@ .admin .table-actions { - align-items: baseline; display: flex; > :not(:first-child) { - margin-left: $line-height / 2; - } - - > p { - align-self: flex-start; + margin-left: rem-calc(10); } a { - @include button($style: hollow); - border-color: $link !important; - color: $link !important; - font-size: $base-font-size; - margin-bottom: 0; + position: relative; - &.destroy-link { - border-color: $alert-color !important; - color: $alert-color !important; + > :first-child { + @include bottom-tooltip; + left: -10000px; + opacity: 0; + transform: translateX(-50%); + transition: opacity 0.3s, left 0s 0.3s; + } + + &:hover, + &:focus { + color: $link-hover; + + > :first-child { + left: 50%; + opacity: 1; + transition: opacity 0.4s 0.2s; + } + } + + &:not(:focus) > :first-child:hover { + left: -10000px; + } + + &::before { + font-size: rem-calc(18); } } + + .edit-link { + @include has-fa-icon(edit, regular); + } + + .destroy-link { + @include has-fa-icon(trash-alt, regular); + color: $alert-color; + } + + .show-link, + .preview-link { + @include has-fa-icon(eye, regular); + } + + .new-link, + .assign-booth-link { + @include has-fa-icon(plus-circle, solid); + color: $color-success; + } + + .create-role-link, + .create-officer-link { + @include has-fa-icon(user-plus, solid); + color: $color-success; + } + + .destroy-role-link, + .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; + } + + .confirm-hide-link { + @include has-fa-icon(flag, regular); + color: $alert-color; + } + + .verify-link { + @include has-fa-icon(user-check, solid); + color: $color-success; + } + + .preview-pending-link { + @include has-fa-icon(search, solid); + } + + .send-pending-link { + @include has-fa-icon(share-square, regular); + color: $color-success; + } + + .configure-link, + .answers-link { + @include has-fa-icon(tools, solid); + } + + .download-link { + @include has-fa-icon(download, solid); + } + + .shifts-link { + @include has-fa-icon(clock, regular); + } + + .investments-link { + @include has-fa-icon(coins, solid); + color: $warning-color; + } + + .groups-link, + .headings-link { + @include has-fa-icon(chart-pie, solid); + color: $color-success; + } + + .manage-link, + .ballots-link { + @include has-fa-icon(archive, solid); + } + + .cards-link { + @include has-fa-icon(images, regular); + } } diff --git a/app/assets/stylesheets/mixins.scss b/app/assets/stylesheets/mixins.scss index 2cb8568f6..f5b7e8fd2 100644 --- a/app/assets/stylesheets/mixins.scss +++ b/app/assets/stylesheets/mixins.scss @@ -176,3 +176,17 @@ } } } + +@mixin bottom-tooltip { + @include tooltip; + line-height: $global-lineheight; + margin-top: $line-height / 8; + white-space: nowrap; + + &::before { + @include css-triangle($tooltip-pip-width, $tooltip-background-color, up); + bottom: 100%; + left: 50%; + transform: translateX(-50%); + } +} diff --git a/app/components/concerns/table_action_link.rb b/app/components/concerns/table_action_link.rb index bca6a91ad..34a146059 100644 --- a/app/components/concerns/table_action_link.rb +++ b/app/components/concerns/table_action_link.rb @@ -2,6 +2,6 @@ module TableActionLink extend ActiveSupport::Concern def link_to(text, url, **options) - super(text, url, options) + super(tag.span(text), url, options) end end From a1cae895baa15f7454e697697d0d170baaf66985 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 3 Nov 2020 14:14:46 +0100 Subject: [PATCH 5/5] Use the actions component to render phase actions We forgot to include this table when refactoring in commit 738646a56. --- app/views/admin/budgets/_form.html.erb | 7 ++++--- config/routes/admin.rb | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/views/admin/budgets/_form.html.erb b/app/views/admin/budgets/_form.html.erb index b0c05e38d..ec1c4ccf1 100644 --- a/app/views/admin/budgets/_form.html.erb +++ b/app/views/admin/budgets/_form.html.erb @@ -76,9 +76,10 @@ "> <% end %> diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 5121fec3b..20300220c 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -274,6 +274,10 @@ resolve "Budget::Heading" do |heading, options| [heading.budget, :group, :heading, options.merge(group_id: heading.group, id: heading)] end +resolve "Budget::Phase" do |phase, options| + [phase.budget, :phase, options.merge(id: phase)] +end + resolve "Poll::Booth" do |booth, options| [:booth, options.merge(id: booth)] end
<%= t("admin.poll_shifts.new.date") %><%= t("admin.site_customization.pages.page.created_at") %> <%= t("admin.site_customization.pages.page.updated_at") %> <%= t("admin.site_customization.pages.page.status") %><%= t("admin.actions.actions") %><%= t("admin.actions.actions") %>
<%= t("admin.valuators.index.description") %> <%= t("admin.valuators.index.group") %> <%= t("admin.valuators.index.abilities") %><%= t("admin.actions.actions") %><%= t("admin.actions.actions") %>
- <%= link_to t("admin.budgets.edit.edit_phase"), - edit_admin_budget_budget_phase_path(@budget, phase), - class: "button hollow expanded" %> + <%= render Admin::TableActionsComponent.new(phase, + actions: [:edit], + edit_text: t("admin.budgets.edit.edit_phase") + ) %>