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] 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']")