mirror of
https://github.com/we-promise/sure
synced 2026-04-25 17:15:07 +02:00
Prevent long category labels from overflowing or crowding adjacent controls (#1498)
* Initial plan * Fix category delete dialog dropdown overflow Agent-Logs-Url: https://github.com/we-promise/sure/sessions/200da7a4-fd59-4ae4-a709-f631ccf21e8c Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Tighten category deletion regression test Agent-Logs-Url: https://github.com/we-promise/sure/sessions/200da7a4-fd59-4ae4-a709-f631ccf21e8c Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Fix deletion button text overflow Agent-Logs-Url: https://github.com/we-promise/sure/sessions/e802e01f-079e-4322-ba03-b222ab5d4b84 Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Preserve category menu spacing on mobile Agent-Logs-Url: https://github.com/we-promise/sure/sessions/74b5dd1e-7935-4356-806a-759bff911930 Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Prevent account activity label overlap on mobile Agent-Logs-Url: https://github.com/we-promise/sure/sessions/e94027d6-e230-44c8-99a1-6e5645bec82b Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Fix wide account activity category overflow Agent-Logs-Url: https://github.com/we-promise/sure/sessions/4ad79894-2935-47a3-8d37-037e2bd14376 Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Linter * Fix flaky system tests in CI Agent-Logs-Url: https://github.com/we-promise/sure/sessions/3507447f-363f-4759-807c-c62a2858d270 Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> * Reset system test viewport between tests Agent-Logs-Url: https://github.com/we-promise/sure/sessions/357a43b1-11c5-49be-972d-0592a37d97b1 Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com> Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
This commit is contained in:
@@ -4,7 +4,7 @@
|
||||
<% end %>
|
||||
|
||||
<% unless icon_only? %>
|
||||
<%= text %>
|
||||
<span class="min-w-0 truncate"><%= text %></span>
|
||||
<% end %>
|
||||
|
||||
<% if icon && icon_position == :right %>
|
||||
|
||||
@@ -1,15 +1,17 @@
|
||||
<%# locals: (category:) %>
|
||||
<% category ||= Category.uncategorized %>
|
||||
|
||||
<div>
|
||||
<span class="flex items-center gap-1 text-sm font-medium rounded-full px-1.5 py-1 border truncate focus-visible:outline-none focus-visible:ring-0"
|
||||
<div class="min-w-0 w-full">
|
||||
<span class="flex w-full items-center gap-1 text-sm font-medium rounded-full px-1.5 py-1 border focus-visible:outline-none focus-visible:ring-0"
|
||||
style="
|
||||
background-color: color-mix(in oklab, <%= category.color %> 10%, transparent);
|
||||
border-color: color-mix(in oklab, <%= category.color %> 10%, transparent);
|
||||
color: <%= category.color %>;">
|
||||
<% if category.lucide_icon.present? %>
|
||||
<%= icon category.lucide_icon, size: "sm", color: "current" %>
|
||||
<span class="shrink-0">
|
||||
<%= icon category.lucide_icon, size: "sm", color: "current" %>
|
||||
</span>
|
||||
<% end %>
|
||||
<%= category.name %>
|
||||
<span class="min-w-0 flex-1 truncate" data-testid="category-name"><%= category.name %></span>
|
||||
</span>
|
||||
</div>
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
<%# locals: (category:) %>
|
||||
|
||||
<div id="<%= dom_id(category) %>" class="flex justify-between items-center px-4 pb-4 <%= "pt-4" unless category.subcategory? %> <%= "pb-4" unless category.subcategories.any? %> bg-container">
|
||||
<div class="flex w-full items-center gap-2.5">
|
||||
<div id="<%= dom_id(category) %>" class="flex items-center gap-3 px-4 pb-4 <%= "pt-4" unless category.subcategory? %> <%= "pb-4" unless category.subcategories.any? %> bg-container">
|
||||
<div class="flex min-w-0 flex-1 items-center gap-2.5" data-testid="category-content">
|
||||
<% if category.subcategory? %>
|
||||
<span style="color: <%= category.color %>">
|
||||
<%= icon "corner-down-right", size: "sm", color: "current", class: "ml-2" %>
|
||||
@@ -11,7 +11,7 @@
|
||||
<%= render partial: "categories/badge", locals: { category: category } %>
|
||||
</div>
|
||||
|
||||
<div class="justify-self-end">
|
||||
<div class="shrink-0" data-testid="category-actions">
|
||||
<%= render DS::Menu.new do |menu| %>
|
||||
<% menu.with_item(variant: "link", text: t(".edit"), icon: "pencil", href: edit_category_path(category), data: { turbo_frame: :modal }) %>
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<span id="category_name_mobile_<%= transaction.id %>" class="text-secondary lg:hidden">
|
||||
<span id="category_name_mobile_<%= transaction.id %>" class="text-secondary lg:hidden min-w-0 truncate">
|
||||
<% if transaction.transfer&.categorizable? || transaction.transfer.nil? %>
|
||||
<%= transaction.category&.name || Category.uncategorized.name %>
|
||||
<% else %>
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
<%# locals: (transaction:) %>
|
||||
|
||||
<%= render DS::Menu.new(variant: "button") do |menu| %>
|
||||
<% menu.with_button do %>
|
||||
<div class="hidden lg:flex">
|
||||
<% menu.with_button(class: "block w-full overflow-hidden") do %>
|
||||
<div class="hidden min-w-0 w-full lg:flex">
|
||||
<%= render partial: "categories/badge", locals: { category: transaction.category } %>
|
||||
</div>
|
||||
<div class="flex lg:hidden">
|
||||
|
||||
@@ -144,10 +144,10 @@
|
||||
data: { turbo_frame: "_top" },
|
||||
class: "hover:underline" %>
|
||||
</span>
|
||||
<div class="flex items-center gap-1 truncate">
|
||||
<div class="flex min-w-0 items-center gap-1">
|
||||
<%= render "categories/category_name_mobile", transaction: transaction %>
|
||||
<% if transaction.merchant&.present? %>
|
||||
<span class="lg:hidden truncate">• <%= transaction.merchant.name %></span>
|
||||
<span class="lg:hidden min-w-0 truncate">• <%= transaction.merchant.name %></span>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
@@ -158,7 +158,7 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="hidden md:flex items-center gap-1 col-span-2">
|
||||
<div class="hidden md:flex min-w-0 items-center gap-1 col-span-2">
|
||||
<% if entry.account.investment? && !transaction.transfer? %>
|
||||
<%# For investment accounts, show activity label instead of category %>
|
||||
<%= render "investment_activity/quick_edit_badge", entry: entry, entryable: transaction %>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
<%# locals: (transaction:, variant:) %>
|
||||
|
||||
<div id="<%= dom_id(transaction, "category_menu_#{variant}") %>">
|
||||
<div id="<%= dom_id(transaction, "category_menu_#{variant}") %>" class="min-w-0 overflow-hidden">
|
||||
<% if transaction.transfer&.categorizable? || transaction.transfer.nil? %>
|
||||
<%= render "categories/menu", transaction: transaction %>
|
||||
<% else %>
|
||||
|
||||
@@ -2,6 +2,9 @@ require "test_helper"
|
||||
require "socket"
|
||||
|
||||
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
|
||||
DEFAULT_VIEWPORT_WIDTH = 1400
|
||||
DEFAULT_VIEWPORT_HEIGHT = 1400
|
||||
|
||||
setup do
|
||||
Capybara.default_max_wait_time = 5
|
||||
|
||||
@@ -14,6 +17,8 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
|
||||
Capybara.always_include_port = true
|
||||
Capybara.app_host = "http://#{app_host}:#{server_port}"
|
||||
end
|
||||
|
||||
reset_viewport
|
||||
end
|
||||
|
||||
if ENV["SELENIUM_REMOTE_URL"].present?
|
||||
@@ -34,8 +39,17 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
|
||||
driven_by :selenium, using: ENV["CI"].present? ? :headless_chrome : ENV.fetch("E2E_BROWSER", :chrome).to_sym, screen_size: [ 1400, 1400 ]
|
||||
end
|
||||
|
||||
def teardown
|
||||
reset_viewport
|
||||
super
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def reset_viewport
|
||||
page.current_window.resize_to(DEFAULT_VIEWPORT_WIDTH, DEFAULT_VIEWPORT_HEIGHT) if page&.current_window
|
||||
end
|
||||
|
||||
def sign_in(user)
|
||||
visit new_session_path
|
||||
within %(form[action='#{sessions_path}']) do
|
||||
|
||||
@@ -50,6 +50,31 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_select "a[href*='page=2'][href*='tab=holdings']", count: 0
|
||||
end
|
||||
|
||||
test "account activity constrains long category labels before the amount on wide screens" do
|
||||
category = categories(:food_and_drink)
|
||||
category.update!(name: "Super Long Category Name That Should Stop Before The Amount On Wide Screens Too")
|
||||
|
||||
entry = @account.entries.create!(
|
||||
name: "Wide category verification",
|
||||
date: Date.current,
|
||||
amount: 187.65,
|
||||
currency: @account.currency,
|
||||
entryable: Transaction.new(category: category)
|
||||
)
|
||||
|
||||
get account_url(@account, tab: "activity")
|
||||
|
||||
assert_response :success
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")}"
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")}.min-w-0"
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")}.overflow-hidden"
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")} button.block"
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")} button.w-full"
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")} button.overflow-hidden"
|
||||
assert_select "##{dom_id(entry.entryable, "category_menu_desktop")} [data-testid='category-name']"
|
||||
assert_select "div.hidden.md\\:flex.min-w-0"
|
||||
end
|
||||
|
||||
test "should sync account" do
|
||||
post sync_account_url(@account)
|
||||
assert_redirected_to account_url(@account)
|
||||
|
||||
@@ -4,11 +4,15 @@ class CategoriesControllerTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
sign_in users(:family_admin)
|
||||
@transaction = transactions :one
|
||||
ensure_tailwind_build
|
||||
end
|
||||
|
||||
test "index" do
|
||||
get categories_url
|
||||
assert_response :success
|
||||
assert_select "#category_#{categories(:food_and_drink).id} > [data-testid='category-content']", count: 1
|
||||
assert_select "#category_#{categories(:food_and_drink).id} > [data-testid='category-actions']", count: 1
|
||||
assert_select "#category_#{categories(:food_and_drink).id} [data-testid='category-name']", text: categories(:food_and_drink).name
|
||||
end
|
||||
|
||||
test "new" do
|
||||
|
||||
@@ -4,11 +4,14 @@ class Category::DeletionsControllerTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
sign_in users(:family_admin)
|
||||
@category = categories(:food_and_drink)
|
||||
ensure_tailwind_build
|
||||
end
|
||||
|
||||
test "new" do
|
||||
get new_category_deletion_url(@category)
|
||||
assert_response :success
|
||||
assert_select "turbo-frame#modal"
|
||||
assert_select "turbo-frame#modal button span.min-w-0.truncate", text: /Delete "Food & Drink" and leave uncategorized/
|
||||
end
|
||||
|
||||
test "create with replacement" do
|
||||
|
||||
@@ -9,9 +9,7 @@ class CoinstatsItemsControllerTest < ActionDispatch::IntegrationTest
|
||||
name: "Test CoinStats Connection",
|
||||
api_key: "test_api_key_123"
|
||||
)
|
||||
tailwind_build = Rails.root.join("app/assets/builds/tailwind.css")
|
||||
FileUtils.mkdir_p(tailwind_build.dirname)
|
||||
File.write(tailwind_build, "/* test */") unless tailwind_build.exist?
|
||||
ensure_tailwind_build
|
||||
end
|
||||
|
||||
# Helper to wrap data in Provider::Response
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
require "application_system_test_case"
|
||||
|
||||
class AccountActivityTest < ApplicationSystemTestCase
|
||||
DEFAULT_VIEWPORT_WIDTH = 1400
|
||||
DEFAULT_VIEWPORT_HEIGHT = 1400
|
||||
|
||||
setup do
|
||||
ensure_tailwind_build
|
||||
sign_in users(:family_admin)
|
||||
reset_viewport
|
||||
|
||||
@account = accounts(:depository)
|
||||
@transaction_entry = @account.entries.create!(
|
||||
@@ -10,7 +15,7 @@ class AccountActivityTest < ApplicationSystemTestCase
|
||||
date: Date.current,
|
||||
amount: 42.50,
|
||||
currency: "USD",
|
||||
entryable: Transaction.new
|
||||
entryable: Transaction.new(category: categories(:food_and_drink))
|
||||
)
|
||||
@valuation_entry = @account.entries.create!(
|
||||
name: "Current balance",
|
||||
@@ -40,4 +45,74 @@ class AccountActivityTest < ApplicationSystemTestCase
|
||||
assert_selector "a[title='Duplicate'].hidden", visible: false
|
||||
end
|
||||
end
|
||||
|
||||
test "account activity keeps long category names from overflowing the amount on mobile" do
|
||||
category = categories(:food_and_drink)
|
||||
category.update!(name: "Super Long Category Name That Should Stop Before The Amount On Mobile")
|
||||
|
||||
page.current_window.resize_to(315, 643)
|
||||
|
||||
visit account_url(@account, tab: "activity")
|
||||
|
||||
row = find("##{dom_id(@transaction_entry)}")
|
||||
amount = row.find("p.privacy-sensitive", visible: true)
|
||||
category_name = row.find("#category_name_mobile_#{@transaction_entry.entryable.id}", visible: true)
|
||||
|
||||
assert amount.visible?
|
||||
assert category_name.visible?
|
||||
|
||||
row_rect = row.native.rect
|
||||
amount_rect = amount.native.rect
|
||||
viewport_width = page.evaluate_script("window.innerWidth")
|
||||
page_scroll_width = page.evaluate_script("document.documentElement.scrollWidth")
|
||||
|
||||
assert_operator amount_rect.x + amount_rect.width, :<=, row_rect.x + row_rect.width
|
||||
assert_operator page_scroll_width, :<=, viewport_width
|
||||
end
|
||||
|
||||
test "account activity keeps long category names from overlapping the amount on wide screens" do
|
||||
category = categories(:food_and_drink)
|
||||
category.update!(name: "Super Long Category Name That Should Stop Before The Amount On Wide Screens Too")
|
||||
|
||||
page.current_window.resize_to(1280, 900)
|
||||
|
||||
visit account_url(@account, tab: "activity")
|
||||
|
||||
metrics = page.evaluate_script(<<~JS)
|
||||
(() => {
|
||||
const row = document.getElementById("#{dom_id(@transaction_entry)}");
|
||||
const categoryButton = row.querySelector("##{dom_id(@transaction_entry.entryable, "category_menu_desktop")} button");
|
||||
const categoryName = categoryButton.querySelector("[data-testid='category-name']");
|
||||
const amount = row.querySelector(".privacy-sensitive");
|
||||
const categoryRect = categoryButton.getBoundingClientRect();
|
||||
const amountRect = amount.getBoundingClientRect();
|
||||
|
||||
return {
|
||||
categoryRight: categoryRect.right,
|
||||
amountLeft: amountRect.left,
|
||||
categoryOverflow: categoryName.scrollWidth > categoryName.clientWidth
|
||||
};
|
||||
})()
|
||||
JS
|
||||
|
||||
assert_operator metrics["categoryRight"], :<=, metrics["amountLeft"]
|
||||
assert metrics["categoryOverflow"]
|
||||
end
|
||||
|
||||
private
|
||||
def ensure_tailwind_build
|
||||
return if self.class.instance_variable_defined?(:@tailwind_css_built)
|
||||
|
||||
system({ "RAILS_ENV" => "test" }, "bin/rails", "tailwindcss:build", exception: true)
|
||||
self.class.instance_variable_set(:@tailwind_css_built, true)
|
||||
end
|
||||
|
||||
def teardown
|
||||
reset_viewport
|
||||
super
|
||||
end
|
||||
|
||||
def reset_viewport
|
||||
page.current_window.resize_to(DEFAULT_VIEWPORT_WIDTH, DEFAULT_VIEWPORT_HEIGHT) if page&.current_window
|
||||
end
|
||||
end
|
||||
|
||||
@@ -23,4 +23,23 @@ class CategoriesTest < ApplicationSystemTestCase
|
||||
|
||||
assert_text "Name has already been taken"
|
||||
end
|
||||
|
||||
test "long category names truncate before the actions menu on mobile" do
|
||||
category = categories(:food_and_drink)
|
||||
category.update!(name: "Super Long Category Name That Should Stop Before The Menu Button On Mobile")
|
||||
|
||||
page.current_window.resize_to(315, 643)
|
||||
|
||||
visit categories_url
|
||||
|
||||
row = find("##{ActionView::RecordIdentifier.dom_id(category)}")
|
||||
actions = row.find("[data-testid='category-actions'] button", visible: true)
|
||||
|
||||
assert actions.visible?
|
||||
|
||||
viewport_width = page.evaluate_script("window.innerWidth")
|
||||
page_scroll_width = page.evaluate_script("document.documentElement.scrollWidth")
|
||||
|
||||
assert_operator page_scroll_width, :<=, viewport_width
|
||||
end
|
||||
end
|
||||
|
||||
@@ -15,11 +15,7 @@ class PropertiesEditTest < ApplicationSystemTestCase
|
||||
click_link "[system test] Property Account"
|
||||
find("[data-testid='account-menu']").click
|
||||
click_on "Edit"
|
||||
assert_selector "#account_accountable_attributes_subtype"
|
||||
assert_selector(
|
||||
"#account_accountable_attributes_subtype option[selected]",
|
||||
text: "Single Family Home"
|
||||
)
|
||||
assert_equal "single_family_home", find("#account_accountable_attributes_subtype").value
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -81,6 +81,12 @@ module ActiveSupport
|
||||
post sessions_path, params: { email: user.email, password: user_password_test }
|
||||
end
|
||||
|
||||
def ensure_tailwind_build
|
||||
tailwind_build = Rails.root.join("app/assets/builds/tailwind.css")
|
||||
FileUtils.mkdir_p(tailwind_build.dirname)
|
||||
File.write(tailwind_build, "/* test */") unless tailwind_build.exist?
|
||||
end
|
||||
|
||||
def with_env_overrides(overrides = {}, &block)
|
||||
ClimateControl.modify(**overrides, &block)
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user