diff --git a/app/assets/stylesheets/filters.css b/app/assets/stylesheets/filters.css index 8ce52d66c..2b9b9929a 100644 --- a/app/assets/stylesheets/filters.css +++ b/app/assets/stylesheets/filters.css @@ -108,7 +108,15 @@ } .quick-filter { - &:not(.default-value):has(.checked) { + .checked { + display: none; + } + + [aria-checked="true"] .checked { + display: block; + } + + &:has([aria-checked="true"]):not(.quick-filter--with-default) { .input--select { --input-background: var(--color-selected); } @@ -130,5 +138,13 @@ display: block; } } + + .filters__manage { + display: none; + } + + .filters--has-filters-set .filters__manage { + display: inline-flex; + } } diff --git a/app/controllers/concerns/filter_scoped.rb b/app/controllers/concerns/filter_scoped.rb index b4e9aa526..c5e0e5303 100644 --- a/app/controllers/concerns/filter_scoped.rb +++ b/app/controllers/concerns/filter_scoped.rb @@ -13,13 +13,11 @@ module FilterScoped end private - DEFAULT_PARAMS = { indexed_by: "all", sorted_by: "latest" } - def set_filter if params[:filter_id].present? @filter = Current.user.filters.find(params[:filter_id]) else - @filter = Current.user.filters.from_params params.reverse_merge(**DEFAULT_PARAMS).permit(*Filter::PERMITTED_PARAMS) + @filter = Current.user.filters.from_params params.reverse_merge(**Filter.default_values).permit(*Filter::PERMITTED_PARAMS) end end diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb index cbb44963b..03a62ebed 100644 --- a/app/controllers/filters_controller.rb +++ b/app/controllers/filters_controller.rb @@ -1,25 +1,18 @@ class FiltersController < ApplicationController - before_action :set_filter, only: :destroy + before_action :set_filters def create @filter = Current.user.filters.remember filter_params - redirect_to cards_path(filter_id: @filter.id) end def destroy - filter_params = @filter.as_params + @filter = Current.user.filters.find(params[:id]) @filter.destroy! - - if request.referer == root_url - redirect_to root_path - else - redirect_to cards_path(filter_params) - end end private - def set_filter - @filter = Current.user.filters.find params[:id] + def set_filters + @filters = Current.user.filters end def filter_params diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 8b33a23b1..7c75954ae 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -35,8 +35,8 @@ module FiltersHelper }, &block end - def collapsible_nav_section(title, &block) - tag.details class: "popup__section", data: { action: "toggle->nav-section-expander#toggle", nav_section_expander_target: "section", nav_section_expander_key_value: title.parameterize }, open: true do + def collapsible_nav_section(title, **properties, &block) + tag.details class: "popup__section", data: { action: "toggle->nav-section-expander#toggle", nav_section_expander_target: "section", nav_section_expander_key_value: title.parameterize }, open: true, **properties do concat(tag.summary(class: "popup__section-title") do concat title concat icon_tag "caret-down" diff --git a/app/javascript/controllers/combobox_controller.js b/app/javascript/controllers/combobox_controller.js new file mode 100644 index 000000000..02579e5a1 --- /dev/null +++ b/app/javascript/controllers/combobox_controller.js @@ -0,0 +1,85 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + #hiddenField + + static targets = [ "label", "item", "hiddenFieldTemplate" ] + static values = { + selectPropertyName: { type: String, default: "aria-checked" }, + defaultValue: String, + defaultLabel: String + } + static classes = ["withDefault"] + + connect() { + this.#selectedItem = this.#selectedItem + } + + change(event) { + const item = event.target.closest("[role='checkbox']") + if (item) { + this.#selectedItem = item + } + } + + get #selectedLabel() { + const selectedValue = this.#selectedItemValue() + + if (this.hasDefaultLabelValue && (selectedValue === this.defaultValueValue || !selectedValue)) { + return this.defaultLabelValue + } + + return this.#selectedItem?.dataset?.comboboxLabel || "" + } + + get #selectedItem() { + return this.itemTargets.find(item => item.getAttribute(this.selectPropertyNameValue) === "true") + } + + #selectedItemValue() { + return this.#selectedItem?.dataset?.comboboxValue || "" + } + + set #selectedItem(item) { + if (!item) return + + this.#clearSelection() + item.setAttribute(this.selectPropertyNameValue, "true") + this.labelTarget.textContent = this.#selectedLabel + this.hiddenField.value = item.dataset.comboboxValue + this.hiddenField.disabled = !item.dataset.comboboxValue + this.#updateWithDefaultClass() + } + + #clearSelection() { + this.itemTargets.forEach(target => { + target.setAttribute(this.selectPropertyNameValue, "false") + }) + } + + get hiddenField() { + if (!this.#hiddenField) { + this.#hiddenField = this.#buildHiddenField() + } + return this.#hiddenField + } + + #buildHiddenField() { + const [field] = this.hiddenFieldTemplateTarget.content.cloneNode(true).children + this.element.appendChild(field) + return field + } + + #updateWithDefaultClass() { + if (this.hasWithDefaultClass && this.hasDefaultValueValue) { + const selectedValue = this.#selectedItemValue() + const shouldHaveClass = selectedValue === this.defaultValueValue + + if (shouldHaveClass) { + this.element.classList.add(this.withDefaultClass) + } else { + this.element.classList.remove(this.withDefaultClass) + } + } + } +} diff --git a/app/javascript/controllers/filter_settings_controller.js b/app/javascript/controllers/filter_settings_controller.js new file mode 100644 index 000000000..6646cfba7 --- /dev/null +++ b/app/javascript/controllers/filter_settings_controller.js @@ -0,0 +1,45 @@ +import { Controller } from "@hotwired/stimulus" +import { debounce } from "helpers/timing_helpers"; + +export default class extends Controller { + static classes = ["filtersSet"] + static targets = ["field"] + + initialize() { + this.debouncedChange = debounce(this.change.bind(this), 50) + } + + connect() { + this.change() + } + + change() { + this.#toggleFiltersSetClass() + } + + async fieldTargetConnected(field) { + this.debouncedChange() + } + + #toggleFiltersSetClass(shouldAdd) { + this.element.classList.toggle(this.filtersSetClass, this.#hasFiltersSet) + } + + get #hasFiltersSet() { + return this.fieldTargets.some(field => this.#isFieldSet(field)) + } + + #isFieldSet(field) { + const value = field.value?.trim() + + if (!value) return false + + const defaultValue = this.#defaultValueForField(field) + return defaultValue ? value !== defaultValue : true + } + + #defaultValueForField(field) { + const comboboxContainer = field.closest("[data-combobox-default-value-value]") + return comboboxContainer?.dataset?.comboboxDefaultValueValue + } +} diff --git a/app/javascript/controllers/multi_selection_combobox_controller.js b/app/javascript/controllers/multi_selection_combobox_controller.js new file mode 100644 index 000000000..70a853b2b --- /dev/null +++ b/app/javascript/controllers/multi_selection_combobox_controller.js @@ -0,0 +1,98 @@ +import { Controller } from "@hotwired/stimulus" +import { toSentence } from "helpers/text_helpers" + +export default class extends Controller { + #hiddenField + + static targets = [ "label", "item", "hiddenFieldTemplate" ] + static values = { + selectPropertyName: { type: String, default: "aria-checked" }, + defaultValue: String, + noSelectionLabel: { type: String, default: "No selection" }, + labelPrefix: String + } + + connect() { + this.labelTarget.textContent = this.#selectedLabel + this.#updateHiddenFields() + } + + change(event) { + const item = event.target.closest("[role='checkbox']") + if (item) { + this.#toggleSelection(item) + } + } + + + clear(event) { + this.#deselectAll() + this.#updateHiddenFields() + this.labelTarget.textContent = this.#selectedLabel + } + + get #selectedLabel() { + const selectedValues = this.#selectedValues() + if (selectedValues.length === 0) { + return this.noSelectionLabelValue + } + + const labels = this.#selectedItems().map(item => item.dataset.multiSelectionComboboxLabel) + const sentence = toSentence(labels, { + two_words_connector: " or ", + last_word_connector: ", or " + }) + + return this.hasLabelPrefixValue ? `${this.labelPrefixValue} ${sentence}` : sentence + } + + #toggleSelection(item) { + const isSelected = item.getAttribute(this.selectPropertyNameValue) === "true" + + if (isSelected) { + item.setAttribute(this.selectPropertyNameValue, "false") + } else { + item.setAttribute(this.selectPropertyNameValue, "true") + } + + this.#updateHiddenFields() + this.labelTarget.textContent = this.#selectedLabel + } + + #updateHiddenFields() { + this.#clearHiddenFields() + this.#addHiddenFields() + } + + #deselectAll() { + this.itemTargets.forEach(item => { + item.setAttribute(this.selectPropertyNameValue, "false") + }) + } + + #selectedItems() { + return this.itemTargets.filter(item => + item.getAttribute(this.selectPropertyNameValue) === "true" + ) + } + + #selectedValues() { + return this.#selectedItems().map(item => item.dataset.multiSelectionComboboxValue) + } + + #clearHiddenFields() { + this.element.querySelectorAll('input[type="hidden"]').forEach(field => { + if (field !== this.hiddenField) { + field.remove() + } + }) + } + + #addHiddenFields() { + this.#selectedValues().forEach(value => { + const [field] = this.hiddenFieldTemplateTarget.content.cloneNode(true).children + field.value = value + this.element.appendChild(field) + }) + } +} diff --git a/app/javascript/helpers/text_helpers.js b/app/javascript/helpers/text_helpers.js index 22593fe6f..d886e94b9 100644 --- a/app/javascript/helpers/text_helpers.js +++ b/app/javascript/helpers/text_helpers.js @@ -11,3 +11,27 @@ export function normalizeFilteredText(string) { export function filterMatches(text, potentialMatch) { return normalizeFilteredText(text).includes(normalizeFilteredText(potentialMatch)) } + +export function toSentence(array, options = {}) { + const defaultConnectors = { + words_connector: ", ", + two_words_connector: " and ", + last_word_connector: ", and " + } + + const connectors = { ...defaultConnectors, ...options } + + if (array.length === 0) { + return "" + } + + if (array.length === 1) { + return array[0] + } + + if (array.length === 2) { + return array.join(connectors.two_words_connector) + } + + return array.slice(0, -1).join(connectors.words_connector) + connectors.last_word_connector + array[array.length - 1] +} diff --git a/app/models/card.rb b/app/models/card.rb index fe41c5d9b..02394131c 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -41,10 +41,6 @@ class Card < ApplicationRecord delegate :accessible_to?, to: :collection - def cache_key - [ super, collection.name ].compact.join("/") - end - def card self end diff --git a/app/models/card/cacheable.rb b/app/models/card/cacheable.rb index b6c381ceb..94c6b6d11 100644 --- a/app/models/card/cacheable.rb +++ b/app/models/card/cacheable.rb @@ -1,6 +1,10 @@ module Card::Cacheable extend ActiveSupport::Concern + def cache_key + [ super, collection.name ].compact.join("/") + end + def cache_invalidation_parts @cache_invalidation_parts ||= InvalidationParts.new(self) end diff --git a/app/models/cards/columns.rb b/app/models/cards/columns.rb index 419437116..2ef9b9957 100644 --- a/app/models/cards/columns.rb +++ b/app/models/cards/columns.rb @@ -9,15 +9,15 @@ class Cards::Columns end def considering - @considering ||= build_column(filter.with(engagement_status: "considering")) + @considering ||= build_column_for "considering" end def on_deck - @on_deck ||= build_column(filter.with(engagement_status: "on_deck")) + @on_deck ||= build_column_for "on_deck" end def doing - @doing ||= build_column(filter.with(engagement_status: "doing")) + @doing ||= build_column_for "doing" end def closed @@ -33,6 +33,10 @@ class Cards::Columns end private + def build_column_for(engagement_status) + build_column(filter.with(engagement_status: engagement_status)) + end + def build_column(filter, &block) cards = block ? yield(filter.cards) : filter.cards diff --git a/app/models/filter/resources.rb b/app/models/filter/resources.rb index e4d84ba99..da71ce982 100644 --- a/app/models/filter/resources.rb +++ b/app/models/filter/resources.rb @@ -21,4 +21,16 @@ module Filter::Resources def collections creator.collections.where id: super.ids end + + def collection_titles + if collections.none? + [ Collection.one? ? collections.first.name : "All collections" ] + else + collections.map(&:name) + end + end + + def collections_label + collection_titles.to_sentence + end end diff --git a/app/models/user/filtering.rb b/app/models/user/filtering.rb index d0fddc962..9f321bbed 100644 --- a/app/models/user/filtering.rb +++ b/app/models/user/filtering.rb @@ -3,7 +3,7 @@ class User::Filtering attr_reader :user, :filter, :expanded - delegate :as_params, :any?, :single_collection, to: :filter + delegate :as_params, :single_collection, to: :filter delegate :only_closed?, to: :filter def initialize(user, filter, expanded: false) @@ -15,15 +15,11 @@ class User::Filtering end def selected_collection_titles - if filter.collections.none? - [ collections.one? ? collections.first.name : "All collections" ] - else - filter.collections.map(&:name) - end + filter.collection_titles end def selected_collections_label - selected_collection_titles.to_sentence + filter.collections_label end def tags diff --git a/app/views/filters/_filter_toggle.html.erb b/app/views/filters/_filter_toggle.html.erb new file mode 100644 index 000000000..011bd0fab --- /dev/null +++ b/app/views/filters/_filter_toggle.html.erb @@ -0,0 +1,14 @@ +
+ <% 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 %> + <%= 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 6b2278817..66e856160 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", toggle_class_toggle_class: "filters--expanded" } do %> + data: { controller: "toggle-class filter-settings", toggle_class_toggle_class: "filters--expanded", filter_settings_filters_set_class: "filters--has-filters-set" } do %>
<%= render "filters/settings/terms", user_filtering: user_filtering %> <%= render "filters/settings/controls", user_filtering: user_filtering %> diff --git a/app/views/filters/create.turbo_stream.erb b/app/views/filters/create.turbo_stream.erb new file mode 100644 index 000000000..30013fc92 --- /dev/null +++ b/app/views/filters/create.turbo_stream.erb @@ -0,0 +1,2 @@ +<%= turbo_stream.replace("filter-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/destroy.turbo_stream.erb b/app/views/filters/destroy.turbo_stream.erb new file mode 100644 index 000000000..30013fc92 --- /dev/null +++ b/app/views/filters/destroy.turbo_stream.erb @@ -0,0 +1,2 @@ +<%= turbo_stream.replace("filter-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/menu/_custom.html.erb b/app/views/filters/menu/_custom.html.erb index 2de2cae1d..e2ef1d9b7 100644 --- a/app/views/filters/menu/_custom.html.erb +++ b/app/views/filters/menu/_custom.html.erb @@ -1,6 +1,6 @@ -<%= collapsible_nav_section "Saved filters" do %> +<%= collapsible_nav_section "Saved filters", id: "my-filters" do %> <%= form_with url: cards_path, method: :get, data: { controller: "form" } do |form| %> <%= render "filters/menu/custom/create" %> - <%= render partial: "filters/menu/custom/filter", collection: user_filtering.filters, as: :filter, locals: { user_filtering: user_filtering } %> + <%= render partial: "filters/menu/custom/filter", collection: filters, as: :filter %> <% end %> <% end %> diff --git a/app/views/filters/menu/custom/_filter.html.erb b/app/views/filters/menu/custom/_filter.html.erb index 7f40cb83f..5ed324e03 100644 --- a/app/views/filters/menu/custom/_filter.html.erb +++ b/app/views/filters/menu/custom/_filter.html.erb @@ -2,7 +2,7 @@ <%= icon_tag "bookmark", class: "popup__icon" %> <%= link_to cards_path(filter_id: filter.id), class: "popup__btn btn" do %>
-
<%= filter_selected_collections_title(user_filtering) %>
+
<%= filter.collections_label %>
<%= filter.summary %>
<% end %> diff --git a/app/views/filters/settings/_assignees.html.erb b/app/views/filters/settings/_assignees.html.erb index a27ccc7e0..80035e7aa 100644 --- a/app/views/filters/settings/_assignees.html.erb +++ b/app/views/filters/settings/_assignees.html.erb @@ -1,74 +1,47 @@ <% filter = user_filtering.filter %> <%= tag.div class: "quick-filter position-relative", - data: { controller: "dialog", action: "keydown.esc->dialog#close click@document->dialog#closeOnClickOutside", filter_show: user_filtering.show_assignees? } do %> - + + <%= filter_dialog "Assigned to…" do %> Assigned to… <% if User.active.many? %> - <%= text_field_tag :search, nil, placeholder: "Filter…", class: "input input--transparent txt-small", autofocus: true, + <%= text_field_tag nil, nil, placeholder: "Filter…", class: "input input--transparent txt-small", autofocus: true, type: "search", autocorrect: "off", autocomplete: "off", data: { "1p-ignore": "true", filter_target: "input", action: "input->filter#filter" } %> <% end %> -
<%= render "filters/menu/hotkeys" %> - <%= render "filters/menu/custom", user_filtering: @user_filtering %> + <%= render "filters/menu/custom", filters: @user_filtering.filters %> <%= render "filters/menu/collections", user_filtering: @user_filtering %> <%= render "filters/menu/tags", user_filtering: @user_filtering %> <%= render "filters/menu/users", user_filtering: @user_filtering %> diff --git a/test/controllers/filters_controller_test.rb b/test/controllers/filters_controller_test.rb index ef50cc5a7..c1717b898 100644 --- a/test/controllers/filters_controller_test.rb +++ b/test/controllers/filters_controller_test.rb @@ -12,9 +12,9 @@ class FiltersControllerTest < ActionDispatch::IntegrationTest assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ], assignee_ids: [ users(:jz).id ], - collection_ids: [ collections(:writebook).id ] } + collection_ids: [ collections(:writebook).id ] }, as: :turbo_stream end - assert_redirected_to cards_path(filter_id: Filter.last.id) + assert_response :success filter = Filter.last assert_predicate filter.indexed_by, :closed? @@ -29,8 +29,8 @@ class FiltersControllerTest < ActionDispatch::IntegrationTest expected_params = filter.as_params assert_difference "users(:david).filters.count", -1 do - delete filter_path(filter) + delete filter_path(filter), as: :turbo_stream end - assert_redirected_to cards_path(expected_params) + assert_response :success end end