From c8252ed15e6228de2a6b0a85745ce9c56eec4c35 Mon Sep 17 00:00:00 2001 From: Victor Dusart <43795504+vdusart@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:44:24 +0200 Subject: [PATCH] feat(icons): add search field for icon selection (#2862) * feat(icons): add search field for icon selection * fix(icons): address review suggestions on the icon picker * fix(goals): prevent the icon picker popover from collapsing `updatePopupPosition` sets `bottom: 0px` when the popover would run past the fold, but never clears Tailwind's `top-full`. With both offsets set and `height: auto`, CSS derives the height from the offsets instead of the content, collapsing the popover to 26px. This PR is what makes it reachable: the search row (42px plus an 8px gap) and the grid going max-h-40 -> max-h-52 grow the popover ~98px, moving the overflow trigger point far enough to hit standalone /goals/new at 1280x800. `h-fit` makes the height non-auto, so the over-constraint resolves by ignoring `bottom`, which is why categories, already carrying it, was never affected. --- app/components/DS/icon_picker.html.erb | 32 ++++++++ app/components/DS/icon_picker.rb | 16 ++++ .../controllers/list_filter_controller.js | 27 ++++++- app/views/categories/_form.html.erb | 11 +-- app/views/goals/_color_picker.html.erb | 13 +--- config/locales/views/components/en.yml | 3 + test/components/DS/icon_picker_test.rb | 65 ++++++++++++++++ .../previews/icon_picker_component_preview.rb | 13 ++++ .../default.html.erb | 5 ++ test/system/categories_test.rb | 76 +++++++++++++++++++ 10 files changed, 239 insertions(+), 22 deletions(-) create mode 100644 app/components/DS/icon_picker.html.erb create mode 100644 app/components/DS/icon_picker.rb create mode 100644 test/components/DS/icon_picker_test.rb create mode 100644 test/components/previews/icon_picker_component_preview.rb create mode 100644 test/components/previews/icon_picker_component_preview/default.html.erb diff --git a/app/components/DS/icon_picker.html.erb b/app/components/DS/icon_picker.html.erb new file mode 100644 index 000000000..aee7d79ef --- /dev/null +++ b/app/components/DS/icon_picker.html.erb @@ -0,0 +1,32 @@ +
+ <%# Enter in the search field would otherwise submit the surrounding + category/goal form. %> + <%= render DS::SearchInput.new( + variant: :standalone, + placeholder: placeholder, + data: { + list_filter_target: "input", + action: "input->list-filter#filter keydown.enter->list-filter#filter:prevent" + } + ) %> + +
+ + + <%# The `label > input + div > svg` nesting is load-bearing: `color-icon-picker` + tints `input.nextElementSibling` and clones the svg out of the closest label. %> + <% icons.each do |icon_name| %> + + <% end %> +
+
diff --git a/app/components/DS/icon_picker.rb b/app/components/DS/icon_picker.rb new file mode 100644 index 000000000..70fc09968 --- /dev/null +++ b/app/components/DS/icon_picker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# `DS::IconPicker` is a shared, searchable lucide-icon grid. +# +# `method` is a parameter because the backing columns can differ: +# for now `categories.lucide_icon` and `goals.icon`. +class DS::IconPicker < DesignSystemComponent + attr_reader :form, :method, :icons, :placeholder + + def initialize(form:, method:, icons:, placeholder: nil) + @form = form + @method = method + @icons = icons + @placeholder = placeholder || I18n.t("ds.icon_picker.search_placeholder") + end +end diff --git a/app/javascript/controllers/list_filter_controller.js b/app/javascript/controllers/list_filter_controller.js index b7362232a..880f2209d 100644 --- a/app/javascript/controllers/list_filter_controller.js +++ b/app/javascript/controllers/list_filter_controller.js @@ -8,6 +8,29 @@ export default class extends Controller { this.inputTarget.focus(); this.highlightedIndex = -1; this.updateAriaActiveDescendant(); + + this.hostDetails = this.element.closest("details"); + if (this.hostDetails) { + this._onHostToggle = () => { + if (this.hostDetails.open) { + this.inputTarget.focus(); + } else { + this.reset(); + } + }; + this.hostDetails.addEventListener("toggle", this._onHostToggle); + } + } + + disconnect() { + if (this.hostDetails) { + this.hostDetails.removeEventListener("toggle", this._onHostToggle); + } + } + + reset() { + this.inputTarget.value = ""; + this.filter(); } filter() { @@ -118,7 +141,9 @@ export default class extends Controller { clearHighlights() { this.listTarget.querySelectorAll(".filterable-item").forEach((item) => { item.classList.remove("bg-container-inset-hover"); - item.setAttribute("aria-selected", "false"); + if (item.hasAttribute("aria-selected")) { + item.setAttribute("aria-selected", "false"); + } }); } diff --git a/app/views/categories/_form.html.erb b/app/views/categories/_form.html.erb index b974d191d..c482570bb 100644 --- a/app/views/categories/_form.html.erb +++ b/app/views/categories/_form.html.erb @@ -49,16 +49,7 @@

<%= t(".icon") %>

-
- <% Category.icon_codes.each do |icon| %> - - <% end %> -
+ <%= render DS::IconPicker.new(form: f, method: :lucide_icon, icons: Category.icon_codes) %>
<% end %> diff --git a/app/views/goals/_color_picker.html.erb b/app/views/goals/_color_picker.html.erb index e178db443..522d6121d 100644 --- a/app/views/goals/_color_picker.html.erb +++ b/app/views/goals/_color_picker.html.erb @@ -26,7 +26,7 @@ <%= icon("pen", size: "xs") %> <% end %> -
@@ -58,16 +58,7 @@

<%= t("goals.color_picker.icon_heading") %>

-
- <% icons.each do |icon_name| %> - - <% end %> -
+ <%= render DS::IconPicker.new(form: form, method: :icon, icons: icons) %>
<% end %> diff --git a/config/locales/views/components/en.yml b/config/locales/views/components/en.yml index dabd8fce5..fcf8b23f6 100644 --- a/config/locales/views/components/en.yml +++ b/config/locales/views/components/en.yml @@ -99,6 +99,9 @@ en: trigger_label: More info link: opens_in_new_tab: (opens in new tab) + icon_picker: + no_matching_icons: No matching icons + search_placeholder: Search icons... provider_sync_summary: title: Sync summary last_sync: "Last sync: %{time_ago} ago" diff --git a/test/components/DS/icon_picker_test.rb b/test/components/DS/icon_picker_test.rb new file mode 100644 index 000000000..299637fe2 --- /dev/null +++ b/test/components/DS/icon_picker_test.rb @@ -0,0 +1,65 @@ +require "test_helper" + +class DS::IconPickerTest < ViewComponent::TestCase + test "renders one filterable label per icon, keyed by icon code" do + render_icon_picker(icons: %w[pizza coffee dog]) + + assert_selector "label.filterable-item", count: 3 + assert_selector "label.filterable-item[data-filter-name='pizza']" + assert_selector "label.filterable-item[data-filter-name='coffee']" + assert_selector "label.filterable-item[data-filter-name='dog']" + end + + test "wires the list-filter controller with a search input and empty message" do + render_icon_picker(icons: %w[pizza]) + + assert_selector "[data-controller='list-filter']" + assert_selector "input[type='search'][data-list-filter-target='input']" + assert_selector "[data-list-filter-target='list']" + assert_selector "[data-list-filter-target='emptyMessage']", text: "No matching icons", visible: :all + end + + test "prevents Enter in the search field from submitting the form" do + render_icon_picker(icons: %w[pizza]) + + action = page.find("input[type='search']")["data-action"] + assert_includes action, "input->list-filter#filter" + assert_includes action, "keydown.enter->list-filter#filter:prevent" + end + + test "preserves the color-icon-picker contract on each radio" do + render_icon_picker(icons: %w[pizza]) + + radio = page.find("input[type='radio']", visible: :all) + assert_equal "icon", radio["data-color-icon-picker-target"] + assert_includes radio["data-action"], "change->color-icon-picker#handleIconChange" + assert_includes radio["data-action"], "change->color-icon-picker#handleIconColorChange" + + assert_selector "label.filterable-item > input[type='radio'] + div > svg", visible: :all + end + + test "binds radios to the method it was given" do + render_icon_picker(icons: %w[pizza]) + assert_selector "input[name='category[lucide_icon]']", visible: :all + + render_inline DS::IconPicker.new( + form: form_builder_for(Goal.new, "goal"), + method: :icon, + icons: %w[pizza] + ) + assert_selector "input[name='goal[icon]']", visible: :all + end + + private + def render_icon_picker(icons:) + render_inline DS::IconPicker.new( + form: form_builder_for(Category.new, "category"), + method: :lucide_icon, + icons: icons + ) + end + + def form_builder_for(object, name) + StyledFormBuilder.new(name, object, vc_test_controller.view_context, {}) + end +end diff --git a/test/components/previews/icon_picker_component_preview.rb b/test/components/previews/icon_picker_component_preview.rb new file mode 100644 index 000000000..2c59f1356 --- /dev/null +++ b/test/components/previews/icon_picker_component_preview.rb @@ -0,0 +1,13 @@ +class IconPickerComponentPreview < ViewComponent::Preview + # @display container_classes max-w-[400px] + def default + render_with_template(locals: { icons: Category.icon_codes }) + end + + # A short list, to show the "no matching icons" empty state on any + # search term that isn't one of these. + # @display container_classes max-w-[400px] + def few_icons + render_with_template(template: "icon_picker_component_preview/default", locals: { icons: %w[pizza coffee wrench] }) + end +end diff --git a/test/components/previews/icon_picker_component_preview/default.html.erb b/test/components/previews/icon_picker_component_preview/default.html.erb new file mode 100644 index 000000000..654dff06b --- /dev/null +++ b/test/components/previews/icon_picker_component_preview/default.html.erb @@ -0,0 +1,5 @@ +
+ <%= form_with model: Category.new, url: "#" do |f| %> + <%= render DS::IconPicker.new(form: f, method: :lucide_icon, icons: icons) %> + <% end %> +
diff --git a/test/system/categories_test.rb b/test/system/categories_test.rb index 5a5bf1732..f1cd39d2d 100644 --- a/test/system/categories_test.rb +++ b/test/system/categories_test.rb @@ -24,6 +24,82 @@ class CategoriesTest < ApplicationSystemTestCase assert_text "Name has already been taken" end + test "can search the icon picker and save the icon it finds" do + visit categories_url + click_link I18n.t("categories.new.new_category") + fill_in "Name", with: "Takeout" + + picker = find("summary[aria-label='#{I18n.t("categories.form.trigger_label")}']") + picker.click + + # Exact-match on `data-controller` so this can't pick up the parent-category + # `DS::Select`, whose own search field lives under "select list-filter ...". + within "[data-controller='list-filter']" do + search = find("input[type='search']") + + search.set("zzzz") + assert_text I18n.t("ds.icon_picker.no_matching_icons") + + search.set("pizza") + assert_selector "label[data-filter-name='pizza']" + assert_no_selector "label[data-filter-name='coffee']" + + find("label[data-filter-name='pizza']").click + end + + picker.click + + click_button "Create Category" + + # Wait for the redirect back to the list before reading the record — + # `click_button` returns before the Turbo round-trip finishes. + assert_text "Takeout" + + assert_equal "pizza", @user.family.categories.find_by!(name: "Takeout").lucide_icon + end + + test "reopening the icon picker clears the previous search" do + visit categories_url + click_link I18n.t("categories.new.new_category") + + picker = find("summary[aria-label='#{I18n.t("categories.form.trigger_label")}']") + picker.click + + within "[data-controller='list-filter']" do + find("input[type='search']").set("pizza") + assert_no_selector "label[data-filter-name='coffee']" + end + + picker.click + picker.click + + within "[data-controller='list-filter']" do + assert_selector "label[data-filter-name='coffee']" + assert_equal "", find("input[type='search']").value + end + end + + test "pressing enter in the icon search does not submit the category form" do + visit categories_url + click_link I18n.t("categories.new.new_category") + fill_in "Name", with: "Not Saved Yet" + + find("summary[aria-label='#{I18n.t("categories.form.trigger_label")}']").click + + within "[data-controller='list-filter']" do + search = find("input[type='search']") + search.set("pizza") + search.send_keys(:enter) + end + + # Round-trip to the index before reading the record. Capybara's negative + # matchers return as soon as they hold, so asserting straight after the + # keypress would pass while an accidental POST was still in flight. + visit categories_url + assert_no_text "Not Saved Yet" + assert_nil @user.family.categories.find_by(name: "Not Saved Yet") + 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")