diff --git a/app/javascript/controllers/clickable_row_controller.js b/app/javascript/controllers/clickable_row_controller.js new file mode 100644 index 000000000..b1dda43e3 --- /dev/null +++ b/app/javascript/controllers/clickable_row_controller.js @@ -0,0 +1,38 @@ +import { Controller } from "@hotwired/stimulus"; + +// Delegates a click anywhere on a list row (avatar, whitespace, cells with +// no control of their own) to the row's primary link, so the whole row +// looks and behaves as clickable rather than only the exact link text. +// Clicks on real interactive descendants (checkbox, category menu, account +// link, kebab menu, etc.) are left alone so they keep handling themselves. +export default class extends Controller { + static targets = ["link"]; + + open(event) { + if (event.target.closest("a, button, input, select, textarea, label")) return; + // Popover/menu/dropdown panels (category dropdown, account/kebab menu, + // investment activity quick-edit) render as `position: fixed`/`absolute` + // but stay DOM descendants of the row, so a click anywhere on their + // background (padding, headings, plain text) would otherwise fall + // through to the row's own link. + if ( + event.target.closest( + '[data-ds--popover-target="content"], [data-ds--menu-target="content"], [data-activity-label-quick-edit-target="dropdown"]', + ) + ) { + return; + } + if (window.getSelection().toString().length > 0) return; + + // Browsers only honor Ctrl/Cmd/Shift (open in new tab/window) on + // trusted, native link activation — a synthetic/dispatched click event + // is always untrusted, so its modifier-key flags are ignored. Open the + // link explicitly instead of delegating to `linkTarget.click()`. + if (event.ctrlKey || event.metaKey || event.shiftKey) { + window.open(this.linkTarget.href, "_blank", "noopener"); + return; + } + + this.linkTarget.click(); + } +} diff --git a/app/views/accounts/_account.html.erb b/app/views/accounts/_account.html.erb index 4a1c73ca8..c46ccda65 100644 --- a/app/views/accounts/_account.html.erb +++ b/app/views/accounts/_account.html.erb @@ -3,7 +3,11 @@ <% is_default = Current.user&.default_account_id == account.id %> <%= turbo_frame_tag dom_id(account) do %> -
+
+ data-controller="clickable-row" + data-action="click->clickable-row#open" + <% end %>>
<%= render "accounts/logo", account: account, size: "md" %> @@ -19,7 +23,7 @@

<% else %>
- <%= link_to account.name, account, class: [(account.active? ? "text-primary" : "text-subdued"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %> + <%= link_to account.name, account, class: [(account.active? ? "text-primary" : "text-subdued"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top", clickable_row_target: "link" } %> <% if account.exclude_from_reports? %> <%= render DS::Tooltip.new(text: t("accounts.account.excluded_from_reports_indicator"), as: :span) do %> diff --git a/app/views/investment_activity/_quick_edit_badge.html.erb b/app/views/investment_activity/_quick_edit_badge.html.erb index 05c243894..16b75ff08 100644 --- a/app/views/investment_activity/_quick_edit_badge.html.erb +++ b/app/views/investment_activity/_quick_edit_badge.html.erb @@ -48,22 +48,18 @@ data-activity-label-quick-edit-convert-url-value="<%= convert_url %>" <% end %>> - + <% end %> <% unless income_trade %> diff --git a/app/views/trades/_trade.html.erb b/app/views/trades/_trade.html.erb index 162a7da09..95ac57d86 100644 --- a/app/views/trades/_trade.html.erb +++ b/app/views/trades/_trade.html.erb @@ -5,7 +5,9 @@ <%= turbo_frame_tag dom_id(entry) do %> <%= turbo_frame_tag dom_id(trade) do %> -
text-sm font-medium p-4"> +
text-sm font-medium p-4" + data-controller="clickable-row" + data-action="click->clickable-row#open">
<%= check_box_tag dom_id(entry, "selection"), class: "checkbox checkbox--light hidden lg:block", @@ -41,7 +43,7 @@
<%= link_to entry.name, entry_path(entry), - data: { turbo_frame: "drawer", turbo_prefetch: false }, + data: { turbo_frame: "drawer", turbo_prefetch: false, clickable_row_target: "link" }, class: "hover:underline" %>
<% end %> diff --git a/app/views/transactions/_split_parent_row.html.erb b/app/views/transactions/_split_parent_row.html.erb index f2e68eca6..e6227351f 100644 --- a/app/views/transactions/_split_parent_row.html.erb +++ b/app/views/transactions/_split_parent_row.html.erb @@ -1,7 +1,9 @@ <%# locals: (entry:) %> <% transaction = entry.entryable %> -
+
<%# Empty space where checkbox would be, for alignment %> @@ -31,7 +33,7 @@
<%= link_to entry.name, entry_path(entry), - data: { turbo_frame: "drawer", turbo_prefetch: false }, + data: { turbo_frame: "drawer", turbo_prefetch: false, clickable_row_target: "link" }, class: "hover:underline" %>
diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index 213e11964..98adcf26e 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -2,10 +2,17 @@ <% transaction = entry.entryable %> <% transaction_security_logo_url = transaction.activity_security&.display_logo_url %> +<% open_path = if transaction.transfer? + transaction.transfer.present? ? transfer_path(transaction.transfer) : entry_path(entry) + else + in_split_group ? entry_path(entry, grouped: true) : entry_path(entry) + end %> <%= turbo_frame_tag dom_id(entry) do %> <%= turbo_frame_tag dom_id(transaction) do %> -
"> +
" + data-controller="clickable-row" + data-action="click->clickable-row#open">
<%= check_box_tag dom_id(entry, "selection"), @@ -66,27 +73,16 @@
- <% if transaction.transfer? %> - <%= link_to( - entry.name, - transaction.transfer.present? ? transfer_path(transaction.transfer) : entry_path(entry), - data: { - turbo_frame: "drawer", - turbo_prefetch: false - }, - class: "hover:underline" - ) %> - <% else %> - <%= link_to( - entry.name, - in_split_group ? entry_path(entry, grouped: true) : entry_path(entry), - data: { - turbo_frame: "drawer", - turbo_prefetch: false - }, - class: "hover:underline" - ) %> - <% end %> + <%= link_to( + entry.name, + open_path, + data: { + turbo_frame: "drawer", + turbo_prefetch: false, + clickable_row_target: "link" + }, + class: "hover:underline" + ) %>
diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index 2bb01af1b..44fa65cc5 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -16,6 +16,18 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest assert_select "p.ml-auto.privacy-sensitive" end + test "index delegates whole-row account clicks to the account link" do + get accounts_url + + assert_response :success + doc = Nokogiri::HTML::Document.parse(response.body) + row = doc.at_css("turbo-frame##{dom_id(@account)} [data-controller='clickable-row']") + account_link = row.at_css("a[data-clickable-row-target='link']") + + assert_equal "click->clickable-row#open", row["data-action"] + assert_equal account_path(@account), account_link["href"] + end + test "index localizes the Plaid add accounts action" do ensure_tailwind_build @user.update!(locale: "de") @@ -128,6 +140,21 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest assert_equal 0, per_row_transfer, "N+1 per-row transfer queries detected (#{per_row_transfer})" end + test "show delegates whole-row trade clicks to the drawer link" do + investment_account = accounts(:investment) + entry = entries(:trade) + + get account_url(investment_account) + + assert_response :success + doc = Nokogiri::HTML::Document.parse(response.body) + row = doc.at_css("turbo-frame##{dom_id(entry.entryable)} [data-controller='clickable-row']") + drawer_link = row.at_css("a[data-clickable-row-target='link']") + + assert_equal "click->clickable-row#open", row["data-action"] + assert_equal entry_path(entry), drawer_link["href"] + end + test "show avoids N+1 split-parent queries across paginated entries" do queries = capture_sql_queries { get account_url(@account) } assert_response :success diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index 570ea1c65..4dcfe010c 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -488,6 +488,43 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest assert_select ".split-group > div.opacity-50 p.privacy-sensitive", count: 1 end + # Row only opened on a precise click on the name text (whitespace between + # name/avatar/amount looked clickable via the row's hover styling but did + # nothing). A row-level click delegates to the name link now, so the whole + # row opens the drawer while interactive descendants (checkbox, category + # menu, account link) keep handling their own clicks. + test "transaction row delegates whole-row clicks to the drawer link" do + get transactions_url + + assert_response :success + doc = Nokogiri::HTML::Document.parse(response.body) + frame_id = ActionView::RecordIdentifier.dom_id(@entry.entryable) + row = doc.at_css("turbo-frame##{frame_id} [data-controller='clickable-row']") + drawer_link = row.at_css("a[data-clickable-row-target='link']") + + assert_equal "click->clickable-row#open", row["data-action"] + assert_equal entry_path(@entry), drawer_link["href"] + end + + test "split parent row delegates whole-row clicks to the drawer link" do + entry = create_transaction(account: accounts(:depository), amount: 100, name: "Split parent") + + entry.split!([ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 40, category_id: nil } + ]) + + get transactions_url + + assert_response :success + doc = Nokogiri::HTML::Document.parse(response.body) + row = doc.at_css(".split-group [data-controller='clickable-row']") + drawer_link = row.at_css("a[data-clickable-row-target='link']") + + assert_equal "click->clickable-row#open", row["data-action"] + assert_equal entry_path(entry), drawer_link["href"] + end + test "can paginate" do family = families(:empty) sign_in users(:empty)