From 717f921a737f96765d0d3811b9bc479555841a47 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 19 Sep 2025 18:37:02 +0200 Subject: [PATCH 1/3] Refresh save filter toggle when changing filter --- .../save_toggle_refreshes_controller.rb | 7 ++++++ .../controllers/filter_settings_controller.js | 25 ++++++++++++++++++- app/views/filters/_settings.html.erb | 2 +- app/views/filters/settings/_controls.html.erb | 2 +- app/views/filters/settings/_terms.html.erb | 2 +- config/routes.rb | 8 +++++- 6 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 app/controllers/filters/save_toggle_refreshes_controller.rb diff --git a/app/controllers/filters/save_toggle_refreshes_controller.rb b/app/controllers/filters/save_toggle_refreshes_controller.rb new file mode 100644 index 000000000..2c09e8e10 --- /dev/null +++ b/app/controllers/filters/save_toggle_refreshes_controller.rb @@ -0,0 +1,7 @@ +class Filters::SaveToggleRefreshesController < ApplicationController + include FilterScoped + + def create + turbo_stream.replace("filter-toggle", partial: "filters/filter_toggle", locals: { filter: @filter }) + end +end diff --git a/app/javascript/controllers/filter_settings_controller.js b/app/javascript/controllers/filter_settings_controller.js index 6646cfba7..73ec2fb4b 100644 --- a/app/javascript/controllers/filter_settings_controller.js +++ b/app/javascript/controllers/filter_settings_controller.js @@ -1,9 +1,11 @@ import { Controller } from "@hotwired/stimulus" import { debounce } from "helpers/timing_helpers"; +import { post } from "@rails/request.js" export default class extends Controller { static classes = ["filtersSet"] - static targets = ["field"] + static targets = ["field", "form"] + static values = { refreshSaveToggleUrl: String } initialize() { this.debouncedChange = debounce(this.change.bind(this), 50) @@ -15,6 +17,7 @@ export default class extends Controller { change() { this.#toggleFiltersSetClass() + this.#refreshSaveToggleButton() } async fieldTargetConnected(field) { @@ -42,4 +45,24 @@ export default class extends Controller { const comboboxContainer = field.closest("[data-combobox-default-value-value]") return comboboxContainer?.dataset?.comboboxDefaultValueValue } + + #refreshSaveToggleButton() { + post(this.refreshSaveToggleUrlValue, { + body: this.#collectFilterFormData(), + responseKind: "turbo-stream" + }) + } + + #collectFilterFormData() { + const formData = new FormData() + + this.formTargets.forEach(form => { + const hiddenFields = form.querySelectorAll('input[type="hidden"]:not([disabled])[name]') + hiddenFields.forEach(field => { + formData.append(field.name, field.value) + }) + }) + + return formData + } } diff --git a/app/views/filters/_settings.html.erb b/app/views/filters/_settings.html.erb index 66e856160..2b2e3e09e 100644 --- a/app/views/filters/_settings.html.erb +++ b/app/views/filters/_settings.html.erb @@ -1,7 +1,7 @@ <% cache user_filtering do %> <%= tag.aside id: "filter-settings", class: class_names("filters flex align-center gap-half justify-center center margin-block-end", { "filters--expanded": user_filtering.expanded? }), - data: { controller: "toggle-class filter-settings", toggle_class_toggle_class: "filters--expanded", filter_settings_filters_set_class: "filters--has-filters-set" } do %> + data: { controller: "toggle-class filter-settings", toggle_class_toggle_class: "filters--expanded", filter_settings_filters_set_class: "filters--has-filters-set", filter_settings_refresh_save_toggle_url_value: save_toggle_refresh_path } do %>
<%= render "filters/settings/terms", user_filtering: user_filtering %> <%= render "filters/settings/controls", user_filtering: user_filtering %> diff --git a/app/views/filters/settings/_controls.html.erb b/app/views/filters/settings/_controls.html.erb index d9d8c25c4..7470f4fa4 100644 --- a/app/views/filters/settings/_controls.html.erb +++ b/app/views/filters/settings/_controls.html.erb @@ -1,5 +1,5 @@ <%= form_with url: cards_path, method: :get, class: "flex-inline center align-center gap-half", - data: { controller: "form", turbo_frame: "card_columns", turbo_action: "advance" } do |form| %> + data: { controller: "form", turbo_frame: "card_columns", turbo_action: "advance", filter_settings_target: "form" } do |form| %> <%= hidden_field_tag :expand_all, true, disabled: !user_filtering.expanded?, data: { toggle_enable_target: "element" } %> <% user_filtering.filter.collections.each do |collection| %> <%= filter_hidden_field_tag "collection_ids[]", collection.id %> diff --git a/app/views/filters/settings/_terms.html.erb b/app/views/filters/settings/_terms.html.erb index 914bcd43c..12b522d20 100644 --- a/app/views/filters/settings/_terms.html.erb +++ b/app/views/filters/settings/_terms.html.erb @@ -1,6 +1,6 @@ <% filter = user_filtering.filter %> -<%= form_with url: cards_path, method: :get, class: "", data: { controller: "form", turbo_frame: "card_columns", action: "input->form#debouncedSubmit" } do |form| %> +<%= form_with url: cards_path, method: :get, class: "", data: { controller: "form", turbo_frame: "card_columns", action: "input->form#debouncedSubmit", filter_settings_target: "form" } do |form| %> <% filter.as_params.except(:terms).each do |key, value| %> <%= filter_hidden_field_tag key, value %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 608b2aafd..0823de4ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -81,7 +81,13 @@ Rails.application.routes.draw do resources :queries end - resources :filters + resources :filters do + scope module: :filters do + collection do + resource :save_toggle_refresh, only: :create + end + end + end resources :events, only: :index namespace :events do From ce80c843d0a9b0feefb9bccc5d4cca2e0135be99 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 22 Sep 2025 12:55:19 +0200 Subject: [PATCH 2/3] Several fixes for saving filters - Unify terms with the settings form so that terms are saved normally in the same form. - Fix filter normalization since it was sometimes failing to match the saved filters due to using quotes around number strings. --- app/controllers/concerns/filter_scoped.rb | 6 ++++- .../save_toggle_refreshes_controller.rb | 7 ----- .../filters/settings_refreshes_controller.rb | 6 +++++ app/controllers/filters_controller.rb | 2 +- .../controllers/filter_settings_controller.js | 14 +++++----- app/models/filter/params.rb | 8 +++++- app/views/filters/_filter_toggle.html.erb | 7 +++-- app/views/filters/_settings.html.erb | 27 ++++++++++++++----- app/views/filters/create.turbo_stream.erb | 2 +- app/views/filters/settings/_controls.html.erb | 20 +++++--------- app/views/filters/settings/_manage.html.erb | 2 +- app/views/filters/settings/_terms.html.erb | 26 +++++------------- .../create.turbo_stream.erb | 1 + config/routes.rb | 2 +- 14 files changed, 66 insertions(+), 64 deletions(-) delete mode 100644 app/controllers/filters/save_toggle_refreshes_controller.rb create mode 100644 app/controllers/filters/settings_refreshes_controller.rb create mode 100644 app/views/filters/settings_refreshes/create.turbo_stream.erb diff --git a/app/controllers/concerns/filter_scoped.rb b/app/controllers/concerns/filter_scoped.rb index c5e0e5303..2c422be2a 100644 --- a/app/controllers/concerns/filter_scoped.rb +++ b/app/controllers/concerns/filter_scoped.rb @@ -17,10 +17,14 @@ module FilterScoped if params[:filter_id].present? @filter = Current.user.filters.find(params[:filter_id]) else - @filter = Current.user.filters.from_params params.reverse_merge(**Filter.default_values).permit(*Filter::PERMITTED_PARAMS) + @filter = Current.user.filters.from_params filter_params end end + def filter_params + params.reverse_merge(**Filter.default_values).permit(*Filter::PERMITTED_PARAMS) + end + def set_user_filtering @user_filtering = User::Filtering.new(Current.user, @filter, expanded: expanded_param) end diff --git a/app/controllers/filters/save_toggle_refreshes_controller.rb b/app/controllers/filters/save_toggle_refreshes_controller.rb deleted file mode 100644 index 2c09e8e10..000000000 --- a/app/controllers/filters/save_toggle_refreshes_controller.rb +++ /dev/null @@ -1,7 +0,0 @@ -class Filters::SaveToggleRefreshesController < ApplicationController - include FilterScoped - - def create - turbo_stream.replace("filter-toggle", partial: "filters/filter_toggle", locals: { filter: @filter }) - end -end diff --git a/app/controllers/filters/settings_refreshes_controller.rb b/app/controllers/filters/settings_refreshes_controller.rb new file mode 100644 index 000000000..b7dfac0ec --- /dev/null +++ b/app/controllers/filters/settings_refreshes_controller.rb @@ -0,0 +1,6 @@ +class Filters::SettingsRefreshesController < ApplicationController + include FilterScoped + + def create + end +end diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb index 03a62ebed..efb06aaef 100644 --- a/app/controllers/filters_controller.rb +++ b/app/controllers/filters_controller.rb @@ -16,6 +16,6 @@ class FiltersController < ApplicationController end def filter_params - params.permit(*Filter::PERMITTED_PARAMS).compact_blank + Filter.normalize_params(params.permit(*Filter::PERMITTED_PARAMS)) end end diff --git a/app/javascript/controllers/filter_settings_controller.js b/app/javascript/controllers/filter_settings_controller.js index 73ec2fb4b..85784583a 100644 --- a/app/javascript/controllers/filter_settings_controller.js +++ b/app/javascript/controllers/filter_settings_controller.js @@ -5,26 +5,26 @@ import { post } from "@rails/request.js" export default class extends Controller { static classes = ["filtersSet"] static targets = ["field", "form"] - static values = { refreshSaveToggleUrl: String } + static values = { refreshUrl: String } initialize() { - this.debouncedChange = debounce(this.change.bind(this), 50) + this.debouncedToggle = debounce(this.#toggle.bind(this), 50) } connect() { - this.change() + this.#toggle() } change() { - this.#toggleFiltersSetClass() + this.#toggle() this.#refreshSaveToggleButton() } async fieldTargetConnected(field) { - this.debouncedChange() + this.debouncedToggle() } - #toggleFiltersSetClass(shouldAdd) { + #toggle() { this.element.classList.toggle(this.filtersSetClass, this.#hasFiltersSet) } @@ -47,7 +47,7 @@ export default class extends Controller { } #refreshSaveToggleButton() { - post(this.refreshSaveToggleUrlValue, { + post(this.refreshUrlValue, { body: this.#collectFilterFormData(), responseKind: "turbo-stream" }) diff --git a/app/models/filter/params.rb b/app/models/filter/params.rb index 7685c87fc..81ba67c99 100644 --- a/app/models/filter/params.rb +++ b/app/models/filter/params.rb @@ -28,7 +28,13 @@ module Filter::Params end def normalize_params(params) - params.to_h.compact_blank.reject(&method(:default_value?)).sort_by { |name, _| name.to_s } + params + .to_h + .compact_blank + .reject(&method(:default_value?)) + .collect { |name, value| [ name, value.is_a?(Array) ? value.collect(&:to_s) : value.to_s ] } + .sort_by { |name, _| name.to_s } + .to_h end end diff --git a/app/views/filters/_filter_toggle.html.erb b/app/views/filters/_filter_toggle.html.erb index 011bd0fab..76f599384 100644 --- a/app/views/filters/_filter_toggle.html.erb +++ b/app/views/filters/_filter_toggle.html.erb @@ -1,14 +1,13 @@ -
+
<% if filter.persisted? %> <%= button_to filter_path(filter), method: :delete, class: "btn txt-x-small btn--reversed", form_class: "inline" do %> <%= icon_tag "bookmark-outline" %> Remove saved filter <% end %> <% else %> - <% filter_params = filter.as_params.transform_values { |v| v.respond_to?(:to_str) ? v.to_str : v } %> - <%= button_to filters_path, method: :post, params: filter_params, class: "btn txt-x-small", form_class: "inline" do %> + <%= button_to filters_path(filter.as_params), method: :post, class: "btn txt-x-small", form_class: "inline" do %> <%= icon_tag "bookmark-outline" %> Save filter <% end %> <% end %> -
\ No newline at end of file +
diff --git a/app/views/filters/_settings.html.erb b/app/views/filters/_settings.html.erb index 2b2e3e09e..3e8c1aad1 100644 --- a/app/views/filters/_settings.html.erb +++ b/app/views/filters/_settings.html.erb @@ -1,13 +1,26 @@ <% cache user_filtering do %> + <%= tag.aside id: "filter-settings", class: class_names("filters flex align-center gap-half justify-center center margin-block-end", { "filters--expanded": user_filtering.expanded? }), - data: { controller: "toggle-class filter-settings", toggle_class_toggle_class: "filters--expanded", filter_settings_filters_set_class: "filters--has-filters-set", filter_settings_refresh_save_toggle_url_value: save_toggle_refresh_path } do %> -
- <%= render "filters/settings/terms", user_filtering: user_filtering %> - <%= render "filters/settings/controls", user_filtering: user_filtering %> + data: { + controller: "toggle-class filter-settings", + toggle_class_toggle_class: "filters--expanded", + filter_settings_filters_set_class: "filters--has-filters-set", + filter_settings_refresh_url_value: settings_refresh_path } do %> + <%= form_with url: cards_path, method: :get, class: "", data: { controller: "form", turbo_frame: "card_columns", filter_settings_target: "form", turbo_action: "advance" } do |form| %> + <%= hidden_field_tag :expand_all, true, disabled: !user_filtering.expanded?, data: { toggle_enable_target: "element" } %> + <% user_filtering.filter.collections.each do |collection| %> + <%= filter_hidden_field_tag "collection_ids[]", collection.id %> + <% end %> - <%= render "filters/settings/toggle", user_filtering: user_filtering %> - <%= render "filters/settings/manage", user_filtering: user_filtering %> -
+
+ <%= render "filters/settings/terms", filter: user_filtering.filter, form: form %> + <%= render "filters/settings/controls", user_filtering: user_filtering, form: form %> + + <%= render "filters/settings/toggle", user_filtering: user_filtering %> +
+ <% end %> + <%= render "filters/settings/manage", user_filtering: user_filtering %> <% end %> + <% end %> diff --git a/app/views/filters/create.turbo_stream.erb b/app/views/filters/create.turbo_stream.erb index 30013fc92..a17a53c52 100644 --- a/app/views/filters/create.turbo_stream.erb +++ b/app/views/filters/create.turbo_stream.erb @@ -1,2 +1,2 @@ -<%= turbo_stream.replace("filter-toggle", partial: "filters/filter_toggle", locals: { filter: @filter }) %> +<%= turbo_stream.replace("filter-settings-save-toggle", partial: "filters/filter_toggle", locals: { filter: @filter }) %> <%= turbo_stream.replace("my-filters", partial: "filters/menu/custom", locals: { filters: @filters }) %> diff --git a/app/views/filters/settings/_controls.html.erb b/app/views/filters/settings/_controls.html.erb index 7470f4fa4..60f3338e3 100644 --- a/app/views/filters/settings/_controls.html.erb +++ b/app/views/filters/settings/_controls.html.erb @@ -1,15 +1,7 @@ -<%= form_with url: cards_path, method: :get, class: "flex-inline center align-center gap-half", - data: { controller: "form", turbo_frame: "card_columns", turbo_action: "advance", filter_settings_target: "form" } do |form| %> - <%= hidden_field_tag :expand_all, true, disabled: !user_filtering.expanded?, data: { toggle_enable_target: "element" } %> - <% user_filtering.filter.collections.each do |collection| %> - <%= filter_hidden_field_tag "collection_ids[]", collection.id %> - <% end %> - - <%= render "filters/settings/sorted_by", user_filtering: user_filtering %> - <%= render "filters/settings/indexed_by", user_filtering: user_filtering %> - <%= render "filters/settings/tags", user_filtering: user_filtering %> - <%= render "filters/settings/assignees", user_filtering: user_filtering %> - <%= render "filters/settings/creators", user_filtering: user_filtering %> - <%= render "filters/settings/closers", user_filtering: user_filtering %> -<% end %> +<%= render "filters/settings/sorted_by", user_filtering: user_filtering %> +<%= render "filters/settings/indexed_by", user_filtering: user_filtering %> +<%= render "filters/settings/tags", user_filtering: user_filtering %> +<%= render "filters/settings/assignees", user_filtering: user_filtering %> +<%= render "filters/settings/creators", user_filtering: user_filtering %> +<%= render "filters/settings/closers", user_filtering: user_filtering %> diff --git a/app/views/filters/settings/_manage.html.erb b/app/views/filters/settings/_manage.html.erb index 1fc7bfa17..bc361b7fd 100644 --- a/app/views/filters/settings/_manage.html.erb +++ b/app/views/filters/settings/_manage.html.erb @@ -1,6 +1,6 @@ <% filter = user_filtering.filter %> -
+
<%= render "filters/filter_toggle", filter: filter %> <%= link_to cards_path(filter.as_params.except(:assignee_ids, :assignment_status, :card_ids, :creator_ids, :closer_ids, :stage_ids, :tag_ids, :terms, :indexed_by, :sorted_by, :creation, :closure)), class: "btn btn--remove txt-x-small" do %> diff --git a/app/views/filters/settings/_terms.html.erb b/app/views/filters/settings/_terms.html.erb index 12b522d20..2dca44044 100644 --- a/app/views/filters/settings/_terms.html.erb +++ b/app/views/filters/settings/_terms.html.erb @@ -1,24 +1,12 @@ -<% filter = user_filtering.filter %> - -<%= form_with url: cards_path, method: :get, class: "", data: { controller: "form", turbo_frame: "card_columns", action: "input->form#debouncedSubmit", filter_settings_target: "form" } do |form| %> - <% filter.as_params.except(:terms).each do |key, value| %> - <%= filter_hidden_field_tag key, value %> - <% end %> - - <% filter.terms.each do |existing_term| %> - <%= hidden_field_tag "terms[]", existing_term %> - <% end %> - - <% if user_filtering.expanded? %> - <%= hidden_field_tag :expand_all, user_filtering.expanded? %> - <% end %> - - <%= form.text_field "terms[]", placeholder: "Filter these cards…", class: "input txt-x-small filter__terms", - autofocus: false, autocomplete: :off, autocorrect: "off", data: { - "1p-ignore": "true", - action: "input->form#debouncedSubmit keydown.enter->form#submitToTopTarget" } %> +<% filter.terms.each do |existing_term| %> + <%= hidden_field_tag "terms[]", existing_term %> <% end %> +<%= form.text_field "terms[]", placeholder: "Filter these cards…", class: "input txt-x-small filter__terms", + autofocus: false, autocomplete: :off, autocorrect: "off", data: { + "1p-ignore": "true", + action: "input->form#debouncedSubmit keydown.enter->form#submitToTopTarget" } %> + <% if filter.terms.present? %> <% filter.terms.each do |term| %> <%= filter_chip_tag %Q("#{term}"), filter.as_params_without(:terms, term) %> diff --git a/app/views/filters/settings_refreshes/create.turbo_stream.erb b/app/views/filters/settings_refreshes/create.turbo_stream.erb new file mode 100644 index 000000000..c0fc91e7c --- /dev/null +++ b/app/views/filters/settings_refreshes/create.turbo_stream.erb @@ -0,0 +1 @@ +<%= turbo_stream.replace("filter-settings-save-toggle", partial: "filters/filter_toggle", locals: { filter: @filter }) %> diff --git a/config/routes.rb b/config/routes.rb index 0823de4ee..e543ecc8d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -84,7 +84,7 @@ Rails.application.routes.draw do resources :filters do scope module: :filters do collection do - resource :save_toggle_refresh, only: :create + resource :settings_refresh, only: :create end end end From 72525e7d929572aad1c25a1db882994e40a724e6 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 22 Sep 2025 13:05:33 +0200 Subject: [PATCH 3/3] Make toggle visibility sensible to filters too --- app/assets/stylesheets/filters.css | 2 +- app/views/filters/settings/_manage.html.erb | 2 +- app/views/filters/settings/_terms.html.erb | 6 ++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/filters.css b/app/assets/stylesheets/filters.css index 2b9b9929a..e10454be5 100644 --- a/app/assets/stylesheets/filters.css +++ b/app/assets/stylesheets/filters.css @@ -144,7 +144,7 @@ } .filters--has-filters-set .filters__manage { - display: inline-flex; + display: flex; } } diff --git a/app/views/filters/settings/_manage.html.erb b/app/views/filters/settings/_manage.html.erb index bc361b7fd..e46c8dc1f 100644 --- a/app/views/filters/settings/_manage.html.erb +++ b/app/views/filters/settings/_manage.html.erb @@ -1,6 +1,6 @@ <% filter = user_filtering.filter %> -
+
<%= render "filters/filter_toggle", filter: filter %> <%= link_to cards_path(filter.as_params.except(:assignee_ids, :assignment_status, :card_ids, :creator_ids, :closer_ids, :stage_ids, :tag_ids, :terms, :indexed_by, :sorted_by, :creation, :closure)), class: "btn btn--remove txt-x-small" do %> diff --git a/app/views/filters/settings/_terms.html.erb b/app/views/filters/settings/_terms.html.erb index 2dca44044..f70bae4f3 100644 --- a/app/views/filters/settings/_terms.html.erb +++ b/app/views/filters/settings/_terms.html.erb @@ -1,14 +1,12 @@ -<% filter.terms.each do |existing_term| %> - <%= hidden_field_tag "terms[]", existing_term %> -<% end %> - <%= form.text_field "terms[]", placeholder: "Filter these cards…", class: "input txt-x-small filter__terms", autofocus: false, autocomplete: :off, autocorrect: "off", data: { "1p-ignore": "true", + filter_settings_target: "field", action: "input->form#debouncedSubmit keydown.enter->form#submitToTopTarget" } %> <% if filter.terms.present? %> <% filter.terms.each do |term| %> <%= filter_chip_tag %Q("#{term}"), filter.as_params_without(:terms, term) %> + <%= hidden_field_tag "terms[]", term, data: { filter_settings_target: "field" } %> <% end %> <% end %>