From 5c7d87f763ae73e2b286bcefe5d4a21f0a65cc68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 5 Oct 2023 21:51:16 +0200 Subject: [PATCH] Open admin links in the same window In the admin section, when clicking on a link that leads to a page in the public area, sometimes the page was opened in the same window and sometimes it would open in a new window, with no clear criteria regarding when either scenario would take place. This was really confusing, so now we're more consistent and open (almost) every link in the same window. The main reason behind it is simple: if we add `target: _blank`, people who want to open those links in the same window can no longer do so, so we're taking control away from them. However, if we don't add this attribute, people can choose whether to open the link on the same tab or to open it on a new one, since all browsers implement a method to do so. More reasons behind this decision can be found in "Opening Links in New Browser Windows and Tabs" [1]. We're keeping some exceptions, though: * Opening the link to edit an investment on the same tab would result in losing all the investment filters already applied when searching for investments, so until we implement a way to keep these filters, we're also opening the link to edit an investment in a new tab * For now, we're also opening links to download files in a new window; we'll deal with this case in the future [1] https://www.nngroup.com/articles/new-browser-windows-and-tabs/ --- .../admin/site_customization/pages/index_component.html.erb | 3 +-- app/components/admin/widget/cards/row_component.html.erb | 3 +-- .../admin/dashboard/administrator_tasks/_form.html.erb | 2 +- app/views/admin/milestones/_milestones.html.erb | 3 +-- app/views/admin/poll/polls/_questions.html.erb | 3 +-- app/views/admin/users/_users.html.erb | 4 ++-- app/views/admin/valuators/_user_row.html.erb | 2 +- spec/system/admin/users_spec.rb | 6 +++--- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/app/components/admin/site_customization/pages/index_component.html.erb b/app/components/admin/site_customization/pages/index_component.html.erb index c5f6ebac0..fb81ae0a1 100644 --- a/app/components/admin/site_customization/pages/index_component.html.erb +++ b/app/components/admin/site_customization/pages/index_component.html.erb @@ -33,8 +33,7 @@ <% if page.status == "published" %> <%= actions.action(:show, text: t("admin.site_customization.pages.index.see_page"), - path: page.url, - target: "_blank") %> + path: page.url) %> <% end %> <% end %> diff --git a/app/components/admin/widget/cards/row_component.html.erb b/app/components/admin/widget/cards/row_component.html.erb index 1a1121df3..3f965e028 100644 --- a/app/components/admin/widget/cards/row_component.html.erb +++ b/app/components/admin/widget/cards/row_component.html.erb @@ -12,8 +12,7 @@ <% if card.image.present? %> - <%= link_to t("admin.shared.show_image"), card.image.variant(:large), - title: card.image.title, target: "_blank" %> + <%= link_to t("admin.shared.show_image"), card.image.variant(:large), title: card.image.title %> <% end %> diff --git a/app/views/admin/dashboard/administrator_tasks/_form.html.erb b/app/views/admin/dashboard/administrator_tasks/_form.html.erb index ef0156f79..a80504128 100644 --- a/app/views/admin/dashboard/administrator_tasks/_form.html.erb +++ b/app/views/admin/dashboard/administrator_tasks/_form.html.erb @@ -11,7 +11,7 @@ <%= link_to administrator_task.source.proposal.title, - proposal_path(administrator_task.source.proposal), target: "_blank" %> + proposal_path(administrator_task.source.proposal) %> <%= administrator_task.source.action.title %> diff --git a/app/views/admin/milestones/_milestones.html.erb b/app/views/admin/milestones/_milestones.html.erb index 2a02ea6d8..4b39134a6 100644 --- a/app/views/admin/milestones/_milestones.html.erb +++ b/app/views/admin/milestones/_milestones.html.erb @@ -40,8 +40,7 @@ <%= link_to t("admin.milestones.index.show_image"), - milestone.image.variant(:large), - target: :_blank if milestone.image.present? %> + milestone.image.variant(:large) if milestone.image.present? %> <% if milestone.documents.present? %> diff --git a/app/views/admin/poll/polls/_questions.html.erb b/app/views/admin/poll/polls/_questions.html.erb index acae1cf4f..1367177f0 100644 --- a/app/views/admin/poll/polls/_questions.html.erb +++ b/app/views/admin/poll/polls/_questions.html.erb @@ -24,8 +24,7 @@ <% if question.proposal.present? %> <%= link_to t("admin.polls.show.see_proposal"), - proposal_path(question.proposal), - target: "_blank" %> + proposal_path(question.proposal) %> <% end %> diff --git a/app/views/admin/users/_users.html.erb b/app/views/admin/users/_users.html.erb index 7f1376ef3..ccf42d9bb 100644 --- a/app/views/admin/users/_users.html.erb +++ b/app/views/admin/users/_users.html.erb @@ -23,10 +23,10 @@ <% @users.each do |user| %> <% if @current_filter == "erased" %> - <%= link_to user.id, user_path(user), target: "_blank" %> + <%= link_to user.id, user_path(user) %> <%= user.erase_reason %> <% else %> - <%= link_to user.name, user_path(user), target: "_blank" %> + <%= link_to user.name, user_path(user) %> <%= user.email %> <%= user.document_number %> <%= display_user_roles(user) %> diff --git a/app/views/admin/valuators/_user_row.html.erb b/app/views/admin/valuators/_user_row.html.erb index 609291eae..905fc9030 100644 --- a/app/views/admin/valuators/_user_row.html.erb +++ b/app/views/admin/valuators/_user_row.html.erb @@ -1,5 +1,5 @@ - <%= link_to user.name, user_path(user), target: "_blank" %> + <%= link_to user.name, user_path(user) %> <%= user.email %> diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index bfc910630..99cc7de92 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -18,9 +18,9 @@ describe "Admin users" do scenario "The username links to their public profile" do visit admin_users_path - within_window(window_opened_by { click_link user.name }) do - expect(page).to have_current_path(user_path(user)) - end + click_link user.name + + expect(page).to have_current_path(user_path(user)) end scenario "Show active or erased users using filters" do