From 55ed5bb647b002376be5e8f5766b945e8e8ee778 Mon Sep 17 00:00:00 2001 From: Jeremy Weathers Date: Wed, 13 Mar 2024 16:14:50 -0500 Subject: [PATCH 01/32] Add multiple selections mode - render selection "pills" in the "field" for clarity and compactness - make `Option#content` public for initial render - stay open on selection - hide selected options from listbox (and keyboard nav) - tweak keyboard nav. to work correctly in single and multiple modes - use `aria-current` instead of `aria-selected` for multiple mode --- .../controllers/hw_combobox_controller.js | 11 ++- app/assets/javascripts/hw_combobox/helpers.js | 4 + .../hw_combobox/models/combobox.js | 1 + .../hw_combobox/models/combobox/filtering.js | 1 - .../models/combobox/multiple_selection.js | 76 +++++++++++++++++++ .../hw_combobox/models/combobox/navigation.js | 57 ++++++++++++-- .../hw_combobox/models/combobox/options.js | 21 ++++- app/assets/stylesheets/hotwire_combobox.css | 62 ++++++++++++--- app/presenters/hotwire_combobox/component.rb | 35 +++++++-- .../component/customizable.rb | 1 + .../hotwire_combobox/listbox/option.rb | 8 +- .../hotwire_combobox/combobox/_input.html.erb | 6 +- .../app/controllers/comboboxes_controller.rb | 6 ++ .../app/views/comboboxes/multiple.html.erb | 10 +++ .../multiple_custom_events.html.erb | 20 +++++ test/dummy/config/routes.rb | 2 + test/system/hotwire_combobox_test.rb | 59 ++++++++++++++ 17 files changed, 349 insertions(+), 31 deletions(-) create mode 100644 app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js create mode 100644 test/dummy/app/views/comboboxes/multiple.html.erb create mode 100644 test/dummy/app/views/comboboxes/multiple_custom_events.html.erb diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 69a6fc1..56109a8 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -13,6 +13,7 @@ const concerns = [ Combobox.Events, Combobox.Filtering, Combobox.FormField, + Combobox.MultipleSelection, Combobox.Navigation, Combobox.NewOptions, Combobox.Options, @@ -24,7 +25,8 @@ const concerns = [ export default class HwComboboxController extends Concerns(...concerns) { static classes = [ "invalid", - "selected" + "selected", + "navigated" ] static targets = [ @@ -36,6 +38,7 @@ export default class HwComboboxController extends Concerns(...concerns) { "endOfOptionsStream", "handle", "hiddenField", + "innerWrapper", "listbox", "mainWrapper" ] @@ -46,6 +49,8 @@ export default class HwComboboxController extends Concerns(...concerns) { autocomplete: String, expanded: Boolean, filterableAttribute: String, + isMultiple: Boolean, + multipleSelections: Object, nameWhenNew: String, originalName: String, prefilledDisplay: String, @@ -86,4 +91,8 @@ export default class HwComboboxController extends Concerns(...concerns) { this._preselect() } } + + isMultiple() { + return this.hasIsMultipleValue && this.isMultipleValue + } } diff --git a/app/assets/javascripts/hw_combobox/helpers.js b/app/assets/javascripts/hw_combobox/helpers.js index 766ab6f..85ce694 100644 --- a/app/assets/javascripts/hw_combobox/helpers.js +++ b/app/assets/javascripts/hw_combobox/helpers.js @@ -2,6 +2,10 @@ export function Concerns(Base, ...mixins) { return mixins.reduce((accumulator, current) => current(accumulator), Base) } +export function unselected(target) { + return target.getAttribute("aria-selected") !== "true" +} + export function visible(target) { return !(target.hidden || target.closest("[hidden]")) } diff --git a/app/assets/javascripts/hw_combobox/models/combobox.js b/app/assets/javascripts/hw_combobox/models/combobox.js index c99d57b..e1c92a4 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox.js +++ b/app/assets/javascripts/hw_combobox/models/combobox.js @@ -7,6 +7,7 @@ import "hw_combobox/models/combobox/dialog" import "hw_combobox/models/combobox/events" import "hw_combobox/models/combobox/filtering" import "hw_combobox/models/combobox/form_field" +import "hw_combobox/models/combobox/multiple_selection" import "hw_combobox/models/combobox/navigation" import "hw_combobox/models/combobox/new_options" import "hw_combobox/models/combobox/options" diff --git a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js index ec5af50..c84685c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js @@ -1,4 +1,3 @@ - import Combobox from "hw_combobox/models/combobox/base" import { applyFilter, debounce, unselectedPortion } from "hw_combobox/helpers" import { get } from "hw_combobox/vendor/requestjs" diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js new file mode 100644 index 0000000..850823f --- /dev/null +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js @@ -0,0 +1,76 @@ +import Combobox from "hw_combobox/models/combobox/base" + +Combobox.MultipleSelection = Base => class extends Base { + deSelectOption(event) { + const element = event.target + const value = element.getAttribute("data-value") + this._commitMultipleSelection({ value }, { selected: false }) + element.parentElement.remove() + this._markInvalid() + } + + _connectMultipleSelection() { + if (this.hasMultipleSelectionsValue) { + const selectedValues = Object.keys(this.multipleSelectionsValue) + selectedValues.forEach((selectedValue) => { + this._renderSelection(selectedValue, this.multipleSelectionsValue[selectedValue]) + }) + this.hiddenFieldTarget.value = JSON.stringify(selectedValues) + } + } + + _renderSelection(value, display) { + const selectionWrapper = document.createElement("div") + selectionWrapper.id = value + selectionWrapper.classList.add("hw-combobox__multiple_selection") + const text = document.createElement("div") + text.textContent = display + const closer = document.createElement("div") + closer.classList.add("hw-combobox__multiple_selection__remove") + closer.setAttribute("data-action", "click->hw-combobox#deSelectOption") + closer.setAttribute("data-value", value) + selectionWrapper.appendChild(text) + selectionWrapper.appendChild(closer) + this.innerWrapperTarget.insertBefore(selectionWrapper, this.comboboxTarget) + } + + _commitMultipleSelection(option, { selected }) { + const newSelections = { ...this.multipleSelectionsValue } + if (selected) { + const value = option.getAttribute("id") + if (value) { + if (!Object.keys(newSelections).includes(value)) { + const display = option.textContent + newSelections[value] = display + this._renderSelection(value, display) + this._markSelected(option, { selected: true }) + } + this._actingCombobox.value = "" + this.filter({}) + } + } else { + const value = option.value + delete newSelections[value] + const realOption = this._allOptions.find(realOption => realOption.dataset.value === value) + if (realOption) this._markSelected(realOption, { selected: false }) + } + this.multipleSelectionsValue = newSelections + this.hiddenFieldTarget.value = JSON.stringify(Object.keys(this.multipleSelectionsValue)) + this._dispatchSelectionEvent({}) + } + + _selectNewForMultiple(query) { + console.log('TODO: _selectNewForMultiple', { query }) + } + + _preselectMultipleOption() { + if (this._allOptions.length < 1000) { + const selectedValues = Object.keys(this.multipleSelectionsValue) + + if (selectedValues.length > 0) { + const options = this._allOptions.filter(option => selectedValues.includes(option.dataset.value)) + options.forEach(option => this._markSelected(option, { selected: true })) + } + } + } +} diff --git a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js index 4e98f1f..60c747c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js @@ -1,5 +1,5 @@ import Combobox from "hw_combobox/models/combobox/base" -import { cancel } from "hw_combobox/helpers" +import { cancel, wrapAroundAccess } from "hw_combobox/helpers" Combobox.Navigation = Base => class extends Base { navigate(event) { @@ -10,24 +10,31 @@ Combobox.Navigation = Base => class extends Base { _keyHandlers = { ArrowUp: (event) => { - this._selectIndex(this._selectedOptionIndex - 1) + this._navigateToIndex(this._navigatedOptionIndex - 1) cancel(event) }, ArrowDown: (event) => { - this._selectIndex(this._selectedOptionIndex + 1) + this._navigateToIndex(this._navigatedOptionIndex + 1) cancel(event) }, Home: (event) => { - this._selectIndex(0) + this._navigateToIndex(0) cancel(event) }, End: (event) => { - this._selectIndex(this._visibleOptionElements.length - 1) + this._navigateToIndex(this._visibleOptionElements.length - 1) cancel(event) }, Enter: (event) => { this.close() - this._actingCombobox.blur() + if (this.isMultiple()) { + const currentIndex = this._navigatedOptionIndex + this._selectNavigatedOption() + this._navigateToIndex(currentIndex) + this._actingCombobox.focus() + } else { + this._actingCombobox.blur() + } cancel(event) }, Escape: (event) => { @@ -36,4 +43,42 @@ Combobox.Navigation = Base => class extends Base { cancel(event) } } + + _navigateToIndex(index) { + const option = wrapAroundAccess(this._visibleOptionElements, index) + if (this.isMultiple()) { + this._navigateMultiple(option) + } else { + this._select(option, { forceAutocomplete: true }) + this._dispatchSelectionEvent({}) + } + } + + _navigateMultiple(option) { + if (!option) return + + this._removeCurrentNavigation(this._navigatedOptionElement) + option.setAttribute('aria-current', true) + if (this.hasNavigatedClass) { + option.classList.toggle(this.navigatedClass, true) + } + option.scrollIntoView({ block: "nearest" }) + } + + _removeCurrentNavigation(option) { + if (!option) return + + if (this.hasNavigatedClass) { + option.classList.toggle(this.navigatedClass, false) + } + option.removeAttribute('aria-current') + } + + _selectNavigatedOption() { + const option = this._navigatedOptionElement + if (option) { + this._select(option) + this._removeCurrentNavigation(option) + } + } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/options.js b/app/assets/javascripts/hw_combobox/models/combobox/options.js index 10634fc..df5c2cf 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/options.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/options.js @@ -1,5 +1,5 @@ import Combobox from "hw_combobox/models/combobox/base" -import { visible } from "hw_combobox/helpers" +import { unselected, visible } from "hw_combobox/helpers" Combobox.Options = Base => class extends Base { _resetOptionsSilently() { @@ -27,8 +27,13 @@ Combobox.Options = Base => class extends Base { return this._actingListbox.querySelectorAll(`[${this.filterableAttributeValue}]`) } + get _unselectedOptionElements() { + return [ ...this._allOptionElements ].filter(unselected) + } + get _visibleOptionElements() { - return [ ...this._allOptionElements ].filter(visible) + const optionElements = this.isMultiple() ? this._unselectedOptionElements : this._allOptionElements + return [ ...optionElements ].filter(visible) } get _selectedOptionElement() { @@ -45,4 +50,16 @@ Combobox.Options = Base => class extends Base { return valueIsMissing && noBlankOptionSelected } + + get _navigatedOptionElement() { + if (this.isMultiple()) { + return this._actingListbox.querySelector("[role=option][aria-current=true]") + } else { + return this._selectedOptionElement + } + } + + get _navigatedOptionIndex() { + return [ ...this._visibleOptionElements ].indexOf(this._navigatedOptionElement) + } } diff --git a/app/assets/stylesheets/hotwire_combobox.css b/app/assets/stylesheets/hotwire_combobox.css index d75545b..63c59e4 100644 --- a/app/assets/stylesheets/hotwire_combobox.css +++ b/app/assets/stylesheets/hotwire_combobox.css @@ -24,17 +24,17 @@ --hw-handle-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='none' viewBox='0 0 20 20'%3E%3Cpath stroke='%236b7280' stroke-linecap='round' stroke-linejoin='round' stroke-width='1.5' d='m6 8 4 4 4-4'/%3E%3C/svg%3E"); --hw-handle-image--queried: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='none' viewBox='0 0 24 24'%3E%3Cpath stroke='%236b7280' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M6 18 18 6M6 6l12 12'/%3E%3C/svg%3E"); --hw-handle-offset-right: 0.375rem; - --hw-handle-width: 1.5em; - --hw-handle-width--queried: 1em; + --hw-handle-width: 1.5rem; + --hw-handle-width--queried: 1rem; --hw-combobox-width: 10rem; --hw-line-height: 1.5rem; --hw-listbox-height: calc(var(--hw-line-height) * 10); - --hw-listbox-offset-top: calc(var(--hw-line-height) * 1.625); --hw-listbox-z-index: 10; + --hw-padding--slimmer: 0.25rem; --hw-padding--slim: 0.375rem; --hw-padding--thick: 0.75rem; @@ -57,19 +57,19 @@ } .hw-combobox__main__wrapper { + border: var(--hw-border-width--slim) solid var(--hw-border-color); + border-radius: var(--hw-border-radius); position: relative; - width: min-content; + width: var(--hw-combobox-width); } .hw-combobox__input { - border: var(--hw-border-width--slim) solid var(--hw-border-color); - border-radius: var(--hw-border-radius); + border: 0; font-size: inherit; line-height: var(--hw-line-height); padding: var(--hw-padding--slim) var(--hw-handle-width) var(--hw-padding--slim) var(--hw-padding--thick); - position: relative; - width: var(--hw-combobox-width); text-overflow: ellipsis; + width: 100%; } .hw-combobox__input:focus-visible { @@ -121,7 +121,7 @@ overflow: auto; padding: 0; position: absolute; - top: var(--hw-listbox-offset-top); + top: calc(100% + 0.2rem); width: 100%; z-index: var(--hw-listbox-z-index); @@ -144,6 +144,7 @@ } .hw-combobox__option:hover, +.hw-combobox__option--navigated, .hw-combobox__option--selected { background-color: var(--hw-active-bg-color); } @@ -217,3 +218,46 @@ padding: var(--hw-padding--thick); } } + +.hw-combobox--multiple { + .hw-combobox__inner__wrapper { + display: flex; + flex: 1 1; + flex-wrap: wrap; + align-items: center; + overflow: hidden; + padding: var(--hw-padding--slim); + padding-right: calc(var(--hw-handle-width) + var(--hw-padding--slimmer)); + } + + .hw-combobox__multiple_selection { + display: flex; + flex-wrap: nowrap; + align-items: center; + border: var(--hw-border-width--slim) solid var(--hw-border-color); + border-radius: var(--hw-border-radius); + font-size: 0.85rem; + line-height: var(--hw-line-height); + margin-right: var(--hw-padding--slimmer); + padding: var(--hw-padding--slimmer); + padding-left: var(--hw-padding--slim); + + .hw-combobox__multiple_selection__remove { + background-image: var(--hw-handle-image--queried); + background-size: var(--hw-handle-width--queried); + background-repeat: no-repeat; + cursor: pointer; + min-height: var(--hw-handle-width--queried); + min-width: var(--hw-handle-width--queried); + margin-left: var(--hw-padding--slimmer); + } + } + + .hw-combobox__input { + flex: 2 1; + } + + .hw-combobox__option--selected { + display: none; + } +} diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index 5a5ba24..119efd0 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -3,7 +3,7 @@ class HotwireCombobox::Component include Customizable - attr_reader :options, :label + attr_reader :options, :label, :multiple def initialize \ view, name, @@ -17,15 +17,16 @@ def initialize \ input: {}, label: nil, mobile_at: "640px", + multiple: false, name_when_new: nil, open: false, options: [], value: nil, **rest @view, @autocomplete, @id, @name, @value, @form, @async_src, @label, - @name_when_new, @open, @data, @mobile_at, @options, @dialog_label = + @name_when_new, @open, @data, @mobile_at, @multiple, @options, @dialog_label = view, autocomplete, id, name.to_s, value, form, async_src, label, - name_when_new, open, data, mobile_at, options, dialog_label + name_when_new, open, data, mobile_at, multiple, options, dialog_label @combobox_attrs = input.reverse_merge(rest).deep_symbolize_keys @association_name = association_name || infer_association_name @@ -39,7 +40,7 @@ def render_in(view_context, &block) def fieldset_attrs apply_customizations_to :fieldset, base: { - class: "hw-combobox", + class: "hw-combobox#{" hw-combobox--multiple" if multiple}", data: fieldset_data } end @@ -72,6 +73,14 @@ def main_wrapper_attrs end + def inner_wrapper_attrs + apply_customizations_to :inner_wrapper, base: { + class: "hw-combobox__inner__wrapper", + data: { hw_combobox_target: "innerWrapper" } + } + end + + def input_attrs nested_attrs = %i[ data aria ] @@ -192,13 +201,18 @@ def fieldset_data hw_combobox_small_viewport_max_width_value: mobile_at, hw_combobox_async_src_value: async_src, hw_combobox_prefilled_display_value: prefilled_display, + hw_combobox_is_multiple_value: multiple, + hw_combobox_multiple_selections_value: multiple_selections&.to_json, hw_combobox_filterable_attribute_value: "data-filterable-as", hw_combobox_autocompletable_attribute_value: "data-autocompletable-as", hw_combobox_selected_class: "hw-combobox__option--selected", - hw_combobox_invalid_class: "hw-combobox__input--invalid" + hw_combobox_invalid_class: "hw-combobox__input--invalid", + hw_combobox_navigated_class: "hw-combobox__option--navigated" end def prefilled_display + return if multiple + if async_src && associated_object associated_object.to_combobox_display elsif hidden_field_value @@ -206,6 +220,14 @@ def prefilled_display end end + def multiple_selections + return unless multiple + + Array(value).each_with_object({}) do |loop_value, hash| + hash[loop_value] = options.find { |option| option.value == loop_value }&.content + end + end + def associated_object @associated_object ||= if association_exists? form.object.public_send association_name @@ -281,7 +303,8 @@ def input_aria owns: listbox_id, haspopup: "listbox", autocomplete: autocomplete, - activedescendant: "" + activedescendant: "", + multiselectable: multiple end diff --git a/app/presenters/hotwire_combobox/component/customizable.rb b/app/presenters/hotwire_combobox/component/customizable.rb index 14013bf..9721c39 100644 --- a/app/presenters/hotwire_combobox/component/customizable.rb +++ b/app/presenters/hotwire_combobox/component/customizable.rb @@ -4,6 +4,7 @@ module HotwireCombobox::Component::Customizable label hidden_field main_wrapper + inner_wrapper input handle listbox diff --git a/app/presenters/hotwire_combobox/listbox/option.rb b/app/presenters/hotwire_combobox/listbox/option.rb index e954091..84a33d6 100644 --- a/app/presenters/hotwire_combobox/listbox/option.rb +++ b/app/presenters/hotwire_combobox/listbox/option.rb @@ -17,6 +17,10 @@ def autocompletable_as option.try(:autocompletable_as) || option.try(:display) end + def content + option.try(:content) || option.try(:display) + end + private Data = Struct.new :id, :value, :display, :content, :blank, :filterable_as, :autocompletable_as, keyword_init: true @@ -44,10 +48,6 @@ def data } end - def content - option.try(:content) || option.try(:display) - end - def filterable_as option.try(:filterable_as) || option.try(:display) end diff --git a/app/views/hotwire_combobox/combobox/_input.html.erb b/app/views/hotwire_combobox/combobox/_input.html.erb index 366ea11..de60d74 100644 --- a/app/views/hotwire_combobox/combobox/_input.html.erb +++ b/app/views/hotwire_combobox/combobox/_input.html.erb @@ -1,2 +1,4 @@ -<%= tag.input **component.input_attrs %> -<%= tag.span **component.handle_attrs %> +<%= tag.div **component.inner_wrapper_attrs do %> + <%= tag.input **component.input_attrs %> + <%= tag.span **component.handle_attrs %> +<% end %> diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index dad1397..90cc76f 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -70,6 +70,12 @@ def render_in_locals @hashes = State.limit(3).map { |state| { display: state.name, value: state.abbreviation } } end + def multiple + end + + def multiple_custom_events + end + private delegate :combobox_options, :html_combobox_options, to: "ApplicationController.helpers", private: true diff --git a/test/dummy/app/views/comboboxes/multiple.html.erb b/test/dummy/app/views/comboboxes/multiple.html.erb new file mode 100644 index 0000000..e3e6770 --- /dev/null +++ b/test/dummy/app/views/comboboxes/multiple.html.erb @@ -0,0 +1,10 @@ + + +<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: ['FL', 'MO'] do |combobox| %> + <% combobox.customize_main_wrapper class: "wide-multiple-field" %> +<% end %> diff --git a/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb b/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb new file mode 100644 index 0000000..0ebb37b --- /dev/null +++ b/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb @@ -0,0 +1,20 @@ + + +
+ <%= combobox_tag :state, + @state_options, + id: "state-field", + data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed" }, + multiple: true, + value: ['FL', 'MO'] do |combobox| %> + <% combobox.customize_main_wrapper class: "wide-multiple-field" %> + <% end %> + + + +
diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 1cae4d2..96de308 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -21,6 +21,8 @@ get "custom_attrs", to: "comboboxes#custom_attrs" get "conflicting_order", to: "comboboxes#conflicting_order" get "render_in_locals", to: "comboboxes#render_in_locals" + get "multiple", to: "comboboxes#multiple" + get "multiple_custom_events", to: "comboboxes#multiple_custom_events" resources :movies, only: %i[ index update ] get "movies_html", to: "movies#index_html" diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 1775521..b262488 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -791,6 +791,61 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_option_with text: "display: Alabama\nvalue: AL" end + test "allows multiple selections (hides already-selected options, stays open)" do + visit multiple_path + + assert_selector "div[id='FL'] div", text: "Florida" + open_combobox "#state-field" + assert_no_selector "li[role=option]", text: "Florida" + + type_in_combobox "#state-field", "mi" + assert_no_selector "li[role=option]", text: "Alabama" + delete_from_combobox "#state-field", "mi", original: "mi" + assert_selector "li[role=option]", text: "Alabama" + assert_no_selector "li[role=option]", text: "Florida" + + type_in_combobox "#state-field", :down + assert_current_option_with text: "Alabama" + type_in_combobox "#state-field", :enter + assert_open_combobox + assert_selector "div[id='AL'] div", text: "Alabama" + find("#AL .hw-combobox__multiple_selection__remove").click + assert_no_selector "div[id='AL'] div", text: "Alabama" + + click_on_option "Minnesota" + assert_open_combobox + assert_selector "div[id='MN'] div", text: "Minnesota" + end + + test "multiple selection with custom events" do + visit multiple_custom_events_path + + assert_text "Ready to listen for hw-combobox events!" + + open_combobox "#state-field" + type_in_combobox "#state-field", "mi" + + assert_no_text "event: hw-combobox:selection" + assert_no_text "event: hw-combobox:closed" + + click_on_option "Minnesota" + + assert_text "event: hw-combobox:selection" + assert_text 'value: ["FL","MO","MN"]' + assert_no_text "event: hw-combobox:closed" + + find("#MO .hw-combobox__multiple_selection__remove").click + assert_text 'value: ["FL","MN"]' + assert_no_text "event: hw-combobox:closed" + + click_away + + assert_text "event: hw-combobox:selection" + assert_text 'value: ["FL","MN"]' + + assert_text "event: hw-combobox:closed" + end + private def open_combobox(selector) find(selector).click @@ -863,6 +918,10 @@ def assert_selected_option_with(selector: "", **kwargs) assert_selector "li[role=option][aria-selected=true]#{selector}".squish, **kwargs end + def assert_current_option_with(selector: "", **kwargs) + assert_selector "li[role=option][aria-current=true]#{selector}".squish, **kwargs + end + def assert_no_visible_selected_option assert_no_selector "li[role=option][aria-selected=true]" end From f62afbcaaa6cd69df4da34a62a581165dfe690e3 Mon Sep 17 00:00:00 2001 From: Jeremy Weathers Date: Thu, 14 Mar 2024 23:09:33 -0500 Subject: [PATCH 02/32] - refactor to recent codebase changes (seems to work correctly) --- .../hw_combobox/models/combobox/filtering.js | 2 +- .../models/combobox/multiple_selection.js | 63 ++++++++++++------- .../hw_combobox/models/combobox/navigation.js | 11 ++-- .../hw_combobox/models/combobox/selection.js | 23 +++++-- .../multiple_custom_events.html.erb | 31 +++++---- test/system/hotwire_combobox_test.rb | 5 ++ 6 files changed, 90 insertions(+), 45 deletions(-) diff --git a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js index c84685c..cf6db63 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js @@ -6,7 +6,7 @@ Combobox.Filtering = Base => class extends Base { filterAndSelect(event) { this._filter(event) - if (this._isSync) { + if (this._isSync && !this.isMultiple()) { this._selectOnQuery(event) } else { // noop, async selection is handled by stimulus callbacks diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js index 850823f..abdc77d 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js @@ -1,7 +1,7 @@ import Combobox from "hw_combobox/models/combobox/base" Combobox.MultipleSelection = Base => class extends Base { - deSelectOption(event) { + removeSelection(event) { const element = event.target const value = element.getAttribute("data-value") this._commitMultipleSelection({ value }, { selected: false }) @@ -10,13 +10,14 @@ Combobox.MultipleSelection = Base => class extends Base { } _connectMultipleSelection() { - if (this.hasMultipleSelectionsValue) { - const selectedValues = Object.keys(this.multipleSelectionsValue) - selectedValues.forEach((selectedValue) => { - this._renderSelection(selectedValue, this.multipleSelectionsValue[selectedValue]) - }) - this.hiddenFieldTarget.value = JSON.stringify(selectedValues) - } + this._renderSelections() + this._setMultipleValue() + } + + _renderSelections() { + this._multipleSelectionValues().forEach((selectedValue) => { + this._renderSelection(selectedValue, this.multipleSelectionsValue[selectedValue]) + }) } _renderSelection(value, display) { @@ -25,16 +26,33 @@ Combobox.MultipleSelection = Base => class extends Base { selectionWrapper.classList.add("hw-combobox__multiple_selection") const text = document.createElement("div") text.textContent = display - const closer = document.createElement("div") - closer.classList.add("hw-combobox__multiple_selection__remove") - closer.setAttribute("data-action", "click->hw-combobox#deSelectOption") - closer.setAttribute("data-value", value) + const deselector = document.createElement("div") + deselector.classList.add("hw-combobox__multiple_selection__remove") + deselector.setAttribute("data-action", "click->hw-combobox#removeSelection") + deselector.setAttribute("data-value", value) selectionWrapper.appendChild(text) - selectionWrapper.appendChild(closer) + selectionWrapper.appendChild(deselector) this.innerWrapperTarget.insertBefore(selectionWrapper, this.comboboxTarget) } + _setMultipleValue() { + this._setFieldValue(JSON.stringify(this._multipleSelectionValues())) + } + + _multipleSelectionValues() { + if (this.hasMultipleSelectionsValue) { + return Object.keys(this.multipleSelectionsValue) + } + return [] + } + + _addSelection(option) { + this._commitMultipleSelection(option, { selected: true }) + this._markValid() + } + _commitMultipleSelection(option, { selected }) { + const previousValue = this._fieldValue const newSelections = { ...this.multipleSelectionsValue } if (selected) { const value = option.getAttribute("id") @@ -43,33 +61,32 @@ Combobox.MultipleSelection = Base => class extends Base { const display = option.textContent newSelections[value] = display this._renderSelection(value, display) - this._markSelected(option, { selected: true }) + this._markSelected(option) } this._actingCombobox.value = "" - this.filter({}) + this._fullQuery = "" } } else { const value = option.value delete newSelections[value] - const realOption = this._allOptions.find(realOption => realOption.dataset.value === value) - if (realOption) this._markSelected(realOption, { selected: false }) + const realOption = this._findOptionByValue(value) + if (realOption) this._markNotSelected(realOption) } this.multipleSelectionsValue = newSelections - this.hiddenFieldTarget.value = JSON.stringify(Object.keys(this.multipleSelectionsValue)) - this._dispatchSelectionEvent({}) + this._setMultipleValue() + this._dispatchSelectionEvent({ isNewAndAllowed: false, previousValue }) } _selectNewForMultiple(query) { console.log('TODO: _selectNewForMultiple', { query }) } - _preselectMultipleOption() { + _preselectMultiple() { if (this._allOptions.length < 1000) { - const selectedValues = Object.keys(this.multipleSelectionsValue) - + const selectedValues = this._multipleSelectionValues() if (selectedValues.length > 0) { const options = this._allOptions.filter(option => selectedValues.includes(option.dataset.value)) - options.forEach(option => this._markSelected(option, { selected: true })) + options.forEach(option => this._markSelected(option)) } } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js index 60c747c..013f8a0 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js @@ -26,13 +26,13 @@ Combobox.Navigation = Base => class extends Base { cancel(event) }, Enter: (event) => { - this.close() if (this.isMultiple()) { const currentIndex = this._navigatedOptionIndex this._selectNavigatedOption() this._navigateToIndex(currentIndex) this._actingCombobox.focus() } else { + this.close() this._actingCombobox.blur() } cancel(event) @@ -49,8 +49,7 @@ Combobox.Navigation = Base => class extends Base { if (this.isMultiple()) { this._navigateMultiple(option) } else { - this._select(option, { forceAutocomplete: true }) - this._dispatchSelectionEvent({}) + this._selectAndAutocompleteFullQuery(option) } } @@ -77,7 +76,11 @@ Combobox.Navigation = Base => class extends Base { _selectNavigatedOption() { const option = this._navigatedOptionElement if (option) { - this._select(option) + if (this.isMultiple()) { + this._addSelection(option) + } else { + this._select(option) + } this._removeCurrentNavigation(option) } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index 6c3c36e..f73f766 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -3,11 +3,17 @@ import { wrapAroundAccess, isDeleteEvent } from "hw_combobox/helpers" Combobox.Selection = Base => class extends Base { selectOnClick(event) { - this._forceSelectionAndFilter(event.currentTarget, event) - this.close() + if (this.isMultiple()) { + this._addSelection(event.currentTarget) + } else { + this._forceSelectionAndFilter(event.currentTarget, event) + this.close() + } } _connectSelection() { + if (this.isMultiple()) return this._connectMultipleSelection() + if (this.hasPrefilledDisplayValue) { this._fullQuery = this.prefilledDisplayValue } @@ -84,15 +90,20 @@ Combobox.Selection = Base => class extends Base { } _preselect() { - if (this._hasValueButNoSelection && this._allOptions.length < 100) { - const option = this._allOptions.find(option => { - return option.dataset.value === this._fieldValue - }) + if(this.isMultiple()) return this._preselectMultiple() + if (this._hasValueButNoSelection && this._allOptions.length < 100) { + const option = this._findOptionByValue(this._fieldValue) if (option) this._markSelected(option) } } + _findOptionByValue(value) { + return this._allOptions.find(option => { + return option.dataset.value === value + }) + } + _selectAndAutocompleteMissingPortion(option) { this._select(option, this._autocompleteMissingPortion.bind(this)) } diff --git a/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb b/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb index 0ebb37b..1ee0a99 100644 --- a/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb +++ b/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb @@ -5,16 +5,25 @@ } -
- <%= combobox_tag :state, - @state_options, - id: "state-field", - data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed" }, - multiple: true, - value: ['FL', 'MO'] do |combobox| %> - <% combobox.customize_main_wrapper class: "wide-multiple-field" %> - <% end %> +
+
+ + +
- - +
+
+
+
+ +
+ <%= combobox_tag :state, + @state_options, + id: "state-field", + data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed" }, + multiple: true, + value: ['FL', 'MO'] do |combobox| %> + <% combobox.customize_main_wrapper class: "wide-multiple-field" %> + <% end %> +
diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index b262488..6393c33 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -827,16 +827,20 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_no_text "event: hw-combobox:selection" assert_no_text "event: hw-combobox:closed" + assert_no_text "selections:" click_on_option "Minnesota" assert_text "event: hw-combobox:selection" assert_text 'value: ["FL","MO","MN"]' + assert_text "selections: 1." assert_no_text "event: hw-combobox:closed" find("#MO .hw-combobox__multiple_selection__remove").click assert_text 'value: ["FL","MN"]' + assert_text "selections: 2." assert_no_text "event: hw-combobox:closed" + assert_no_text "closings:" click_away @@ -844,6 +848,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_text 'value: ["FL","MN"]' assert_text "event: hw-combobox:closed" + assert_text "closings: 1." end private From 459cf40bcc859d8fe55b189aa3cd354c6bece710 Mon Sep 17 00:00:00 2001 From: Jeremy Weathers Date: Fri, 15 Mar 2024 11:27:23 -0500 Subject: [PATCH 03/32] - hide placeholder while selections exist --- .../models/combobox/multiple_selection.js | 15 +++++++++++++++ .../app/controllers/comboboxes_controller.rb | 3 +++ test/dummy/app/views/comboboxes/multiple.html.erb | 2 +- .../views/comboboxes/multiple_prefilled.html.erb | 10 ++++++++++ test/dummy/config/routes.rb | 1 + test/system/hotwire_combobox_test.rb | 14 +++++++++++++- 6 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/dummy/app/views/comboboxes/multiple_prefilled.html.erb diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js index abdc77d..65bcf4d 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js @@ -37,6 +37,7 @@ Combobox.MultipleSelection = Base => class extends Base { _setMultipleValue() { this._setFieldValue(JSON.stringify(this._multipleSelectionValues())) + this._swapPlaceholder() } _multipleSelectionValues() { @@ -46,6 +47,20 @@ Combobox.MultipleSelection = Base => class extends Base { return [] } + _swapPlaceholder() { + if (this._fieldValue == '[]') { + if (this.comboboxTarget.hasAttribute('data-placeholder-disabled')) { + this.comboboxTarget.setAttribute('placeholder', this.comboboxTarget.getAttribute('data-placeholder-disabled')) + this.comboboxTarget.removeAttribute('data-placeholder-disabled') + } + } else { + if (this.comboboxTarget.hasAttribute('placeholder')) { + this.comboboxTarget.setAttribute('data-placeholder-disabled', this.comboboxTarget.getAttribute('placeholder')) + this.comboboxTarget.removeAttribute('placeholder') + } + } + } + _addSelection(option) { this._commitMultipleSelection(option, { selected: true }) this._markValid() diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index 90cc76f..2ccb180 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -73,6 +73,9 @@ def render_in_locals def multiple end + def multiple_prefilled + end + def multiple_custom_events end diff --git a/test/dummy/app/views/comboboxes/multiple.html.erb b/test/dummy/app/views/comboboxes/multiple.html.erb index e3e6770..fc6b1cf 100644 --- a/test/dummy/app/views/comboboxes/multiple.html.erb +++ b/test/dummy/app/views/comboboxes/multiple.html.erb @@ -5,6 +5,6 @@ } -<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: ['FL', 'MO'] do |combobox| %> +<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: [], placeholder: 'State' do |combobox| %> <% combobox.customize_main_wrapper class: "wide-multiple-field" %> <% end %> diff --git a/test/dummy/app/views/comboboxes/multiple_prefilled.html.erb b/test/dummy/app/views/comboboxes/multiple_prefilled.html.erb new file mode 100644 index 0000000..e3e6770 --- /dev/null +++ b/test/dummy/app/views/comboboxes/multiple_prefilled.html.erb @@ -0,0 +1,10 @@ + + +<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: ['FL', 'MO'] do |combobox| %> + <% combobox.customize_main_wrapper class: "wide-multiple-field" %> +<% end %> diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 96de308..14424c9 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -22,6 +22,7 @@ get "conflicting_order", to: "comboboxes#conflicting_order" get "render_in_locals", to: "comboboxes#render_in_locals" get "multiple", to: "comboboxes#multiple" + get "multiple_prefilled", to: "comboboxes#multiple_prefilled" get "multiple_custom_events", to: "comboboxes#multiple_custom_events" resources :movies, only: %i[ index update ] diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 6393c33..5789940 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -791,9 +791,21 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_option_with text: "display: Alabama\nvalue: AL" end - test "allows multiple selections (hides already-selected options, stays open)" do + test "allows multiple selections (stays open, swaps out placeholder when selected)" do visit multiple_path + assert_selector "input[placeholder='State']" + open_combobox "#state-field" + click_on_option "Alabama" + assert_open_combobox + assert_no_selector "input[placeholder='State']" + find("#AL .hw-combobox__multiple_selection__remove").click + assert_selector "input[placeholder='State']" + end + + test "prefills multiple selections and hides already-selected options" do + visit multiple_prefilled_path + assert_selector "div[id='FL'] div", text: "Florida" open_combobox "#state-field" assert_no_selector "li[role=option]", text: "Florida" From 0acf48e54666074471a9739f3846267b468b93b9 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Fri, 15 Mar 2024 11:23:21 -0600 Subject: [PATCH 04/32] Reset JS --- .../controllers/hw_combobox_controller.js | 11 +--- app/assets/javascripts/hw_combobox/helpers.js | 4 -- .../hw_combobox/models/combobox.js | 1 - .../hw_combobox/models/combobox/filtering.js | 3 +- .../hw_combobox/models/combobox/navigation.js | 62 +++---------------- .../hw_combobox/models/combobox/options.js | 21 +------ .../hw_combobox/models/combobox/selection.js | 23 ++----- 7 files changed, 18 insertions(+), 107 deletions(-) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 56109a8..69a6fc1 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -13,7 +13,6 @@ const concerns = [ Combobox.Events, Combobox.Filtering, Combobox.FormField, - Combobox.MultipleSelection, Combobox.Navigation, Combobox.NewOptions, Combobox.Options, @@ -25,8 +24,7 @@ const concerns = [ export default class HwComboboxController extends Concerns(...concerns) { static classes = [ "invalid", - "selected", - "navigated" + "selected" ] static targets = [ @@ -38,7 +36,6 @@ export default class HwComboboxController extends Concerns(...concerns) { "endOfOptionsStream", "handle", "hiddenField", - "innerWrapper", "listbox", "mainWrapper" ] @@ -49,8 +46,6 @@ export default class HwComboboxController extends Concerns(...concerns) { autocomplete: String, expanded: Boolean, filterableAttribute: String, - isMultiple: Boolean, - multipleSelections: Object, nameWhenNew: String, originalName: String, prefilledDisplay: String, @@ -91,8 +86,4 @@ export default class HwComboboxController extends Concerns(...concerns) { this._preselect() } } - - isMultiple() { - return this.hasIsMultipleValue && this.isMultipleValue - } } diff --git a/app/assets/javascripts/hw_combobox/helpers.js b/app/assets/javascripts/hw_combobox/helpers.js index 85ce694..766ab6f 100644 --- a/app/assets/javascripts/hw_combobox/helpers.js +++ b/app/assets/javascripts/hw_combobox/helpers.js @@ -2,10 +2,6 @@ export function Concerns(Base, ...mixins) { return mixins.reduce((accumulator, current) => current(accumulator), Base) } -export function unselected(target) { - return target.getAttribute("aria-selected") !== "true" -} - export function visible(target) { return !(target.hidden || target.closest("[hidden]")) } diff --git a/app/assets/javascripts/hw_combobox/models/combobox.js b/app/assets/javascripts/hw_combobox/models/combobox.js index e1c92a4..c99d57b 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox.js +++ b/app/assets/javascripts/hw_combobox/models/combobox.js @@ -7,7 +7,6 @@ import "hw_combobox/models/combobox/dialog" import "hw_combobox/models/combobox/events" import "hw_combobox/models/combobox/filtering" import "hw_combobox/models/combobox/form_field" -import "hw_combobox/models/combobox/multiple_selection" import "hw_combobox/models/combobox/navigation" import "hw_combobox/models/combobox/new_options" import "hw_combobox/models/combobox/options" diff --git a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js index cf6db63..ec5af50 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js @@ -1,3 +1,4 @@ + import Combobox from "hw_combobox/models/combobox/base" import { applyFilter, debounce, unselectedPortion } from "hw_combobox/helpers" import { get } from "hw_combobox/vendor/requestjs" @@ -6,7 +7,7 @@ Combobox.Filtering = Base => class extends Base { filterAndSelect(event) { this._filter(event) - if (this._isSync && !this.isMultiple()) { + if (this._isSync) { this._selectOnQuery(event) } else { // noop, async selection is handled by stimulus callbacks diff --git a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js index 013f8a0..4e98f1f 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js @@ -1,5 +1,5 @@ import Combobox from "hw_combobox/models/combobox/base" -import { cancel, wrapAroundAccess } from "hw_combobox/helpers" +import { cancel } from "hw_combobox/helpers" Combobox.Navigation = Base => class extends Base { navigate(event) { @@ -10,31 +10,24 @@ Combobox.Navigation = Base => class extends Base { _keyHandlers = { ArrowUp: (event) => { - this._navigateToIndex(this._navigatedOptionIndex - 1) + this._selectIndex(this._selectedOptionIndex - 1) cancel(event) }, ArrowDown: (event) => { - this._navigateToIndex(this._navigatedOptionIndex + 1) + this._selectIndex(this._selectedOptionIndex + 1) cancel(event) }, Home: (event) => { - this._navigateToIndex(0) + this._selectIndex(0) cancel(event) }, End: (event) => { - this._navigateToIndex(this._visibleOptionElements.length - 1) + this._selectIndex(this._visibleOptionElements.length - 1) cancel(event) }, Enter: (event) => { - if (this.isMultiple()) { - const currentIndex = this._navigatedOptionIndex - this._selectNavigatedOption() - this._navigateToIndex(currentIndex) - this._actingCombobox.focus() - } else { - this.close() - this._actingCombobox.blur() - } + this.close() + this._actingCombobox.blur() cancel(event) }, Escape: (event) => { @@ -43,45 +36,4 @@ Combobox.Navigation = Base => class extends Base { cancel(event) } } - - _navigateToIndex(index) { - const option = wrapAroundAccess(this._visibleOptionElements, index) - if (this.isMultiple()) { - this._navigateMultiple(option) - } else { - this._selectAndAutocompleteFullQuery(option) - } - } - - _navigateMultiple(option) { - if (!option) return - - this._removeCurrentNavigation(this._navigatedOptionElement) - option.setAttribute('aria-current', true) - if (this.hasNavigatedClass) { - option.classList.toggle(this.navigatedClass, true) - } - option.scrollIntoView({ block: "nearest" }) - } - - _removeCurrentNavigation(option) { - if (!option) return - - if (this.hasNavigatedClass) { - option.classList.toggle(this.navigatedClass, false) - } - option.removeAttribute('aria-current') - } - - _selectNavigatedOption() { - const option = this._navigatedOptionElement - if (option) { - if (this.isMultiple()) { - this._addSelection(option) - } else { - this._select(option) - } - this._removeCurrentNavigation(option) - } - } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/options.js b/app/assets/javascripts/hw_combobox/models/combobox/options.js index df5c2cf..10634fc 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/options.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/options.js @@ -1,5 +1,5 @@ import Combobox from "hw_combobox/models/combobox/base" -import { unselected, visible } from "hw_combobox/helpers" +import { visible } from "hw_combobox/helpers" Combobox.Options = Base => class extends Base { _resetOptionsSilently() { @@ -27,13 +27,8 @@ Combobox.Options = Base => class extends Base { return this._actingListbox.querySelectorAll(`[${this.filterableAttributeValue}]`) } - get _unselectedOptionElements() { - return [ ...this._allOptionElements ].filter(unselected) - } - get _visibleOptionElements() { - const optionElements = this.isMultiple() ? this._unselectedOptionElements : this._allOptionElements - return [ ...optionElements ].filter(visible) + return [ ...this._allOptionElements ].filter(visible) } get _selectedOptionElement() { @@ -50,16 +45,4 @@ Combobox.Options = Base => class extends Base { return valueIsMissing && noBlankOptionSelected } - - get _navigatedOptionElement() { - if (this.isMultiple()) { - return this._actingListbox.querySelector("[role=option][aria-current=true]") - } else { - return this._selectedOptionElement - } - } - - get _navigatedOptionIndex() { - return [ ...this._visibleOptionElements ].indexOf(this._navigatedOptionElement) - } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index f73f766..6c3c36e 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -3,17 +3,11 @@ import { wrapAroundAccess, isDeleteEvent } from "hw_combobox/helpers" Combobox.Selection = Base => class extends Base { selectOnClick(event) { - if (this.isMultiple()) { - this._addSelection(event.currentTarget) - } else { - this._forceSelectionAndFilter(event.currentTarget, event) - this.close() - } + this._forceSelectionAndFilter(event.currentTarget, event) + this.close() } _connectSelection() { - if (this.isMultiple()) return this._connectMultipleSelection() - if (this.hasPrefilledDisplayValue) { this._fullQuery = this.prefilledDisplayValue } @@ -90,20 +84,15 @@ Combobox.Selection = Base => class extends Base { } _preselect() { - if(this.isMultiple()) return this._preselectMultiple() - if (this._hasValueButNoSelection && this._allOptions.length < 100) { - const option = this._findOptionByValue(this._fieldValue) + const option = this._allOptions.find(option => { + return option.dataset.value === this._fieldValue + }) + if (option) this._markSelected(option) } } - _findOptionByValue(value) { - return this._allOptions.find(option => { - return option.dataset.value === value - }) - } - _selectAndAutocompleteMissingPortion(option) { this._select(option, this._autocompleteMissingPortion.bind(this)) } From 9e1f5413d14f6f0ac93e63e9a1065872c8abec1c Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Fri, 15 Mar 2024 11:35:57 -0600 Subject: [PATCH 05/32] Baseline component support --- app/presenters/hotwire_combobox/component.rb | 37 ++++++++----------- .../hotwire_combobox/listbox/option.rb | 7 +++- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index 119efd0..8fb0448 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -40,7 +40,7 @@ def render_in(view_context, &block) def fieldset_attrs apply_customizations_to :fieldset, base: { - class: "hw-combobox#{" hw-combobox--multiple" if multiple}", + class: [ "hw-combobox", { "hw-combobox--multiple": multiple } ], data: fieldset_data } end @@ -75,8 +75,7 @@ def main_wrapper_attrs def inner_wrapper_attrs apply_customizations_to :inner_wrapper, base: { - class: "hw-combobox__inner__wrapper", - data: { hw_combobox_target: "innerWrapper" } + class: "hw-combobox__inner__wrapper" } end @@ -112,7 +111,8 @@ def listbox_attrs role: :listbox, class: "hw-combobox__listbox", hidden: "", - data: listbox_data + data: listbox_data, + aria: listbox_aria } end @@ -159,7 +159,8 @@ def dialog_listbox_attrs id: dialog_listbox_id, class: "hw-combobox__dialog__listbox", role: :listbox, - data: dialog_listbox_data + data: dialog_listbox_data, + aria: dialog_listbox_aria } end @@ -201,18 +202,13 @@ def fieldset_data hw_combobox_small_viewport_max_width_value: mobile_at, hw_combobox_async_src_value: async_src, hw_combobox_prefilled_display_value: prefilled_display, - hw_combobox_is_multiple_value: multiple, - hw_combobox_multiple_selections_value: multiple_selections&.to_json, hw_combobox_filterable_attribute_value: "data-filterable-as", hw_combobox_autocompletable_attribute_value: "data-autocompletable-as", hw_combobox_selected_class: "hw-combobox__option--selected", - hw_combobox_invalid_class: "hw-combobox__input--invalid", - hw_combobox_navigated_class: "hw-combobox__option--navigated" + hw_combobox_invalid_class: "hw-combobox__input--invalid" end def prefilled_display - return if multiple - if async_src && associated_object associated_object.to_combobox_display elsif hidden_field_value @@ -220,14 +216,6 @@ def prefilled_display end end - def multiple_selections - return unless multiple - - Array(value).each_with_object({}) do |loop_value, hash| - hash[loop_value] = options.find { |option| option.value == loop_value }&.content - end - end - def associated_object @associated_object ||= if association_exists? form.object.public_send association_name @@ -303,8 +291,7 @@ def input_aria owns: listbox_id, haspopup: "listbox", autocomplete: autocomplete, - activedescendant: "", - multiselectable: multiple + activedescendant: "" end @@ -324,6 +311,10 @@ def listbox_data { hw_combobox_target: "listbox" } end + def listbox_aria + { multiselectable: multiple } + end + def dialog_data { @@ -363,6 +354,10 @@ def dialog_listbox_data { hw_combobox_target: "dialogListbox" } end + def dialog_listbox_aria + { multiselectable: multiple } + end + def dialog_focus_trap_data { hw_combobox_target: "dialogFocusTrap" } end diff --git a/app/presenters/hotwire_combobox/listbox/option.rb b/app/presenters/hotwire_combobox/listbox/option.rb index 84a33d6..2b2cc02 100644 --- a/app/presenters/hotwire_combobox/listbox/option.rb +++ b/app/presenters/hotwire_combobox/listbox/option.rb @@ -31,7 +31,8 @@ def options id: id, role: :option, class: [ "hw-combobox__option", { "hw-combobox__option--blank": blank? } ], - data: data + data: data, + aria: aria } end @@ -48,6 +49,10 @@ def data } end + def aria + { selected: false } + end + def filterable_as option.try(:filterable_as) || option.try(:display) end From 76fe9a6c5b564dd61d30433a80652bbf0e80e196 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Fri, 15 Mar 2024 12:42:23 -0600 Subject: [PATCH 06/32] Update CSS --- app/assets/stylesheets/hotwire_combobox.css | 9 +++++---- test/dummy/app/views/comboboxes/multiple.html.erb | 11 +---------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/hotwire_combobox.css b/app/assets/stylesheets/hotwire_combobox.css index 63c59e4..68a61d8 100644 --- a/app/assets/stylesheets/hotwire_combobox.css +++ b/app/assets/stylesheets/hotwire_combobox.css @@ -28,6 +28,7 @@ --hw-handle-width--queried: 1rem; --hw-combobox-width: 10rem; + --hw-combobox-width--multiple: 30rem; --hw-line-height: 1.5rem; @@ -220,6 +221,10 @@ } .hw-combobox--multiple { + .hw-combobox__main__wrapper { + width: var(--hw-combobox-width--multiple); + } + .hw-combobox__inner__wrapper { display: flex; flex: 1 1; @@ -256,8 +261,4 @@ .hw-combobox__input { flex: 2 1; } - - .hw-combobox__option--selected { - display: none; - } } diff --git a/test/dummy/app/views/comboboxes/multiple.html.erb b/test/dummy/app/views/comboboxes/multiple.html.erb index fc6b1cf..15c0eaa 100644 --- a/test/dummy/app/views/comboboxes/multiple.html.erb +++ b/test/dummy/app/views/comboboxes/multiple.html.erb @@ -1,10 +1 @@ - - -<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: [], placeholder: 'State' do |combobox| %> - <% combobox.customize_main_wrapper class: "wide-multiple-field" %> -<% end %> +<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: [], placeholder: 'State' %> From c8e584b345c2f8762776f53497bd6b7bd4109e17 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Fri, 15 Mar 2024 20:01:37 -0600 Subject: [PATCH 07/32] Spike on server-rendered approach --- .../controllers/hw_combobox_controller.js | 9 ++ .../hw_combobox/models/combobox.js | 1 + .../models/combobox/multiple_selection.js | 108 ------------------ .../models/combobox/multiselect.js | 63 ++++++++++ .../hw_combobox/models/combobox/navigation.js | 10 +- .../hw_combobox/models/combobox/toggle.js | 10 +- app/assets/stylesheets/hotwire_combobox.css | 65 ++++++----- app/presenters/hotwire_combobox/component.rb | 62 +++++----- .../component/customizable.rb | 1 - .../hotwire_combobox/_selection_chip.html.erb | 8 ++ .../hotwire_combobox/combobox/_input.html.erb | 6 +- lib/hotwire_combobox/helper.rb | 18 +++ .../app/controllers/state_chips_controller.rb | 5 + .../app/views/comboboxes/multiple.html.erb | 5 +- .../views/state_chips/new.turbo_stream.erb | 1 + test/dummy/config/routes.rb | 1 + test/system/hotwire_combobox_test.rb | 6 +- 17 files changed, 196 insertions(+), 183 deletions(-) delete mode 100644 app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js create mode 100644 app/assets/javascripts/hw_combobox/models/combobox/multiselect.js create mode 100644 app/views/hotwire_combobox/_selection_chip.html.erb create mode 100644 test/dummy/app/controllers/state_chips_controller.rb create mode 100644 test/dummy/app/views/state_chips/new.turbo_stream.erb diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 69a6fc1..1e5d81a 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -13,6 +13,7 @@ const concerns = [ Combobox.Events, Combobox.Filtering, Combobox.FormField, + Combobox.Multiselect, Combobox.Navigation, Combobox.NewOptions, Combobox.Options, @@ -29,6 +30,7 @@ export default class HwComboboxController extends Concerns(...concerns) { static targets = [ "combobox", + "chipDismisser", "dialog", "dialogCombobox", "dialogFocusTrap", @@ -49,6 +51,7 @@ export default class HwComboboxController extends Concerns(...concerns) { nameWhenNew: String, originalName: String, prefilledDisplay: String, + selectionChipSrc: String, smallViewportMaxWidth: String } @@ -86,4 +89,10 @@ export default class HwComboboxController extends Concerns(...concerns) { this._preselect() } } + + // Use +_printStack+ for debugging purposes + _printStack() { + const err = new Error() + console.log(err.stack || err.stacktrace) + } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox.js b/app/assets/javascripts/hw_combobox/models/combobox.js index c99d57b..a57bb10 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox.js +++ b/app/assets/javascripts/hw_combobox/models/combobox.js @@ -7,6 +7,7 @@ import "hw_combobox/models/combobox/dialog" import "hw_combobox/models/combobox/events" import "hw_combobox/models/combobox/filtering" import "hw_combobox/models/combobox/form_field" +import "hw_combobox/models/combobox/multiselect" import "hw_combobox/models/combobox/navigation" import "hw_combobox/models/combobox/new_options" import "hw_combobox/models/combobox/options" diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js b/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js deleted file mode 100644 index 65bcf4d..0000000 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiple_selection.js +++ /dev/null @@ -1,108 +0,0 @@ -import Combobox from "hw_combobox/models/combobox/base" - -Combobox.MultipleSelection = Base => class extends Base { - removeSelection(event) { - const element = event.target - const value = element.getAttribute("data-value") - this._commitMultipleSelection({ value }, { selected: false }) - element.parentElement.remove() - this._markInvalid() - } - - _connectMultipleSelection() { - this._renderSelections() - this._setMultipleValue() - } - - _renderSelections() { - this._multipleSelectionValues().forEach((selectedValue) => { - this._renderSelection(selectedValue, this.multipleSelectionsValue[selectedValue]) - }) - } - - _renderSelection(value, display) { - const selectionWrapper = document.createElement("div") - selectionWrapper.id = value - selectionWrapper.classList.add("hw-combobox__multiple_selection") - const text = document.createElement("div") - text.textContent = display - const deselector = document.createElement("div") - deselector.classList.add("hw-combobox__multiple_selection__remove") - deselector.setAttribute("data-action", "click->hw-combobox#removeSelection") - deselector.setAttribute("data-value", value) - selectionWrapper.appendChild(text) - selectionWrapper.appendChild(deselector) - this.innerWrapperTarget.insertBefore(selectionWrapper, this.comboboxTarget) - } - - _setMultipleValue() { - this._setFieldValue(JSON.stringify(this._multipleSelectionValues())) - this._swapPlaceholder() - } - - _multipleSelectionValues() { - if (this.hasMultipleSelectionsValue) { - return Object.keys(this.multipleSelectionsValue) - } - return [] - } - - _swapPlaceholder() { - if (this._fieldValue == '[]') { - if (this.comboboxTarget.hasAttribute('data-placeholder-disabled')) { - this.comboboxTarget.setAttribute('placeholder', this.comboboxTarget.getAttribute('data-placeholder-disabled')) - this.comboboxTarget.removeAttribute('data-placeholder-disabled') - } - } else { - if (this.comboboxTarget.hasAttribute('placeholder')) { - this.comboboxTarget.setAttribute('data-placeholder-disabled', this.comboboxTarget.getAttribute('placeholder')) - this.comboboxTarget.removeAttribute('placeholder') - } - } - } - - _addSelection(option) { - this._commitMultipleSelection(option, { selected: true }) - this._markValid() - } - - _commitMultipleSelection(option, { selected }) { - const previousValue = this._fieldValue - const newSelections = { ...this.multipleSelectionsValue } - if (selected) { - const value = option.getAttribute("id") - if (value) { - if (!Object.keys(newSelections).includes(value)) { - const display = option.textContent - newSelections[value] = display - this._renderSelection(value, display) - this._markSelected(option) - } - this._actingCombobox.value = "" - this._fullQuery = "" - } - } else { - const value = option.value - delete newSelections[value] - const realOption = this._findOptionByValue(value) - if (realOption) this._markNotSelected(realOption) - } - this.multipleSelectionsValue = newSelections - this._setMultipleValue() - this._dispatchSelectionEvent({ isNewAndAllowed: false, previousValue }) - } - - _selectNewForMultiple(query) { - console.log('TODO: _selectNewForMultiple', { query }) - } - - _preselectMultiple() { - if (this._allOptions.length < 1000) { - const selectedValues = this._multipleSelectionValues() - if (selectedValues.length > 0) { - const options = this._allOptions.filter(option => selectedValues.includes(option.dataset.value)) - options.forEach(option => this._markSelected(option)) - } - } - } -} diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js new file mode 100644 index 0000000..fe5955a --- /dev/null +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -0,0 +1,63 @@ +import Combobox from "hw_combobox/models/combobox/base" +import { cancel } from "hw_combobox/helpers" +import { get } from "hw_combobox/vendor/requestjs" + +Combobox.Multiselect = Base => class extends Base { + async createChip() { + if (!this._isMultiselect || !this._fieldValue) return + + await get(this.selectionChipSrcValue, { + responseKind: "turbo-stream", + query: { + for_id: this.element.dataset.asyncId, + combobox_value: this._fieldValue + } + }) + + this._clearQuery() + + if (!this._isSmallViewport) { + this.openByFocusing() + } + } + + navigateChip(event) { + this._chipKeyHandlers[event.key]?.call(this, event) + } + + removeChip(event) { + event.currentTarget.closest("[data-hw-combobox-chip]").remove() + console.log("removing ", event.params.value) // TODO: implement removal + + if (!this._isSmallViewport) { + this.openByFocusing() + } + } + + _chipKeyHandlers = { + Backspace: (event) => { + this.removeChip(event) + cancel(event) + }, + Enter: (event) => { + this.removeChip(event) + cancel(event) + }, + Space: (event) => { + this.removeChip(event) + cancel(event) + }, + Escape: (event) => { + this.openByFocusing() + cancel(event) + } + } + + _focusLastChipDismisser() { + this.chipDismisserTargets[this.chipDismisserTargets.length - 1].focus() + } + + get _isMultiselect() { + return this.hasSelectionChipSrcValue + } +} diff --git a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js index 4e98f1f..29257a9 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js @@ -4,11 +4,11 @@ import { cancel } from "hw_combobox/helpers" Combobox.Navigation = Base => class extends Base { navigate(event) { if (this._autocompletesList) { - this._keyHandlers[event.key]?.call(this, event) + this._navigationKeyHandlers[event.key]?.call(this, event) } } - _keyHandlers = { + _navigationKeyHandlers = { ArrowUp: (event) => { this._selectIndex(this._selectedOptionIndex - 1) cancel(event) @@ -34,6 +34,12 @@ Combobox.Navigation = Base => class extends Base { this.close() this._actingCombobox.blur() cancel(event) + }, + Backspace: (event) => { + if (this._isMultiselect && !this._fullQuery) { + this._focusLastChipDismisser() + cancel(event) + } } } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index 4cc8d8f..07a1322 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -6,6 +6,10 @@ Combobox.Toggle = Base => class extends Base { this.expandedValue = true } + openByFocusing() { + this._actingCombobox.focus() + } + close() { if (this._isOpen) { this._lockInSelection() @@ -21,7 +25,7 @@ Combobox.Toggle = Base => class extends Base { if (this.expandedValue) { this.close() } else { - this._openByFocusing() + this.openByFocusing() } } @@ -62,10 +66,6 @@ Combobox.Toggle = Base => class extends Base { return clientX >= left && clientX <= right && clientY >= top && clientY <= bottom } - _openByFocusing() { - this._actingCombobox.focus() - } - _isDialogDismisser(target) { return target.closest("dialog") && target.role != "combobox" } diff --git a/app/assets/stylesheets/hotwire_combobox.css b/app/assets/stylesheets/hotwire_combobox.css index 68a61d8..980533d 100644 --- a/app/assets/stylesheets/hotwire_combobox.css +++ b/app/assets/stylesheets/hotwire_combobox.css @@ -10,6 +10,9 @@ --hw-border-width--slim: 1px; --hw-border-width--thick: 2px; + --hw-combobox-width: 10rem; + --hw-combobox-width--multiple: 30rem; + --hw-dialog-font-size: 1.25rem; --hw-dialog-input-height: 2.5rem; --hw-dialog-label-alignment: center; @@ -27,10 +30,8 @@ --hw-handle-width: 1.5rem; --hw-handle-width--queried: 1rem; - --hw-combobox-width: 10rem; - --hw-combobox-width--multiple: 30rem; - --hw-line-height: 1.5rem; + --hw-line-height--multiple: 2.125rem; --hw-listbox-height: calc(var(--hw-line-height) * 10); --hw-listbox-z-index: 10; @@ -39,6 +40,8 @@ --hw-padding--slim: 0.375rem; --hw-padding--thick: 0.75rem; + --hw-selection-chip-font-size: 0.875rem; + --hw-visual-viewport-height: 100vh; } @@ -60,21 +63,26 @@ .hw-combobox__main__wrapper { border: var(--hw-border-width--slim) solid var(--hw-border-color); border-radius: var(--hw-border-radius); + padding: var(--hw-padding--slim) calc(var(--hw-handle-width) + var(--hw-padding--slimmer)) var(--hw-padding--slim) var(--hw-padding--thick); position: relative; width: var(--hw-combobox-width); + + &:focus-within { + box-shadow: 0 0 0 var(--hw-border-width--thick) var(--hw-focus-color); + } } .hw-combobox__input { border: 0; font-size: inherit; line-height: var(--hw-line-height); - padding: var(--hw-padding--slim) var(--hw-handle-width) var(--hw-padding--slim) var(--hw-padding--thick); + min-width: 0; + padding: 0; text-overflow: ellipsis; width: 100%; } .hw-combobox__input:focus-visible { - box-shadow: 0 0 0 var(--hw-border-width--thick) var(--hw-focus-color); outline: none; } @@ -222,43 +230,42 @@ .hw-combobox--multiple { .hw-combobox__main__wrapper { - width: var(--hw-combobox-width--multiple); - } - - .hw-combobox__inner__wrapper { + align-items: center; display: flex; - flex: 1 1; flex-wrap: wrap; - align-items: center; - overflow: hidden; - padding: var(--hw-padding--slim); - padding-right: calc(var(--hw-handle-width) + var(--hw-padding--slimmer)); + gap: var(--hw-padding--slimmer); + width: var(--hw-combobox-width--multiple); + + &:has(.hw-combobox__chip) .hw-combobox__input::placeholder { + color: transparent; + } } - .hw-combobox__multiple_selection { - display: flex; - flex-wrap: nowrap; + .hw-combobox__chip { align-items: center; border: var(--hw-border-width--slim) solid var(--hw-border-color); border-radius: var(--hw-border-radius); - font-size: 0.85rem; + display: flex; + font-size: var(--hw-selection-chip-font-size); line-height: var(--hw-line-height); - margin-right: var(--hw-padding--slimmer); padding: var(--hw-padding--slimmer); padding-left: var(--hw-padding--slim); + } - .hw-combobox__multiple_selection__remove { - background-image: var(--hw-handle-image--queried); - background-size: var(--hw-handle-width--queried); - background-repeat: no-repeat; - cursor: pointer; - min-height: var(--hw-handle-width--queried); - min-width: var(--hw-handle-width--queried); - margin-left: var(--hw-padding--slimmer); - } + .hw-combobox__chip__dismisser { + background-image: var(--hw-handle-image--queried); + background-size: var(--hw-handle-width--queried); + background-repeat: no-repeat; + cursor: pointer; + margin-left: var(--hw-padding--slimmer); + min-height: var(--hw-handle-width--queried); + min-width: var(--hw-handle-width--queried); } .hw-combobox__input { - flex: 2 1; + flex-grow: 1; + line-height: var(--hw-line-height--multiple); + max-width: 100%; + width: 1rem; } } diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index 8fb0448..6dd283e 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -3,30 +3,30 @@ class HotwireCombobox::Component include Customizable - attr_reader :options, :label, :multiple + attr_reader :options, :label def initialize \ view, name, - association_name: nil, - async_src: nil, - autocomplete: :both, - data: {}, - dialog_label: nil, - form: nil, - id: nil, - input: {}, - label: nil, - mobile_at: "640px", - multiple: false, - name_when_new: nil, - open: false, - options: [], - value: nil, + association_name: nil, + async_src: nil, + autocomplete: :both, + data: {}, + dialog_label: nil, + form: nil, + id: nil, + input: {}, + label: nil, + mobile_at: "640px", + multiselect_chip_src: nil, + name_when_new: nil, + open: false, + options: [], + value: nil, **rest @view, @autocomplete, @id, @name, @value, @form, @async_src, @label, - @name_when_new, @open, @data, @mobile_at, @multiple, @options, @dialog_label = + @name_when_new, @open, @data, @mobile_at, @multiselect_chip_src, @options, @dialog_label = view, autocomplete, id, name.to_s, value, form, async_src, label, - name_when_new, open, data, mobile_at, multiple, options, dialog_label + name_when_new, open, data, mobile_at, multiselect_chip_src, options, dialog_label @combobox_attrs = input.reverse_merge(rest).deep_symbolize_keys @association_name = association_name || infer_association_name @@ -40,7 +40,7 @@ def render_in(view_context, &block) def fieldset_attrs apply_customizations_to :fieldset, base: { - class: [ "hw-combobox", { "hw-combobox--multiple": multiple } ], + class: [ "hw-combobox", { "hw-combobox--multiple": multiple? } ], data: fieldset_data } end @@ -73,13 +73,6 @@ def main_wrapper_attrs end - def inner_wrapper_attrs - apply_customizations_to :inner_wrapper, base: { - class: "hw-combobox__inner__wrapper" - } - end - - def input_attrs nested_attrs = %i[ data aria ] @@ -183,7 +176,11 @@ def pagination_attrs private attr_reader :view, :autocomplete, :id, :name, :value, :form, :name_when_new, :open, :data, :combobox_attrs, :mobile_at, - :association_name + :association_name, :multiselect_chip_src + + def multiple? + multiselect_chip_src.present? + end def infer_association_name if name.include?("_id") @@ -195,6 +192,7 @@ def fieldset_data data.merge \ async_id: canonical_id, controller: view.token_list("hw-combobox", data[:controller]), + action: "hw-combobox:closed->hw-combobox#createChip", hw_combobox_expanded_value: open, hw_combobox_name_when_new_value: name_when_new, hw_combobox_original_name_value: hidden_field_name, @@ -202,6 +200,7 @@ def fieldset_data hw_combobox_small_viewport_max_width_value: mobile_at, hw_combobox_async_src_value: async_src, hw_combobox_prefilled_display_value: prefilled_display, + hw_combobox_selection_chip_src_value: multiselect_chip_src, hw_combobox_filterable_attribute_value: "data-filterable-as", hw_combobox_autocompletable_attribute_value: "data-autocompletable-as", hw_combobox_selected_class: "hw-combobox__option--selected", @@ -237,7 +236,10 @@ def canonical_id def main_wrapper_data - { hw_combobox_target: "mainWrapper" } + { + action: ("click->hw-combobox#openByFocusing" if multiple?), + hw_combobox_target: "mainWrapper" + } end @@ -312,7 +314,7 @@ def listbox_data end def listbox_aria - { multiselectable: multiple } + { multiselectable: multiple? } end @@ -355,7 +357,7 @@ def dialog_listbox_data end def dialog_listbox_aria - { multiselectable: multiple } + { multiselectable: multiple? } end def dialog_focus_trap_data diff --git a/app/presenters/hotwire_combobox/component/customizable.rb b/app/presenters/hotwire_combobox/component/customizable.rb index 9721c39..14013bf 100644 --- a/app/presenters/hotwire_combobox/component/customizable.rb +++ b/app/presenters/hotwire_combobox/component/customizable.rb @@ -4,7 +4,6 @@ module HotwireCombobox::Component::Customizable label hidden_field main_wrapper - inner_wrapper input handle listbox diff --git a/app/views/hotwire_combobox/_selection_chip.html.erb b/app/views/hotwire_combobox/_selection_chip.html.erb new file mode 100644 index 0000000..93c52cd --- /dev/null +++ b/app/views/hotwire_combobox/_selection_chip.html.erb @@ -0,0 +1,8 @@ +<%# locals: (display:, value:, for_id:) -%> + +<%= turbo_stream.before for_id do %> + <%= tag.div class: "hw-combobox__chip", data: { hw_combobox_chip: "" } do %> + <%= tag.span display %> + <%= tag.span **combobox_chip_dismisser_attrs(value) %> + <% end %> +<% end %> diff --git a/app/views/hotwire_combobox/combobox/_input.html.erb b/app/views/hotwire_combobox/combobox/_input.html.erb index de60d74..366ea11 100644 --- a/app/views/hotwire_combobox/combobox/_input.html.erb +++ b/app/views/hotwire_combobox/combobox/_input.html.erb @@ -1,4 +1,2 @@ -<%= tag.div **component.inner_wrapper_attrs do %> - <%= tag.input **component.input_attrs %> - <%= tag.span **component.handle_attrs %> -<% end %> +<%= tag.input **component.input_attrs %> +<%= tag.span **component.handle_attrs %> diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index 4d29e59..51738a1 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -46,6 +46,24 @@ def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request alias_method :hw_async_combobox_options, :hw_paginated_combobox_options hw_alias :hw_async_combobox_options + def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id]) + render "hotwire_combobox/selection_chip", display: display, value: value, for_id: for_id + end + hw_alias :hw_combobox_selection_chip + + def hw_combobox_chip_dismisser_attrs(value) + { + tabindex: "0", + class: "hw-combobox__chip__dismisser", + data: { + action: "click->hw-combobox#removeChip:stop keydown->hw-combobox#navigateChip", + hw_combobox_target: "chipDismisser", + hw_combobox_value_param: value + } + } + end + hw_alias :hw_combobox_chip_dismisser_attrs + # private library use only def hw_listbox_id(id) "#{id}-hw-listbox" diff --git a/test/dummy/app/controllers/state_chips_controller.rb b/test/dummy/app/controllers/state_chips_controller.rb new file mode 100644 index 0000000..d0af961 --- /dev/null +++ b/test/dummy/app/controllers/state_chips_controller.rb @@ -0,0 +1,5 @@ +class StateChipsController < ApplicationController + def new + @state = State.find params[:combobox_value] + end +end diff --git a/test/dummy/app/views/comboboxes/multiple.html.erb b/test/dummy/app/views/comboboxes/multiple.html.erb index 15c0eaa..a8df8f8 100644 --- a/test/dummy/app/views/comboboxes/multiple.html.erb +++ b/test/dummy/app/views/comboboxes/multiple.html.erb @@ -1 +1,4 @@ -<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: [], placeholder: 'State' %> +<%= combobox_tag "state", State.all, + id: "state-field", + multiselect_chip_src: new_state_chip_path, + placeholder: "select states" %> diff --git a/test/dummy/app/views/state_chips/new.turbo_stream.erb b/test/dummy/app/views/state_chips/new.turbo_stream.erb new file mode 100644 index 0000000..5f771f5 --- /dev/null +++ b/test/dummy/app/views/state_chips/new.turbo_stream.erb @@ -0,0 +1 @@ +<%= combobox_selection_chip display: @state.name %> diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 14424c9..56db049 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -35,6 +35,7 @@ post "new_options_form", to: "new_options_forms#create" resources :states, only: :index + resources :state_chips, only: :new, param: :combobox_value resources :users, only: :update root to: "comboboxes#plain" diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 5789940..151f2c3 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -799,7 +799,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase click_on_option "Alabama" assert_open_combobox assert_no_selector "input[placeholder='State']" - find("#AL .hw-combobox__multiple_selection__remove").click + find("#AL .hw-combobox__chip__dismisser").click assert_selector "input[placeholder='State']" end @@ -821,7 +821,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase type_in_combobox "#state-field", :enter assert_open_combobox assert_selector "div[id='AL'] div", text: "Alabama" - find("#AL .hw-combobox__multiple_selection__remove").click + find("#AL .hw-combobox__chip__dismisser").click assert_no_selector "div[id='AL'] div", text: "Alabama" click_on_option "Minnesota" @@ -848,7 +848,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_text "selections: 1." assert_no_text "event: hw-combobox:closed" - find("#MO .hw-combobox__multiple_selection__remove").click + find("#MO .hw-combobox__chip__dismisser").click assert_text 'value: ["FL","MN"]' assert_text "selections: 2." assert_no_text "event: hw-combobox:closed" From 6d68b2492a1348eef7d9332ec9c6f299cab00e19 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sat, 16 Mar 2024 16:45:53 -0600 Subject: [PATCH 08/32] Persist multiselect values and hide selected options --- .../controllers/hw_combobox_controller.js | 4 +- .../hw_combobox/models/combobox/events.js | 4 +- .../hw_combobox/models/combobox/filtering.js | 25 +++-- .../hw_combobox/models/combobox/form_field.js | 52 +++++++-- .../models/combobox/multiselect.js | 101 ++++++++++++++---- .../hw_combobox/models/combobox/options.js | 16 +-- .../hw_combobox/models/combobox/selection.js | 36 ++++--- .../hw_combobox/models/combobox/toggle.js | 9 +- .../hw_combobox/models/combobox/validity.js | 2 +- app/assets/stylesheets/hotwire_combobox.css | 1 + app/presenters/hotwire_combobox/component.rb | 3 +- 11 files changed, 182 insertions(+), 71 deletions(-) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 1e5d81a..36dc5fe 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -82,9 +82,11 @@ export default class HwComboboxController extends Concerns(...concerns) { const inputType = element.dataset.inputType const delay = window.HOTWIRE_COMBOBOX_STREAM_DELAY + this._resetMultiselectionMarks() + if (inputType && inputType !== "hw:lockInSelection") { if (delay) await sleep(delay) - this._selectOnQuery({ inputType }) + this._selectOnQuery(inputType) } else { this._preselect() } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/events.js b/app/assets/javascripts/hw_combobox/models/combobox/events.js index c1b2e05..5b3ec07 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/events.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/events.js @@ -3,7 +3,7 @@ import { dispatch } from "hw_combobox/helpers" Combobox.Events = Base => class extends Base { _dispatchSelectionEvent({ isNewAndAllowed, previousValue }) { - if (previousValue === this._fieldValue) return + if (previousValue === this._fieldValueString) return dispatch("hw-combobox:selection", { target: this.element, @@ -20,7 +20,7 @@ Combobox.Events = Base => class extends Base { get _eventableDetails() { return { - value: this._fieldValue, + value: this._fieldValueString, display: this._fullQuery, query: this._typedQuery, fieldName: this._fieldName, diff --git a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js index ec5af50..a4f309c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/filtering.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/filtering.js @@ -4,11 +4,11 @@ import { applyFilter, debounce, unselectedPortion } from "hw_combobox/helpers" import { get } from "hw_combobox/vendor/requestjs" Combobox.Filtering = Base => class extends Base { - filterAndSelect(event) { - this._filter(event) + filterAndSelect({ inputType }) { + this._filter(inputType) if (this._isSync) { - this._selectOnQuery(event) + this._selectOnQuery(inputType) } else { // noop, async selection is handled by stimulus callbacks } @@ -18,9 +18,9 @@ Combobox.Filtering = Base => class extends Base { this._debouncedFilterAsync = debounce(this._debouncedFilterAsync.bind(this)) } - _filter(event) { + _filter(inputType) { if (this._isAsync) { - this._debouncedFilterAsync(event) + this._debouncedFilterAsync(inputType) } else { this._filterSync() } @@ -28,14 +28,14 @@ Combobox.Filtering = Base => class extends Base { this._markQueried() } - _debouncedFilterAsync(event) { - this._filterAsync(event) + _debouncedFilterAsync(inputType) { + this._filterAsync(inputType) } - async _filterAsync(event) { + async _filterAsync(inputType) { const query = { q: this._fullQuery, - input_type: event.inputType, + input_type: inputType, for_id: this.element.dataset.asyncId } @@ -43,7 +43,12 @@ Combobox.Filtering = Base => class extends Base { } _filterSync() { - this._allOptionElements.forEach(applyFilter(this._fullQuery, { matching: this.filterableAttributeValue })) + this._allFilterableOptionElements.forEach( + applyFilter( + this._fullQuery, + { matching: this.filterableAttributeValue } + ) + ) } _clearQuery() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js index 537f22c..738eb59 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js @@ -1,19 +1,59 @@ import Combobox from "hw_combobox/models/combobox/base" Combobox.FormField = Base => class extends Base { - _setFieldValue(value) { - this.hiddenFieldTarget.value = value + get _fieldValue() { + if (this._isMultiselect) { + const currentValue = this.hiddenFieldTarget.value + const arrayFromValue = currentValue ? currentValue.split(",") : [] + + return new Set(arrayFromValue) + } else { + return this.hiddenFieldTarget.value + } } - _setFieldName(value) { - this.hiddenFieldTarget.name = value + get _fieldValueString() { + if (this._isMultiselect) { + return this._fieldValueArray.join(",") + } else { + return this.hiddenFieldTarget.value + } } - get _fieldValue() { - return this.hiddenFieldTarget.value + get _fieldValueArray() { + if (this._isMultiselect) { + return Array.from(this._fieldValue) + } else { + return [ this.hiddenFieldTarget.value ] + } + } + + set _fieldValue(value) { + if (this._isMultiselect) { + this.hiddenFieldTarget.dataset.valueForMultiselect = value + this.hiddenFieldTarget.dataset.displayForMultiselect = this._fullQuery + } else { + this.hiddenFieldTarget.value = value + } + } + + get _hasEmptyFieldValue() { + if (this._isMultiselect) { + return this.hiddenFieldTarget.dataset.valueForMultiselect === "" + } else { + return this.hiddenFieldTarget.value === "" + } + } + + get _hasFieldValue() { + return !this._hasEmptyFieldValue } get _fieldName() { return this.hiddenFieldTarget.name } + + set _fieldName(value) { + this.hiddenFieldTarget.name = value + } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index fe5955a..0048b56 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -3,31 +3,19 @@ import { cancel } from "hw_combobox/helpers" import { get } from "hw_combobox/vendor/requestjs" Combobox.Multiselect = Base => class extends Base { - async createChip() { - if (!this._isMultiselect || !this._fieldValue) return - - await get(this.selectionChipSrcValue, { - responseKind: "turbo-stream", - query: { - for_id: this.element.dataset.asyncId, - combobox_value: this._fieldValue - } - }) - - this._clearQuery() - - if (!this._isSmallViewport) { - this.openByFocusing() - } - } - navigateChip(event) { this._chipKeyHandlers[event.key]?.call(this, event) } - removeChip(event) { - event.currentTarget.closest("[data-hw-combobox-chip]").remove() - console.log("removing ", event.params.value) // TODO: implement removal + removeChip({ currentTarget, params }) { + const option = this._optionElementWithValue(params.value) + + this._markNotSelected(option) + this._markNotMultiselected(option) + this._removeFromFieldValue(params.value) + this._filter("hw:multiselectSync") + + currentTarget.closest("[data-hw-combobox-chip]").remove() if (!this._isSmallViewport) { this.openByFocusing() @@ -53,6 +41,73 @@ Combobox.Multiselect = Base => class extends Base { } } + _createChip() { + if (!this._isMultiselect) return + + this._beforeClearingMultiselectQuery(async (display, value) => { + this._fullQuery = "" + this._filter("hw:multiselectSync") + + await get(this.selectionChipSrcValue, { + responseKind: "turbo-stream", + query: { + for_id: this.element.dataset.asyncId, + combobox_value: value, + display_value: display + } + }) + + this._addToFieldValue(value) + }) + } + + _beforeClearingMultiselectQuery(callback) { + const display = this.hiddenFieldTarget.dataset.displayForMultiselect + const value = this.hiddenFieldTarget.dataset.valueForMultiselect + + if (value && !this._fieldValue.has(value)) { + callback(display, value) + } + + this.hiddenFieldTarget.dataset.displayForMultiselect = "" + this.hiddenFieldTarget.dataset.valueForMultiselect = "" + } + + _resetMultiselectionMarks() { + if (!this._isMultiselect) return + + this._fieldValueArray.forEach(value => { + const option = this._optionElementWithValue(value) + option.setAttribute("data-multiselected", "") + option.hidden = true + }) + } + + _markNotMultiselected(option) { + if (!this._isMultiselect) return + + option.removeAttribute("data-multiselected") + option.hidden = false + } + + _addToFieldValue(value) { + let newValue = this._fieldValue + + newValue.add(String(value)) + this.hiddenFieldTarget.value = Array.from(newValue).join(",") + + if (this._isSync) this._resetMultiselectionMarks() + } + + _removeFromFieldValue(value) { + let newValue = this._fieldValue + + newValue.delete(String(value)) + this.hiddenFieldTarget.value = Array.from(newValue).join(",") + + if (this._isSync) this._resetMultiselectionMarks() + } + _focusLastChipDismisser() { this.chipDismisserTargets[this.chipDismisserTargets.length - 1].focus() } @@ -60,4 +115,8 @@ Combobox.Multiselect = Base => class extends Base { get _isMultiselect() { return this.hasSelectionChipSrcValue } + + get _isSingleSelect() { + return !this._isMultiselect + } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/options.js b/app/assets/javascripts/hw_combobox/models/combobox/options.js index 10634fc..7eee6eb 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/options.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/options.js @@ -11,24 +11,28 @@ Combobox.Options = Base => class extends Base { } _resetOptions(deselectionStrategy) { - this._setFieldName(this.originalNameValue) + this._fieldName = this.originalNameValue deselectionStrategy() } + _optionElementWithValue(value) { + return this._actingListbox.querySelector(`[${this.filterableAttributeValue}][data-value='${value}']`) + } + get _allowNew() { return !!this.nameWhenNewValue } get _allOptions() { - return Array.from(this._allOptionElements) + return Array.from(this._allFilterableOptionElements) } - get _allOptionElements() { - return this._actingListbox.querySelectorAll(`[${this.filterableAttributeValue}]`) + get _allFilterableOptionElements() { + return this._actingListbox.querySelectorAll(`[${this.filterableAttributeValue}]:not([data-multiselected])`) } get _visibleOptionElements() { - return [ ...this._allOptionElements ].filter(visible) + return [ ...this._allFilterableOptionElements ].filter(visible) } get _selectedOptionElement() { @@ -40,7 +44,7 @@ Combobox.Options = Base => class extends Base { } get _isUnjustifiablyBlank() { - const valueIsMissing = !this._fieldValue + const valueIsMissing = this._hasEmptyFieldValue const noBlankOptionSelected = !this._selectedOptionElement return valueIsMissing && noBlankOptionSelected diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index 6c3c36e..f7bd3be 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -2,8 +2,8 @@ import Combobox from "hw_combobox/models/combobox/base" import { wrapAroundAccess, isDeleteEvent } from "hw_combobox/helpers" Combobox.Selection = Base => class extends Base { - selectOnClick(event) { - this._forceSelectionAndFilter(event.currentTarget, event) + selectOnClick({ currentTarget, inputType }) { + this._forceSelectionAndFilter(currentTarget, inputType) this.close() } @@ -13,12 +13,12 @@ Combobox.Selection = Base => class extends Base { } } - _selectOnQuery(inputEvent) { - if (this._shouldTreatAsNewOptionForFiltering(!isDeleteEvent(inputEvent))) { + _selectOnQuery(inputType) { + if (this._shouldTreatAsNewOptionForFiltering(!isDeleteEvent({ inputType: inputType }))) { this._selectNew() - } else if (isDeleteEvent(inputEvent)) { + } else if (isDeleteEvent({ inputType: inputType })) { this._deselect() - } else if (inputEvent.inputType === "hw:lockInSelection" && this._ensurableOption) { + } else if (inputType === "hw:lockInSelection" && this._ensurableOption) { this._selectAndAutocompleteMissingPortion(this._ensurableOption) } else if (this._isOpen && this._visibleOptionElements[0]) { this._selectAndAutocompleteMissingPortion(this._visibleOptionElements[0]) @@ -36,13 +36,13 @@ Combobox.Selection = Base => class extends Base { } _select(option, autocompleteStrategy) { - const previousValue = this._fieldValue + const previousValue = this._fieldValueString this._resetOptionsSilently() autocompleteStrategy(option) - this._setFieldValue(option.dataset.value) + this._fieldValue = option.dataset.value this._markSelected(option) this._markValid() this._dispatchSelectionEvent({ isNewAndAllowed: false, previousValue: previousValue }) @@ -51,23 +51,23 @@ Combobox.Selection = Base => class extends Base { } _selectNew() { - const previousValue = this._fieldValue + const previousValue = this._fieldValueString this._resetOptionsSilently() - this._setFieldValue(this._fullQuery) - this._setFieldName(this.nameWhenNewValue) + this._fieldValue = this._fullQuery + this._fieldName = this.nameWhenNewValue this._markValid() this._dispatchSelectionEvent({ isNewAndAllowed: true, previousValue: previousValue }) } _deselect() { - const previousValue = this._fieldValue + const previousValue = this._fieldValueString if (this._selectedOptionElement) { this._markNotSelected(this._selectedOptionElement) } - this._setFieldValue(null) + this._fieldValue = "" this._setActiveDescendant("") return previousValue @@ -84,6 +84,8 @@ Combobox.Selection = Base => class extends Base { } _preselect() { + // TODO: Implement preselection in multiselect, consider associations in form builders + if (this._hasValueButNoSelection && this._allOptions.length < 100) { const option = this._allOptions.find(option => { return option.dataset.value === this._fieldValue @@ -101,9 +103,9 @@ Combobox.Selection = Base => class extends Base { this._select(option, this._replaceFullQueryWithAutocompletedValue.bind(this)) } - _forceSelectionAndFilter(option, event) { + _forceSelectionAndFilter(option, inputType) { this._forceSelectionWithoutFiltering(option) - this._filter(event) + this._filter(inputType) } _forceSelectionWithoutFiltering(option) { @@ -112,7 +114,7 @@ Combobox.Selection = Base => class extends Base { _lockInSelection() { if (this._shouldLockInSelection) { - this._forceSelectionAndFilter(this._ensurableOption, { inputType: "hw:lockInSelection" }) + this._forceSelectionAndFilter(this._ensurableOption, "hw:lockInSelection") } } @@ -137,7 +139,7 @@ Combobox.Selection = Base => class extends Base { } get _hasValueButNoSelection() { - return this._fieldValue && !this._selectedOptionElement + return this._hasFieldValue && !this._selectedOptionElement } get _shouldLockInSelection() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index 07a1322..7036503 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -18,6 +18,7 @@ Combobox.Toggle = Base => class extends Base { this.expandedValue = false this._dispatchClosedEvent() + this._createChip() } } @@ -71,7 +72,9 @@ Combobox.Toggle = Base => class extends Base { } _expand() { - if (this._preselectOnExpansion) this._preselect() + if (this._isSync) { + this._preselect() + } if (this._autocompletesList && this._isSmallViewport) { this._openInDialog() @@ -134,8 +137,4 @@ Combobox.Toggle = Base => class extends Base { get _isOpen() { return this.expandedValue } - - get _preselectOnExpansion() { - return !this._isAsync // async comboboxes preselect based on callbacks - } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/validity.js b/app/assets/javascripts/hw_combobox/models/combobox/validity.js index 46f1ae3..eb68de7 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/validity.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/validity.js @@ -34,7 +34,7 @@ Combobox.Validity = Base => class extends Base { // +_valueIsInvalid+ only checks if `comboboxTarget` (and not `_actingCombobox`) is required // because the `required` attribute is only forwarded to the `comboboxTarget` element get _valueIsInvalid() { - const isRequiredAndEmpty = this.comboboxTarget.required && !this._fieldValue + const isRequiredAndEmpty = this.comboboxTarget.required && this._hasEmptyFieldValue return isRequiredAndEmpty } } diff --git a/app/assets/stylesheets/hotwire_combobox.css b/app/assets/stylesheets/hotwire_combobox.css index 980533d..5f745f3 100644 --- a/app/assets/stylesheets/hotwire_combobox.css +++ b/app/assets/stylesheets/hotwire_combobox.css @@ -263,6 +263,7 @@ } .hw-combobox__input { + min-width: calc(var(--hw-combobox-width) / 2); flex-grow: 1; line-height: var(--hw-line-height--multiple); max-width: 100%; diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index 6dd283e..1726f7d 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -192,7 +192,6 @@ def fieldset_data data.merge \ async_id: canonical_id, controller: view.token_list("hw-combobox", data[:controller]), - action: "hw-combobox:closed->hw-combobox#createChip", hw_combobox_expanded_value: open, hw_combobox_name_when_new_value: name_when_new, hw_combobox_original_name_value: hidden_field_name, @@ -237,7 +236,7 @@ def canonical_id def main_wrapper_data { - action: ("click->hw-combobox#openByFocusing" if multiple?), + action: ("click->hw-combobox#openByFocusing:self" if multiple?), hw_combobox_target: "mainWrapper" } end From 7f894beaff37b98f09edcd423832aa0c1ae9bfe5 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sat, 16 Mar 2024 18:15:27 -0600 Subject: [PATCH 09/32] Reopen multiselect --- .../javascripts/controllers/hw_combobox_controller.js | 5 +++++ .../hw_combobox/models/combobox/multiselect.js | 1 + .../hw_combobox/models/combobox/navigation.js | 6 ++---- .../hw_combobox/models/combobox/selection.js | 2 +- .../javascripts/hw_combobox/models/combobox/toggle.js | 11 ++++++++--- app/views/hotwire_combobox/_selection_chip.html.erb | 4 ++-- lib/hotwire_combobox/helper.rb | 10 ++++++++-- 7 files changed, 27 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 36dc5fe..ba4d15b 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -31,6 +31,7 @@ export default class HwComboboxController extends Concerns(...concerns) { static targets = [ "combobox", "chipDismisser", + "closer", "dialog", "dialogCombobox", "dialogFocusTrap", @@ -92,6 +93,10 @@ export default class HwComboboxController extends Concerns(...concerns) { } } + closerTargetConnected() { + this._closeAndBlur() + } + // Use +_printStack+ for debugging purposes _printStack() { const err = new Error() diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index 0048b56..37f688c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -58,6 +58,7 @@ Combobox.Multiselect = Base => class extends Base { }) this._addToFieldValue(value) + this.openByFocusing() }) } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js index 29257a9..182b85a 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js @@ -26,13 +26,11 @@ Combobox.Navigation = Base => class extends Base { cancel(event) }, Enter: (event) => { - this.close() - this._actingCombobox.blur() + this._closeAndBlur() cancel(event) }, Escape: (event) => { - this.close() - this._actingCombobox.blur() + this._closeAndBlur() cancel(event) }, Backspace: (event) => { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index f7bd3be..2e7e949 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -4,7 +4,7 @@ import { wrapAroundAccess, isDeleteEvent } from "hw_combobox/helpers" Combobox.Selection = Base => class extends Base { selectOnClick({ currentTarget, inputType }) { this._forceSelectionAndFilter(currentTarget, inputType) - this.close() + this._closeAndBlur() } _connectSelection() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index 7036503..b2e68e0 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -24,7 +24,7 @@ Combobox.Toggle = Base => class extends Base { toggle() { if (this.expandedValue) { - this.close() + this._closeAndBlur() } else { this.openByFocusing() } @@ -37,14 +37,14 @@ Combobox.Toggle = Base => class extends Base { if (this.mainWrapperTarget.contains(target) && !this._isDialogDismisser(target)) return if (this._withinElementBounds(event)) return - this.close() + this._closeAndBlur() } closeOnFocusOutside({ target }) { if (!this._isOpen) return if (this.element.contains(target)) return - this.close() + this._closeAndBlur() } clearOrToggleOnHandleClick() { @@ -56,6 +56,11 @@ Combobox.Toggle = Base => class extends Base { } } + _closeAndBlur() { + this.close() + this._actingCombobox.blur() + } + // Some browser extensions like 1Password overlay elements on top of the combobox. // Hovering over these elements emits a click event for some reason. // These events don't contain any telling information, so we use `_withinElementBounds` diff --git a/app/views/hotwire_combobox/_selection_chip.html.erb b/app/views/hotwire_combobox/_selection_chip.html.erb index 93c52cd..ec88cb8 100644 --- a/app/views/hotwire_combobox/_selection_chip.html.erb +++ b/app/views/hotwire_combobox/_selection_chip.html.erb @@ -1,7 +1,7 @@ -<%# locals: (display:, value:, for_id:) -%> +<%# locals: (display:, value:, for_id:, data:) -%> <%= turbo_stream.before for_id do %> - <%= tag.div class: "hw-combobox__chip", data: { hw_combobox_chip: "" } do %> + <%= tag.div class: "hw-combobox__chip", data: data do %> <%= tag.span display %> <%= tag.span **combobox_chip_dismisser_attrs(value) %> <% end %> diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index 51738a1..dadd611 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -46,11 +46,17 @@ def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request alias_method :hw_async_combobox_options, :hw_paginated_combobox_options hw_alias :hw_async_combobox_options - def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id]) - render "hotwire_combobox/selection_chip", display: display, value: value, for_id: for_id + def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id], data: {}) + data = { hw_combobox_chip: "" }.reverse_merge data + render "hotwire_combobox/selection_chip", display: display, value: value, for_id: for_id, data: data end hw_alias :hw_combobox_selection_chip + def hw_dismissing_combobox_selection_chip(*args, **kwargs) + hw_combobox_selection_chip(*args, **kwargs, data: { hw_combobox_target: "closer" }) + end + hw_alias :hw_dismissing_combobox_selection_chip + def hw_combobox_chip_dismisser_attrs(value) { tabindex: "0", From 39b1861dfe137b0b33d7834bc9f47615145caa78 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sat, 16 Mar 2024 18:55:50 -0600 Subject: [PATCH 10/32] Move dismissing concern to chip remover --- .../hotwire_combobox/_selection_chip.html.erb | 6 ++-- lib/hotwire_combobox/helper.rb | 29 ++++++++++++++----- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/app/views/hotwire_combobox/_selection_chip.html.erb b/app/views/hotwire_combobox/_selection_chip.html.erb index ec88cb8..cbfe3ea 100644 --- a/app/views/hotwire_combobox/_selection_chip.html.erb +++ b/app/views/hotwire_combobox/_selection_chip.html.erb @@ -1,8 +1,8 @@ -<%# locals: (display:, value:, for_id:, data:) -%> +<%# locals: (display:, value:, for_id:, remover_attrs:) -%> <%= turbo_stream.before for_id do %> - <%= tag.div class: "hw-combobox__chip", data: data do %> + <%= tag.div class: "hw-combobox__chip", data: { hw_combobox_chip: "" } do %> <%= tag.span display %> - <%= tag.span **combobox_chip_dismisser_attrs(value) %> + <%= tag.span **remover_attrs %> <% end %> <% end %> diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index dadd611..25607cd 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -46,21 +46,29 @@ def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request alias_method :hw_async_combobox_options, :hw_paginated_combobox_options hw_alias :hw_async_combobox_options - def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id], data: {}) - data = { hw_combobox_chip: "" }.reverse_merge data - render "hotwire_combobox/selection_chip", display: display, value: value, for_id: for_id, data: data + def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id], remover_attrs: hw_combobox_chip_remover_attrs(display, value)) + render "hotwire_combobox/selection_chip", + display: display, + value: value, + for_id: for_id, + remover_attrs: remover_attrs end hw_alias :hw_combobox_selection_chip - def hw_dismissing_combobox_selection_chip(*args, **kwargs) - hw_combobox_selection_chip(*args, **kwargs, data: { hw_combobox_target: "closer" }) + def hw_dismissing_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id]) + hw_combobox_selection_chip \ + display: display, + value: value, + for_id: for_id, + remover_attrs: hw_combobox_dismissing_chip_remover_attrs(display, value) end hw_alias :hw_dismissing_combobox_selection_chip - def hw_combobox_chip_dismisser_attrs(value) + def hw_combobox_chip_remover_attrs(display, value) { tabindex: "0", class: "hw-combobox__chip__dismisser", + aria: { label: "Remove #{display}" }, data: { action: "click->hw-combobox#removeChip:stop keydown->hw-combobox#navigateChip", hw_combobox_target: "chipDismisser", @@ -68,7 +76,14 @@ def hw_combobox_chip_dismisser_attrs(value) } } end - hw_alias :hw_combobox_chip_dismisser_attrs + hw_alias :hw_combobox_chip_remover_attrs + + def hw_combobox_dismissing_chip_remover_attrs(display, value) + hw_combobox_chip_remover_attrs(display, value).tap do |attrs| + attrs[:data][:hw_combobox_target] = token_list(attrs[:data][:hw_combobox_target], "closer") + end + end + hw_alias :hw_combobox_dismissing_chip_remover_attrs # private library use only def hw_listbox_id(id) From c4e0f02911b8d3dd9fcf81dc952f9ac71a3da1b3 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sat, 16 Mar 2024 18:57:21 -0600 Subject: [PATCH 11/32] Group hw_alias calls --- lib/hotwire_combobox/helper.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index 25607cd..e309d7e 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -13,14 +13,12 @@ def hw_alias(method_name) def hw_combobox_style_tag(*args, **kwargs) stylesheet_link_tag HotwireCombobox.stylesheet_path, *args, **kwargs end - hw_alias :hw_combobox_style_tag def hw_combobox_tag(name, options_or_src = [], render_in: {}, include_blank: nil, **kwargs, &block) options, src = hw_extract_options_and_src(options_or_src, render_in, include_blank) component = HotwireCombobox::Component.new self, name, options: options, async_src: src, **kwargs render component, &block end - hw_alias :hw_combobox_tag def hw_combobox_options(options, render_in: {}, include_blank: nil, display: :to_combobox_display, **methods) if options.first.is_a? HotwireCombobox::Listbox::Option @@ -31,7 +29,6 @@ def hw_combobox_options(options, render_in: {}, include_blank: nil, display: :to opts end end - hw_alias :hw_combobox_options def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request.path, next_page: nil, render_in: {}, include_blank: {}, **methods) include_blank = params[:page] ? nil : include_blank @@ -41,10 +38,7 @@ def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request safe_join [ this_page, next_page ] end - hw_alias :hw_paginated_combobox_options - alias_method :hw_async_combobox_options, :hw_paginated_combobox_options - hw_alias :hw_async_combobox_options def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id], remover_attrs: hw_combobox_chip_remover_attrs(display, value)) render "hotwire_combobox/selection_chip", @@ -53,7 +47,6 @@ def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: for_id: for_id, remover_attrs: remover_attrs end - hw_alias :hw_combobox_selection_chip def hw_dismissing_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id]) hw_combobox_selection_chip \ @@ -62,7 +55,6 @@ def hw_dismissing_combobox_selection_chip(display:, value: params[:combobox_valu for_id: for_id, remover_attrs: hw_combobox_dismissing_chip_remover_attrs(display, value) end - hw_alias :hw_dismissing_combobox_selection_chip def hw_combobox_chip_remover_attrs(display, value) { @@ -76,13 +68,21 @@ def hw_combobox_chip_remover_attrs(display, value) } } end - hw_alias :hw_combobox_chip_remover_attrs def hw_combobox_dismissing_chip_remover_attrs(display, value) hw_combobox_chip_remover_attrs(display, value).tap do |attrs| attrs[:data][:hw_combobox_target] = token_list(attrs[:data][:hw_combobox_target], "closer") end end + + hw_alias :hw_combobox_style_tag + hw_alias :hw_combobox_tag + hw_alias :hw_combobox_options + hw_alias :hw_paginated_combobox_options + hw_alias :hw_async_combobox_options + hw_alias :hw_combobox_selection_chip + hw_alias :hw_dismissing_combobox_selection_chip + hw_alias :hw_combobox_chip_remover_attrs hw_alias :hw_combobox_dismissing_chip_remover_attrs # private library use only From 70673bc60fefabc61fb5c82449262970cf8a8250 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sun, 17 Mar 2024 11:22:31 -0600 Subject: [PATCH 12/32] Announce selections to screenreader --- README.md | 1 + .../controllers/hw_combobox_controller.js | 2 ++ .../hw_combobox/models/combobox.js | 1 + .../models/combobox/announcements.js | 7 +++++ .../models/combobox/multiselect.js | 4 +++ .../hw_combobox/models/combobox/options.js | 2 +- .../hw_combobox/models/combobox/toggle.js | 5 ++++ app/assets/stylesheets/hotwire_combobox.css | 2 -- app/presenters/hotwire_combobox/component.rb | 29 +++++++++++++++++++ .../hotwire_combobox/_component.html.erb | 1 + .../app/views/comboboxes/multiple.html.erb | 3 +- 11 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 app/assets/javascripts/hw_combobox/models/combobox/announcements.js diff --git a/README.md b/README.md index ffc7152..deeea9d 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,7 @@ These are the exceptions: 2. The escape key closes the listbox and blurs the combobox. It does not clear the combobox. 3. The listbox has wrap-around selection. That is, pressing `Up Arrow` when the user is on the first option will select the last option. And pressing `Down Arrow` when on the last option will select the first option. In paginated comboboxes, the first and last options refer to the currently available options. More options may be loaded after navigating to the last currently available option. 4. It is possible to have an unlabled combobox, as that responsibility is delegated to the implementing user. +5. There are currently [no APG guidelines](https://github.com/w3c/aria-practices/issues/1512) for a multiselect combobox. We've introduced some mechanisms to make the experience accessible, like announcing multi-selections via a live region. But we'd welcome feedback on how to make it better until official guidelines are available. It should be noted none of the maintainers use assistive technologies in their daily lives. If you do, and you feel these exceptions are detrimental to your ability to use the component, or if you find an undocumented exception, please [open a GitHub issue](https://github.com/josefarias/hotwire_combobox/issues). We'll get it sorted. diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index ba4d15b..0e3e955 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -7,6 +7,7 @@ window.HOTWIRE_COMBOBOX_STREAM_DELAY = 0 // ms, for testing purposes const concerns = [ Controller, Combobox.Actors, + Combobox.Announcements, Combobox.AsyncLoading, Combobox.Autocomplete, Combobox.Dialog, @@ -29,6 +30,7 @@ export default class HwComboboxController extends Concerns(...concerns) { ] static targets = [ + "announcer", "combobox", "chipDismisser", "closer", diff --git a/app/assets/javascripts/hw_combobox/models/combobox.js b/app/assets/javascripts/hw_combobox/models/combobox.js index a57bb10..dedfcad 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox.js +++ b/app/assets/javascripts/hw_combobox/models/combobox.js @@ -1,6 +1,7 @@ import Combobox from "hw_combobox/models/combobox/base" import "hw_combobox/models/combobox/actors" +import "hw_combobox/models/combobox/announcements" import "hw_combobox/models/combobox/async_loading" import "hw_combobox/models/combobox/autocomplete" import "hw_combobox/models/combobox/dialog" diff --git a/app/assets/javascripts/hw_combobox/models/combobox/announcements.js b/app/assets/javascripts/hw_combobox/models/combobox/announcements.js new file mode 100644 index 0000000..9c5633c --- /dev/null +++ b/app/assets/javascripts/hw_combobox/models/combobox/announcements.js @@ -0,0 +1,7 @@ +import Combobox from "hw_combobox/models/combobox/base" + +Combobox.Announcements = Base => class extends Base { + _announceToScreenReader(display, action) { + this.announcerTarget.innerText = `${display} ${action}` + } +} diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index 37f688c..b703a44 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -9,6 +9,7 @@ Combobox.Multiselect = Base => class extends Base { removeChip({ currentTarget, params }) { const option = this._optionElementWithValue(params.value) + const display = option.getAttribute(this.autocompletableAttributeValue) this._markNotSelected(option) this._markNotMultiselected(option) @@ -20,6 +21,8 @@ Combobox.Multiselect = Base => class extends Base { if (!this._isSmallViewport) { this.openByFocusing() } + + this._announceToScreenReader(display, "removed") } _chipKeyHandlers = { @@ -59,6 +62,7 @@ Combobox.Multiselect = Base => class extends Base { this._addToFieldValue(value) this.openByFocusing() + this._announceToScreenReader(display, "multi-selected. Press Shift + Tab, then Enter to remove.") }) } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/options.js b/app/assets/javascripts/hw_combobox/models/combobox/options.js index 7eee6eb..514fff9 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/options.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/options.js @@ -36,7 +36,7 @@ Combobox.Options = Base => class extends Base { } get _selectedOptionElement() { - return this._actingListbox.querySelector("[role=option][aria-selected=true]") + return this._actingListbox.querySelector("[role=option][aria-selected=true]:not([data-multiselected])") } get _selectedOptionIndex() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index b2e68e0..33685e6 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -19,6 +19,11 @@ Combobox.Toggle = Base => class extends Base { this._dispatchClosedEvent() this._createChip() + + if (this._isSingleSelect) { + const display = this._selectedOptionElement.getAttribute(this.autocompletableAttributeValue) + this._announceToScreenReader(display, "selected") + } } } diff --git a/app/assets/stylesheets/hotwire_combobox.css b/app/assets/stylesheets/hotwire_combobox.css index 5f745f3..c022d4c 100644 --- a/app/assets/stylesheets/hotwire_combobox.css +++ b/app/assets/stylesheets/hotwire_combobox.css @@ -114,7 +114,6 @@ .hw-combobox__input[data-queried] + .hw-combobox__handle::before { background-image: var(--hw-handle-image--queried); background-size: var(--hw-handle-width--queried); - cursor: pointer; } .hw-combobox__listbox { @@ -256,7 +255,6 @@ background-image: var(--hw-handle-image--queried); background-size: var(--hw-handle-width--queried); background-repeat: no-repeat; - cursor: pointer; margin-left: var(--hw-padding--slimmer); min-height: var(--hw-handle-width--queried); min-width: var(--hw-handle-width--queried); diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index 1726f7d..d99a05e 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -73,6 +73,23 @@ def main_wrapper_attrs end + def announcer_attrs + { + style: " + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + clip: rect(0, 0, 0, 0); + border: 0;".squish, + aria: announcer_aria, + data: announcer_data + } + end + + def input_attrs nested_attrs = %i[ data aria ] @@ -242,6 +259,18 @@ def main_wrapper_data end + def announcer_aria + { + live: :polite, + atomic: true + } + end + + def announcer_data + { hw_combobox_target: "announcer" } + end + + def hidden_field_id "#{canonical_id}-hw-hidden-field" end diff --git a/app/views/hotwire_combobox/_component.html.erb b/app/views/hotwire_combobox/_component.html.erb index 07c9c8c..1b1daed 100644 --- a/app/views/hotwire_combobox/_component.html.erb +++ b/app/views/hotwire_combobox/_component.html.erb @@ -4,6 +4,7 @@ <%= render "hotwire_combobox/combobox/hidden_field", component: component %> <%= tag.div **component.main_wrapper_attrs do %> + <%= tag.div **component.announcer_attrs %> <%= render "hotwire_combobox/combobox/input", component: component %> <%= render "hotwire_combobox/combobox/paginated_listbox", component: component %> <%= render "hotwire_combobox/combobox/dialog", component: component %> diff --git a/test/dummy/app/views/comboboxes/multiple.html.erb b/test/dummy/app/views/comboboxes/multiple.html.erb index a8df8f8..2385570 100644 --- a/test/dummy/app/views/comboboxes/multiple.html.erb +++ b/test/dummy/app/views/comboboxes/multiple.html.erb @@ -1,4 +1,5 @@ <%= combobox_tag "state", State.all, id: "state-field", + label: "States", multiselect_chip_src: new_state_chip_path, - placeholder: "select states" %> + placeholder: "Select states" %> From 5f6bf902aa3809f6259d45ac909590a3f1d8f447 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sun, 17 Mar 2024 12:10:35 -0600 Subject: [PATCH 13/32] Add location to State and include all 50 fixtures --- .../20240317175203_add_location_to_states.rb | 5 + test/dummy/app/models/state.rb | 2 + test/dummy/db/schema.rb | 3 +- test/fixtures/states.yml | 250 +++++++++++++++++- test/fixtures/users.yml | 4 +- test/system/hotwire_combobox_test.rb | 6 +- 6 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20240317175203_add_location_to_states.rb diff --git a/db/migrate/20240317175203_add_location_to_states.rb b/db/migrate/20240317175203_add_location_to_states.rb new file mode 100644 index 0000000..41f5d56 --- /dev/null +++ b/db/migrate/20240317175203_add_location_to_states.rb @@ -0,0 +1,5 @@ +class AddLocationToStates < ActiveRecord::Migration[7.1] + def change + add_column :states, :location, :integer + end +end diff --git a/test/dummy/app/models/state.rb b/test/dummy/app/models/state.rb index 8f246bd..cb9962e 100644 --- a/test/dummy/app/models/state.rb +++ b/test/dummy/app/models/state.rb @@ -1,6 +1,8 @@ class State < ApplicationRecord default_scope { alphabetically } + enum :location, %i[ South West Northeast Midwest ] + scope :alphabetically, -> { order(:name) } def to_combobox_display diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 300054e..da2d6ff 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_02_20_212536) do +ActiveRecord::Schema[7.1].define(version: 2024_03_17_175203) do create_table "movies", force: :cascade do |t| t.string "title", null: false t.datetime "created_at", null: false @@ -23,6 +23,7 @@ t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "location" end create_table "users", force: :cascade do |t| diff --git a/test/fixtures/states.yml b/test/fixtures/states.yml index 71ebdf2..62496d8 100644 --- a/test/fixtures/states.yml +++ b/test/fixtures/states.yml @@ -1,23 +1,249 @@ -al: - abbreviation: AL +alabama: name: Alabama + abbreviation: AL + location: South -fl: - abbreviation: FL +alaska: + name: Alaska + abbreviation: AK + location: West + +arizona: + name: Arizona + abbreviation: AZ + location: West + +arkansas: + name: Arkansas + abbreviation: AR + location: South + +california: + name: California + abbreviation: CA + location: West + +colorado: + name: Colorado + abbreviation: CO + location: West + +connecticut: + name: Connecticut + abbreviation: CT + location: Northeast + +delaware: + name: Delaware + abbreviation: DE + location: South + +florida: name: Florida + abbreviation: FL + location: South -mi: - abbreviation: MI +georgia: + name: Georgia + abbreviation: GA + location: South + +hawaii: + name: Hawaii + abbreviation: HI + location: West + +idaho: + name: Idaho + abbreviation: ID + location: West + +illinois: + name: Illinois + abbreviation: IL + location: Midwest + +indiana: + name: Indiana + abbreviation: IN + location: Midwest + +iowa: + name: Iowa + abbreviation: IA + location: Midwest + +kansas: + name: Kansas + abbreviation: KS + location: Midwest + +kentucky: + name: Kentucky + abbreviation: KY + location: South + +louisiana: + name: Louisiana + abbreviation: LA + location: South + +maine: + name: Maine + abbreviation: ME + location: Northeast + +maryland: + name: Maryland + abbreviation: MD + location: South + +massachusetts: + name: Massachusetts + abbreviation: MA + location: Northeast + +michigan: name: Michigan + abbreviation: MI + location: Midwest -mn: - abbreviation: MN +minnesota: name: Minnesota + abbreviation: MN + location: Midwest -ms: - abbreviation: MS +mississippi: name: Mississippi + abbreviation: MS + location: South -mo: - abbreviation: MO +missouri: name: Missouri + abbreviation: MO + location: Midwest + +montana: + name: Montana + abbreviation: MT + location: West + +nebraska: + name: Nebraska + abbreviation: NE + location: Midwest + +nevada: + name: Nevada + abbreviation: NV + location: West + +new_hampshire: + name: New Hampshire + abbreviation: NH + location: Northeast + +new_jersey: + name: New Jersey + abbreviation: NJ + location: Northeast + +new_mexico: + name: New Mexico + abbreviation: NM + location: West + +new_york: + name: New York + abbreviation: NY + location: Northeast + +north_carolina: + name: North Carolina + abbreviation: NC + location: South + +north_dakota: + name: North Dakota + abbreviation: ND + location: Midwest + +ohio: + name: Ohio + abbreviation: OH + location: Midwest + +oklahoma: + name: Oklahoma + abbreviation: OK + location: South + +oregon: + name: Oregon + abbreviation: OR + location: West + +pennsylvania: + name: Pennsylvania + abbreviation: PA + location: Northeast + +rhode_island: + name: Rhode Island + abbreviation: RI + location: Northeast + +south_carolina: + name: South Carolina + abbreviation: SC + location: South + +south_dakota: + name: South Dakota + abbreviation: SD + location: Midwest + +tennessee: + name: Tennessee + abbreviation: TN + location: South + +texas: + name: Texas + abbreviation: TX + location: South + +utah: + name: Utah + abbreviation: UT + location: West + +vermont: + name: Vermont + abbreviation: VT + location: Northeast + +virginia: + name: Virginia + abbreviation: VA + location: South + +washington: + name: Washington + abbreviation: WA + location: West + +west_virginia: + name: West Virginia + abbreviation: WV + location: South + +wisconsin: + name: Wisconsin + abbreviation: WI + location: Midwest + +wyoming: + name: Wyoming + abbreviation: WY + location: West diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index f7e9242..b555655 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,3 +1,3 @@ jose: - favorite_state: mi - home_state: fl + favorite_state: michigan + home_state: florida diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 151f2c3..554b82c 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -268,7 +268,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase visit prefilled_form_path assert_closed_combobox - assert_combobox_display_and_value "#user_favorite_state_id", "Michigan", states(:mi).id + assert_combobox_display_and_value "#user_favorite_state_id", "Michigan", states(:michigan).id open_combobox "#user_favorite_state_id" assert_selected_option_with text: "Michigan" @@ -288,7 +288,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase visit prefilled_async_path assert_closed_combobox - assert_combobox_display_and_value "#user_home_state_id", "Florida", states(:fl).id + assert_combobox_display_and_value "#user_home_state_id", "Florida", states(:florida).id open_combobox "#user_home_state_id" assert_selected_option_with text: "Florida" @@ -346,7 +346,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "#allow-new" type_in_combobox "#allow-new", "Ala" - assert_combobox_display_and_value "#allow-new", "Alabama", states(:al).id + assert_combobox_display_and_value "#allow-new", "Alabama", states(:alabama).id assert_selected_option_with text: "Alabama" assert_proper_combobox_name_choice \ original: "user[favorite_state_id]", From 160d2421140b690920286594501aa549e6b98632 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sun, 17 Mar 2024 12:28:37 -0600 Subject: [PATCH 14/32] Create Visit --- test/dummy/app/models/state.rb | 3 +++ test/dummy/app/models/user.rb | 3 +++ test/dummy/app/models/visit.rb | 4 ++++ .../20240317175203_add_location_to_states.rb | 0 .../db/migrate/20240317181056_create_visits.rb | 10 ++++++++++ test/dummy/db/schema.rb | 13 ++++++++++++- test/fixtures/visits.yml | 7 +++++++ 7 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/dummy/app/models/visit.rb rename {db => test/dummy/db}/migrate/20240317175203_add_location_to_states.rb (100%) create mode 100644 test/dummy/db/migrate/20240317181056_create_visits.rb create mode 100644 test/fixtures/visits.yml diff --git a/test/dummy/app/models/state.rb b/test/dummy/app/models/state.rb index cb9962e..f96787c 100644 --- a/test/dummy/app/models/state.rb +++ b/test/dummy/app/models/state.rb @@ -1,6 +1,9 @@ class State < ApplicationRecord default_scope { alphabetically } + has_many :visits + has_many :visitors, through: :visits, source: :user + enum :location, %i[ South West Northeast Midwest ] scope :alphabetically, -> { order(:name) } diff --git a/test/dummy/app/models/user.rb b/test/dummy/app/models/user.rb index ed98364..3aa8a2b 100644 --- a/test/dummy/app/models/user.rb +++ b/test/dummy/app/models/user.rb @@ -2,5 +2,8 @@ class User < ApplicationRecord belongs_to :favorite_state, class_name: :State, optional: true belongs_to :home_state, class_name: :State, optional: true + has_many :visits + has_many :visited_states, through: :visits, source: :state + accepts_nested_attributes_for :favorite_state, :home_state end diff --git a/test/dummy/app/models/visit.rb b/test/dummy/app/models/visit.rb new file mode 100644 index 0000000..8ca7c09 --- /dev/null +++ b/test/dummy/app/models/visit.rb @@ -0,0 +1,4 @@ +class Visit < ApplicationRecord + belongs_to :user + belongs_to :state +end diff --git a/db/migrate/20240317175203_add_location_to_states.rb b/test/dummy/db/migrate/20240317175203_add_location_to_states.rb similarity index 100% rename from db/migrate/20240317175203_add_location_to_states.rb rename to test/dummy/db/migrate/20240317175203_add_location_to_states.rb diff --git a/test/dummy/db/migrate/20240317181056_create_visits.rb b/test/dummy/db/migrate/20240317181056_create_visits.rb new file mode 100644 index 0000000..02874ce --- /dev/null +++ b/test/dummy/db/migrate/20240317181056_create_visits.rb @@ -0,0 +1,10 @@ +class CreateVisits < ActiveRecord::Migration[7.1] + def change + create_table :visits do |t| + t.references :user, null: false, foreign_key: true + t.references :state, null: false, foreign_key: true + + t.timestamps + end + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index da2d6ff..3fd383c 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_03_17_175203) do +ActiveRecord::Schema[7.1].define(version: 2024_03_17_181056) do create_table "movies", force: :cascade do |t| t.string "title", null: false t.datetime "created_at", null: false @@ -35,6 +35,17 @@ t.index ["home_state_id"], name: "index_users_on_home_state_id" end + create_table "visits", force: :cascade do |t| + t.integer "user_id", null: false + t.integer "state_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["state_id"], name: "index_visits_on_state_id" + t.index ["user_id"], name: "index_visits_on_user_id" + end + add_foreign_key "users", "states", column: "favorite_state_id" add_foreign_key "users", "states", column: "home_state_id" + add_foreign_key "visits", "states" + add_foreign_key "visits", "users" end diff --git a/test/fixtures/visits.yml b/test/fixtures/visits.yml new file mode 100644 index 0000000..050ee1a --- /dev/null +++ b/test/fixtures/visits.yml @@ -0,0 +1,7 @@ +florida: + user: jose + state: florida + +illinois: + user: jose + state: illinois From fec7817fe05bd64f61c0fa8a2d98f9df0835cc4d Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sun, 17 Mar 2024 12:29:56 -0600 Subject: [PATCH 15/32] multiple -> multiselect --- test/dummy/app/controllers/comboboxes_controller.rb | 9 ++++++--- .../{multiple.html.erb => multiselect.html.erb} | 0 .../app/views/comboboxes/multiselect_async_html.html.erb | 0 ...vents.html.erb => multiselect_custom_events.html.erb} | 0 ...lled.html.erb => multiselect_prefilled_form.html.erb} | 0 test/dummy/config/routes.rb | 7 ++++--- 6 files changed, 10 insertions(+), 6 deletions(-) rename test/dummy/app/views/comboboxes/{multiple.html.erb => multiselect.html.erb} (100%) create mode 100644 test/dummy/app/views/comboboxes/multiselect_async_html.html.erb rename test/dummy/app/views/comboboxes/{multiple_custom_events.html.erb => multiselect_custom_events.html.erb} (100%) rename test/dummy/app/views/comboboxes/{multiple_prefilled.html.erb => multiselect_prefilled_form.html.erb} (100%) diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index 2ccb180..a1f9a16 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -70,13 +70,16 @@ def render_in_locals @hashes = State.limit(3).map { |state| { display: state.name, value: state.abbreviation } } end - def multiple + def multiselect end - def multiple_prefilled + def multiselect_async_html end - def multiple_custom_events + def multiselect_prefilled_form + end + + def multiselect_custom_events end private diff --git a/test/dummy/app/views/comboboxes/multiple.html.erb b/test/dummy/app/views/comboboxes/multiselect.html.erb similarity index 100% rename from test/dummy/app/views/comboboxes/multiple.html.erb rename to test/dummy/app/views/comboboxes/multiselect.html.erb diff --git a/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb b/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb new file mode 100644 index 0000000..e69de29 diff --git a/test/dummy/app/views/comboboxes/multiple_custom_events.html.erb b/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb similarity index 100% rename from test/dummy/app/views/comboboxes/multiple_custom_events.html.erb rename to test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb diff --git a/test/dummy/app/views/comboboxes/multiple_prefilled.html.erb b/test/dummy/app/views/comboboxes/multiselect_prefilled_form.html.erb similarity index 100% rename from test/dummy/app/views/comboboxes/multiple_prefilled.html.erb rename to test/dummy/app/views/comboboxes/multiselect_prefilled_form.html.erb diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 56db049..45ff0c6 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -21,9 +21,10 @@ get "custom_attrs", to: "comboboxes#custom_attrs" get "conflicting_order", to: "comboboxes#conflicting_order" get "render_in_locals", to: "comboboxes#render_in_locals" - get "multiple", to: "comboboxes#multiple" - get "multiple_prefilled", to: "comboboxes#multiple_prefilled" - get "multiple_custom_events", to: "comboboxes#multiple_custom_events" + get "multiselect", to: "comboboxes#multiselect" + get "multiselect_async_html", to: "comboboxes#multiselect_async_html" + get "multiselect_prefilled_form", to: "comboboxes#multiselect_prefilled_form" + get "multiselect_custom_events", to: "comboboxes#multiselect_custom_events" resources :movies, only: %i[ index update ] get "movies_html", to: "movies#index_html" From 5780bcf6693bee37187ad97650e9ffc05a338942 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Sun, 17 Mar 2024 16:47:38 -0600 Subject: [PATCH 16/32] Implement multi-preselection --- .../controllers/hw_combobox_controller.js | 3 +- .../models/combobox/multiselect.js | 25 ++++++++++------- .../hw_combobox/models/combobox/options.js | 4 +++ .../hw_combobox/models/combobox/selection.js | 28 +++++++++++++------ .../hw_combobox/models/combobox/toggle.js | 2 +- app/presenters/hotwire_combobox/component.rb | 18 +++++++----- ...l.erb => _selection_chip.turbo_stream.erb} | 0 lib/hotwire_combobox/helper.rb | 6 ++-- .../app/controllers/comboboxes_controller.rb | 1 + .../app/controllers/state_chips_controller.rb | 2 +- .../dummy/app/controllers/users_controller.rb | 8 ++++-- .../multiselect_prefilled_form.html.erb | 14 ++++------ .../views/state_chips/new.turbo_stream.erb | 4 ++- 13 files changed, 72 insertions(+), 43 deletions(-) rename app/views/hotwire_combobox/{_selection_chip.html.erb => _selection_chip.turbo_stream.erb} (100%) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 0e3e955..020a33e 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -67,6 +67,7 @@ export default class HwComboboxController extends Concerns(...concerns) { this._connectSelection() this._connectListAutocomplete() this._connectDialog() + this._connectMultiselect() } disconnect() { @@ -91,7 +92,7 @@ export default class HwComboboxController extends Concerns(...concerns) { if (delay) await sleep(delay) this._selectOnQuery(inputType) } else { - this._preselect() + this._preselectSingle() } } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index b703a44..b3450d8 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -44,28 +44,33 @@ Combobox.Multiselect = Base => class extends Base { } } + _connectMultiselect() { + this._preselectMultiple() + } + _createChip() { if (!this._isMultiselect) return this._beforeClearingMultiselectQuery(async (display, value) => { this._fullQuery = "" this._filter("hw:multiselectSync") - - await get(this.selectionChipSrcValue, { - responseKind: "turbo-stream", - query: { - for_id: this.element.dataset.asyncId, - combobox_value: value, - display_value: display - } - }) - + this._requestChips(value) this._addToFieldValue(value) this.openByFocusing() this._announceToScreenReader(display, "multi-selected. Press Shift + Tab, then Enter to remove.") }) } + async _requestChips(values) { + await get(this.selectionChipSrcValue, { + responseKind: "turbo-stream", + query: { + for_id: this.element.dataset.asyncId, + combobox_values: values + } + }) + } + _beforeClearingMultiselectQuery(callback) { const display = this.hiddenFieldTarget.dataset.displayForMultiselect const value = this.hiddenFieldTarget.dataset.valueForMultiselect diff --git a/app/assets/javascripts/hw_combobox/models/combobox/options.js b/app/assets/javascripts/hw_combobox/models/combobox/options.js index 514fff9..176ecda 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/options.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/options.js @@ -39,6 +39,10 @@ Combobox.Options = Base => class extends Base { return this._actingListbox.querySelector("[role=option][aria-selected=true]:not([data-multiselected])") } + get _multiselectedOptionElements() { + return this._actingListbox.querySelectorAll("[role=option][data-multiselected]") + } + get _selectedOptionIndex() { return [ ...this._visibleOptionElements ].indexOf(this._selectedOptionElement) } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index 2e7e949..d72b0ad 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -83,18 +83,20 @@ Combobox.Selection = Base => class extends Base { this._forceSelectionWithoutFiltering(option) } - _preselect() { - // TODO: Implement preselection in multiselect, consider associations in form builders - - if (this._hasValueButNoSelection && this._allOptions.length < 100) { - const option = this._allOptions.find(option => { - return option.dataset.value === this._fieldValue - }) - + _preselectSingle() { + if (this._isSingleSelect && this._hasValueButNoSelection && this._allOptions.length < 100) { + const option = this._optionElementWithValue(this._fieldValue) if (option) this._markSelected(option) } } + _preselectMultiple() { + if (this._isMultiselect && this._hasValueButNoSelection) { + this._requestChips(this._fieldValueString) + this._resetMultiselectionMarks() + } + } + _selectAndAutocompleteMissingPortion(option) { this._select(option, this._autocompleteMissingPortion.bind(this)) } @@ -139,7 +141,15 @@ Combobox.Selection = Base => class extends Base { } get _hasValueButNoSelection() { - return this._hasFieldValue && !this._selectedOptionElement + return this._hasFieldValue && !this._hasSelection + } + + get _hasSelection() { + if (this._isSingleSelect) { + this._selectedOptionElement + } else { + this._multiselectedOptionElements.length > 0 + } } get _shouldLockInSelection() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index 33685e6..165d47c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -83,7 +83,7 @@ Combobox.Toggle = Base => class extends Base { _expand() { if (this._isSync) { - this._preselect() + this._preselectSingle() } if (this._autocompletesList && this._isSmallViewport) { diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index d99a05e..b0b892b 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -40,7 +40,7 @@ def render_in(view_context, &block) def fieldset_attrs apply_customizations_to :fieldset, base: { - class: [ "hw-combobox", { "hw-combobox--multiple": multiple? } ], + class: [ "hw-combobox", { "hw-combobox--multiple": multiselect? } ], data: fieldset_data } end @@ -195,12 +195,12 @@ def pagination_attrs :name_when_new, :open, :data, :combobox_attrs, :mobile_at, :association_name, :multiselect_chip_src - def multiple? + def multiselect? multiselect_chip_src.present? end def infer_association_name - if name.include?("_id") + if name.end_with?("_id") name.sub(/_id\z/, "") end end @@ -224,6 +224,8 @@ def fieldset_data end def prefilled_display + return if multiselect? + if async_src && associated_object associated_object.to_combobox_display elsif hidden_field_value @@ -253,7 +255,7 @@ def canonical_id def main_wrapper_data { - action: ("click->hw-combobox#openByFocusing:self" if multiple?), + action: ("click->hw-combobox#openByFocusing:self" if multiselect?), hw_combobox_target: "mainWrapper" } end @@ -289,7 +291,9 @@ def hidden_field_value if form&.object&.defined_enums&.try :[], name form.object.public_send "#{name}_before_type_cast" else - form&.object&.try name + form&.object&.try(name).then do |value| + value.respond_to?(:map) ? value.join(",") : value + end end end @@ -342,7 +346,7 @@ def listbox_data end def listbox_aria - { multiselectable: multiple? } + { multiselectable: multiselect? } end @@ -385,7 +389,7 @@ def dialog_listbox_data end def dialog_listbox_aria - { multiselectable: multiple? } + { multiselectable: multiselect? } end def dialog_focus_trap_data diff --git a/app/views/hotwire_combobox/_selection_chip.html.erb b/app/views/hotwire_combobox/_selection_chip.turbo_stream.erb similarity index 100% rename from app/views/hotwire_combobox/_selection_chip.html.erb rename to app/views/hotwire_combobox/_selection_chip.turbo_stream.erb diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index e309d7e..f9ebf14 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -15,7 +15,7 @@ def hw_combobox_style_tag(*args, **kwargs) end def hw_combobox_tag(name, options_or_src = [], render_in: {}, include_blank: nil, **kwargs, &block) - options, src = hw_extract_options_and_src(options_or_src, render_in, include_blank) + options, src = hw_extract_options_and_src options_or_src, render_in, include_blank component = HotwireCombobox::Component.new self, name, options: options, async_src: src, **kwargs render component, &block end @@ -40,7 +40,7 @@ def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request end alias_method :hw_async_combobox_options, :hw_paginated_combobox_options - def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id], remover_attrs: hw_combobox_chip_remover_attrs(display, value)) + def hw_combobox_selection_chip(display:, value:, for_id: params[:for_id], remover_attrs: hw_combobox_chip_remover_attrs(display, value)) render "hotwire_combobox/selection_chip", display: display, value: value, @@ -48,7 +48,7 @@ def hw_combobox_selection_chip(display:, value: params[:combobox_value], for_id: remover_attrs: remover_attrs end - def hw_dismissing_combobox_selection_chip(display:, value: params[:combobox_value], for_id: params[:for_id]) + def hw_dismissing_combobox_selection_chip(display:, value:, for_id: params[:for_id]) hw_combobox_selection_chip \ display: display, value: value, diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index a1f9a16..1ec7d87 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -77,6 +77,7 @@ def multiselect_async_html end def multiselect_prefilled_form + @user = User.first || raise("No user found, load fixtures first.") end def multiselect_custom_events diff --git a/test/dummy/app/controllers/state_chips_controller.rb b/test/dummy/app/controllers/state_chips_controller.rb index d0af961..365ebc6 100644 --- a/test/dummy/app/controllers/state_chips_controller.rb +++ b/test/dummy/app/controllers/state_chips_controller.rb @@ -1,5 +1,5 @@ class StateChipsController < ApplicationController def new - @state = State.find params[:combobox_value] + @states = State.find params[:combobox_values].split(",") end end diff --git a/test/dummy/app/controllers/users_controller.rb b/test/dummy/app/controllers/users_controller.rb index f428407..83a51c3 100644 --- a/test/dummy/app/controllers/users_controller.rb +++ b/test/dummy/app/controllers/users_controller.rb @@ -2,8 +2,8 @@ class UsersController < ApplicationController before_action :set_user def update - if @user.update user_params - redirect_to prefilled_async_path, notice: "User updated" + if @user.update user_params.merge(visited_state_ids: visited_state_ids) + redirect_back_or_to prefilled_async_path, notice: "User updated" else head :unprocessable_entity end @@ -23,4 +23,8 @@ def user_params favorite_state_attributes: %i[ name ], home_state_attributes: %i[ name ]) end + + def visited_state_ids + params[:user][:visited_state_ids].split(",") + end end diff --git a/test/dummy/app/views/comboboxes/multiselect_prefilled_form.html.erb b/test/dummy/app/views/comboboxes/multiselect_prefilled_form.html.erb index e3e6770..c91f13b 100644 --- a/test/dummy/app/views/comboboxes/multiselect_prefilled_form.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_prefilled_form.html.erb @@ -1,10 +1,8 @@ - +<% if flash[:notice] %> +

<%= flash[:notice] %>

+<% end %> -<%= combobox_tag "state", @state_options, id: "state-field", multiple: true, value: ['FL', 'MO'] do |combobox| %> - <% combobox.customize_main_wrapper class: "wide-multiple-field" %> +<%= form_with model: @user, html: { style: "display: flex; flex-direction: column; gap: 1rem;" } do |form| %> + <%= form.combobox :visited_state_ids, State.all, label: "Visited states", multiselect_chip_src: new_state_chip_path %> + <%= form.submit style: "padding: 0.5rem;" %> <% end %> diff --git a/test/dummy/app/views/state_chips/new.turbo_stream.erb b/test/dummy/app/views/state_chips/new.turbo_stream.erb index 5f771f5..9ccb1df 100644 --- a/test/dummy/app/views/state_chips/new.turbo_stream.erb +++ b/test/dummy/app/views/state_chips/new.turbo_stream.erb @@ -1 +1,3 @@ -<%= combobox_selection_chip display: @state.name %> +<% @states.each do |state| %> + <%= combobox_selection_chip display: state.name, value: state.id %> +<% end %> From d9c1e1fc3522faa6f149e5f07527cfaa645c541f Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 13:39:55 -0600 Subject: [PATCH 17/32] Pull out selection chip layout --- app/assets/stylesheets/hotwire_combobox.css | 42 +++++++++---------- .../_selection_chip.turbo_stream.erb | 4 +- .../layouts/_selection_chip.turbo_stream.erb | 7 ++++ lib/hotwire_combobox/helper.rb | 13 ++++-- test/system/hotwire_combobox_test.rb | 6 +-- 5 files changed, 42 insertions(+), 30 deletions(-) create mode 100644 app/views/hotwire_combobox/layouts/_selection_chip.turbo_stream.erb diff --git a/app/assets/stylesheets/hotwire_combobox.css b/app/assets/stylesheets/hotwire_combobox.css index c022d4c..4721788 100644 --- a/app/assets/stylesheets/hotwire_combobox.css +++ b/app/assets/stylesheets/hotwire_combobox.css @@ -227,6 +227,26 @@ } } +.hw-combobox__chip { + align-items: center; + border: var(--hw-border-width--slim) solid var(--hw-border-color); + border-radius: var(--hw-border-radius); + display: flex; + font-size: var(--hw-selection-chip-font-size); + line-height: var(--hw-line-height); + padding: var(--hw-padding--slimmer); + padding-left: var(--hw-padding--slim); +} + +.hw-combobox__chip__remover { + background-image: var(--hw-handle-image--queried); + background-size: var(--hw-handle-width--queried); + background-repeat: no-repeat; + margin-left: var(--hw-padding--slimmer); + min-height: var(--hw-handle-width--queried); + min-width: var(--hw-handle-width--queried); +} + .hw-combobox--multiple { .hw-combobox__main__wrapper { align-items: center; @@ -235,31 +255,11 @@ gap: var(--hw-padding--slimmer); width: var(--hw-combobox-width--multiple); - &:has(.hw-combobox__chip) .hw-combobox__input::placeholder { + &:has([data-hw-combobox-chip]) .hw-combobox__input::placeholder { color: transparent; } } - .hw-combobox__chip { - align-items: center; - border: var(--hw-border-width--slim) solid var(--hw-border-color); - border-radius: var(--hw-border-radius); - display: flex; - font-size: var(--hw-selection-chip-font-size); - line-height: var(--hw-line-height); - padding: var(--hw-padding--slimmer); - padding-left: var(--hw-padding--slim); - } - - .hw-combobox__chip__dismisser { - background-image: var(--hw-handle-image--queried); - background-size: var(--hw-handle-width--queried); - background-repeat: no-repeat; - margin-left: var(--hw-padding--slimmer); - min-height: var(--hw-handle-width--queried); - min-width: var(--hw-handle-width--queried); - } - .hw-combobox__input { min-width: calc(var(--hw-combobox-width) / 2); flex-grow: 1; diff --git a/app/views/hotwire_combobox/_selection_chip.turbo_stream.erb b/app/views/hotwire_combobox/_selection_chip.turbo_stream.erb index cbfe3ea..babeabd 100644 --- a/app/views/hotwire_combobox/_selection_chip.turbo_stream.erb +++ b/app/views/hotwire_combobox/_selection_chip.turbo_stream.erb @@ -1,7 +1,7 @@ <%# locals: (display:, value:, for_id:, remover_attrs:) -%> -<%= turbo_stream.before for_id do %> - <%= tag.div class: "hw-combobox__chip", data: { hw_combobox_chip: "" } do %> +<%= within_combobox_selection_chip(for_id: for_id) do %> + <%= tag.div class: "hw-combobox__chip" do %> <%= tag.span display %> <%= tag.span **remover_attrs %> <% end %> diff --git a/app/views/hotwire_combobox/layouts/_selection_chip.turbo_stream.erb b/app/views/hotwire_combobox/layouts/_selection_chip.turbo_stream.erb new file mode 100644 index 0000000..59cdb13 --- /dev/null +++ b/app/views/hotwire_combobox/layouts/_selection_chip.turbo_stream.erb @@ -0,0 +1,7 @@ +<%# locals: (for_id:) -%> + +<%= turbo_stream.before for_id do %> + <%= tag.div data: { hw_combobox_chip: "" } do %> + <%= yield %> + <% end %> +<% end %> diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index f9ebf14..1f6be8d 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -40,7 +40,11 @@ def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request end alias_method :hw_async_combobox_options, :hw_paginated_combobox_options - def hw_combobox_selection_chip(display:, value:, for_id: params[:for_id], remover_attrs: hw_combobox_chip_remover_attrs(display, value)) + def hw_within_combobox_selection_chip(for_id: params[:for_id], &block) + render layout: "hotwire_combobox/layouts/selection_chip", locals: { for_id: for_id }, &block + end + + def hw_combobox_selection_chip(display:, value:, for_id: params[:for_id], remover_attrs: hw_combobox_chip_remover_attrs(display: display, value: value)) render "hotwire_combobox/selection_chip", display: display, value: value, @@ -56,10 +60,10 @@ def hw_dismissing_combobox_selection_chip(display:, value:, for_id: params[:for_ remover_attrs: hw_combobox_dismissing_chip_remover_attrs(display, value) end - def hw_combobox_chip_remover_attrs(display, value) + def hw_combobox_chip_remover_attrs(display:, value:, **kwargs) { tabindex: "0", - class: "hw-combobox__chip__dismisser", + class: token_list("hw-combobox__chip__remover", kwargs[:class]), aria: { label: "Remove #{display}" }, data: { action: "click->hw-combobox#removeChip:stop keydown->hw-combobox#navigateChip", @@ -70,7 +74,7 @@ def hw_combobox_chip_remover_attrs(display, value) end def hw_combobox_dismissing_chip_remover_attrs(display, value) - hw_combobox_chip_remover_attrs(display, value).tap do |attrs| + hw_combobox_chip_remover_attrs(display: display, value: value).tap do |attrs| attrs[:data][:hw_combobox_target] = token_list(attrs[:data][:hw_combobox_target], "closer") end end @@ -80,6 +84,7 @@ def hw_combobox_dismissing_chip_remover_attrs(display, value) hw_alias :hw_combobox_options hw_alias :hw_paginated_combobox_options hw_alias :hw_async_combobox_options + hw_alias :hw_within_combobox_selection_chip hw_alias :hw_combobox_selection_chip hw_alias :hw_dismissing_combobox_selection_chip hw_alias :hw_combobox_chip_remover_attrs diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 554b82c..b554c08 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -799,7 +799,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase click_on_option "Alabama" assert_open_combobox assert_no_selector "input[placeholder='State']" - find("#AL .hw-combobox__chip__dismisser").click + find("#AL .hw-combobox__chip__remover").click assert_selector "input[placeholder='State']" end @@ -821,7 +821,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase type_in_combobox "#state-field", :enter assert_open_combobox assert_selector "div[id='AL'] div", text: "Alabama" - find("#AL .hw-combobox__chip__dismisser").click + find("#AL .hw-combobox__chip__remover").click assert_no_selector "div[id='AL'] div", text: "Alabama" click_on_option "Minnesota" @@ -848,7 +848,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_text "selections: 1." assert_no_text "event: hw-combobox:closed" - find("#MO .hw-combobox__chip__dismisser").click + find("#MO .hw-combobox__chip__remover").click assert_text 'value: ["FL","MN"]' assert_text "selections: 2." assert_no_text "event: hw-combobox:closed" From 3e7aec10a173a30d46537b9b2c40d6eac7adc6f3 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 13:40:50 -0600 Subject: [PATCH 18/32] Add multiselect_dismissing action --- test/dummy/app/controllers/comboboxes_controller.rb | 3 +++ .../dummy/app/views/comboboxes/multiselect_dismissing.html.erb | 0 test/dummy/config/routes.rb | 1 + 3 files changed, 4 insertions(+) create mode 100644 test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index 1ec7d87..db83a09 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -73,6 +73,9 @@ def render_in_locals def multiselect end + def multiselect_dismissing + end + def multiselect_async_html end diff --git a/test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb b/test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb new file mode 100644 index 0000000..e69de29 diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 45ff0c6..104be39 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -22,6 +22,7 @@ get "conflicting_order", to: "comboboxes#conflicting_order" get "render_in_locals", to: "comboboxes#render_in_locals" get "multiselect", to: "comboboxes#multiselect" + get "multiselect_dismissing", to: "comboboxes#multiselect_dismissing" get "multiselect_async_html", to: "comboboxes#multiselect_async_html" get "multiselect_prefilled_form", to: "comboboxes#multiselect_prefilled_form" get "multiselect_custom_events", to: "comboboxes#multiselect_custom_events" From 575f1a146998af14bcd3e2fe0b572b62cdf29792 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 13:42:01 -0600 Subject: [PATCH 19/32] Make dummy state comboboxes searchable --- .../javascripts/hw_combobox/models/combobox/selection.js | 1 + test/dummy/app/controllers/states_controller.rb | 1 + test/dummy/app/models/state.rb | 1 + test/dummy/app/views/states/_state.html.erb | 2 +- test/dummy/app/views/states/index.turbo_stream.erb | 8 +------- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index d72b0ad..cf7b3df 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -51,6 +51,7 @@ Combobox.Selection = Base => class extends Base { } _selectNew() { + // TODO: multiselect const previousValue = this._fieldValueString this._resetOptionsSilently() diff --git a/test/dummy/app/controllers/states_controller.rb b/test/dummy/app/controllers/states_controller.rb index 4b0d1c6..2a4b039 100644 --- a/test/dummy/app/controllers/states_controller.rb +++ b/test/dummy/app/controllers/states_controller.rb @@ -1,4 +1,5 @@ class StatesController < ApplicationController def index + @states = State.search params[:q] end end diff --git a/test/dummy/app/models/state.rb b/test/dummy/app/models/state.rb index f96787c..1093cf4 100644 --- a/test/dummy/app/models/state.rb +++ b/test/dummy/app/models/state.rb @@ -7,6 +7,7 @@ class State < ApplicationRecord enum :location, %i[ South West Northeast Midwest ] scope :alphabetically, -> { order(:name) } + scope :search, ->(q) { q.blank? ? all : where("name LIKE ?", "#{q}%") } def to_combobox_display name diff --git a/test/dummy/app/views/states/_state.html.erb b/test/dummy/app/views/states/_state.html.erb index 38d7005..90a725e 100644 --- a/test/dummy/app/views/states/_state.html.erb +++ b/test/dummy/app/views/states/_state.html.erb @@ -1,4 +1,4 @@
- +

<%= state.name %>

diff --git a/test/dummy/app/views/states/index.turbo_stream.erb b/test/dummy/app/views/states/index.turbo_stream.erb index 0121c8c..cac29c8 100644 --- a/test/dummy/app/views/states/index.turbo_stream.erb +++ b/test/dummy/app/views/states/index.turbo_stream.erb @@ -1,7 +1 @@ -<%= hw_async_combobox_options State.all, - render_in: { partial: "states/state", formats: [:html] }, - for_id: "user_favorite_state_id" %> - -<%= hw_async_combobox_options State.all, - render_in: { partial: "states/state", formats: [:html] }, - for_id: "user_home_state_id" %> +<%= hw_async_combobox_options @states, render_in: { partial: "states/state", formats: [:html] } %> From 38f79d5812c8b8752ce82f50f936c2125bf3d46e Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 14:15:18 -0600 Subject: [PATCH 20/32] Add async html example --- .../controllers/hw_combobox_controller.js | 4 +- .../models/combobox/multiselect.js | 9 ++-- .../app/controllers/state_chips_controller.rb | 11 +++- .../multiselect_async_html.html.erb | 51 +++++++++++++++++++ .../state_chips/new_html.turbo_stream.erb | 9 ++++ test/dummy/config/routes.rb | 1 + 6 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 test/dummy/app/views/state_chips/new_html.turbo_stream.erb diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 020a33e..6c50808 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -88,7 +88,9 @@ export default class HwComboboxController extends Concerns(...concerns) { this._resetMultiselectionMarks() - if (inputType && inputType !== "hw:lockInSelection") { + if (inputType === "hw:multiselectSync") { + this.openByFocusing() + } else if (inputType && inputType !== "hw:lockInSelection") { if (delay) await sleep(delay) this._selectOnQuery(inputType) } else { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index b3450d8..041ebf3 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -56,7 +56,7 @@ Combobox.Multiselect = Base => class extends Base { this._filter("hw:multiselectSync") this._requestChips(value) this._addToFieldValue(value) - this.openByFocusing() + if (this._isSync) this.openByFocusing() this._announceToScreenReader(display, "multi-selected. Press Shift + Tab, then Enter to remove.") }) } @@ -88,8 +88,11 @@ Combobox.Multiselect = Base => class extends Base { this._fieldValueArray.forEach(value => { const option = this._optionElementWithValue(value) - option.setAttribute("data-multiselected", "") - option.hidden = true + + if (option) { + option.setAttribute("data-multiselected", "") + option.hidden = true + } }) } diff --git a/test/dummy/app/controllers/state_chips_controller.rb b/test/dummy/app/controllers/state_chips_controller.rb index 365ebc6..24feb12 100644 --- a/test/dummy/app/controllers/state_chips_controller.rb +++ b/test/dummy/app/controllers/state_chips_controller.rb @@ -1,5 +1,14 @@ class StateChipsController < ApplicationController + before_action :set_states + def new - @states = State.find params[:combobox_values].split(",") end + + def new_html + end + + private + def set_states + @states = State.find params[:combobox_values].split(",") + end end diff --git a/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb b/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb index e69de29..0347d67 100644 --- a/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb @@ -0,0 +1,51 @@ + + +<%= combobox_tag "state", states_path, + id: "state-field", + label: "States", + multiselect_chip_src: new_html_state_chip_path, + value: State.limit(2).ids.join(","), + placeholder: "Select states" %> diff --git a/test/dummy/app/views/state_chips/new_html.turbo_stream.erb b/test/dummy/app/views/state_chips/new_html.turbo_stream.erb new file mode 100644 index 0000000..238d290 --- /dev/null +++ b/test/dummy/app/views/state_chips/new_html.turbo_stream.erb @@ -0,0 +1,9 @@ +<% @states.each do |state| %> + <%= within_combobox_selection_chip do %> + <%= tag.div class: "custom-chip" do %> + <%= tag.span class: "state state--#{state.name.downcase.parameterize}" %> + <%= tag.span state.name %> + <%= tag.span **combobox_chip_remover_attrs(display: state.name, value: state.id, class: "custom-remover") %> + <% end %> + <% end %> +<% end %> diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 104be39..974ca7f 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -38,6 +38,7 @@ resources :states, only: :index resources :state_chips, only: :new, param: :combobox_value + get "new_html_state_chip", to: "state_chips#new_html" resources :users, only: :update root to: "comboboxes#plain" From 3891db7aa6df4837cbff7a92f0d3bc8e4ce49613 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 14:27:06 -0600 Subject: [PATCH 21/32] Create new values dummy action --- .../javascripts/hw_combobox/models/combobox/selection.js | 1 - test/dummy/app/controllers/comboboxes_controller.rb | 3 +++ .../dummy/app/views/comboboxes/multiselect_new_values.html.erb | 0 test/dummy/config/routes.rb | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 test/dummy/app/views/comboboxes/multiselect_new_values.html.erb diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index cf7b3df..d72b0ad 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -51,7 +51,6 @@ Combobox.Selection = Base => class extends Base { } _selectNew() { - // TODO: multiselect const previousValue = this._fieldValueString this._resetOptionsSilently() diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index db83a09..6ac2a68 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -86,6 +86,9 @@ def multiselect_prefilled_form def multiselect_custom_events end + def multiselect_new_values + end + private delegate :combobox_options, :html_combobox_options, to: "ApplicationController.helpers", private: true diff --git a/test/dummy/app/views/comboboxes/multiselect_new_values.html.erb b/test/dummy/app/views/comboboxes/multiselect_new_values.html.erb new file mode 100644 index 0000000..e69de29 diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 974ca7f..9489178 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -26,6 +26,7 @@ get "multiselect_async_html", to: "comboboxes#multiselect_async_html" get "multiselect_prefilled_form", to: "comboboxes#multiselect_prefilled_form" get "multiselect_custom_events", to: "comboboxes#multiselect_custom_events" + get "multiselect_new_values", to: "comboboxes#multiselect_new_values" resources :movies, only: %i[ index update ] get "movies_html", to: "movies#index_html" From 31a1d5c293c82ea52b223d0042fbf8bdb7eda3a5 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 14:52:54 -0600 Subject: [PATCH 22/32] Get tests passing --- .../hw_combobox/models/combobox/options.js | 4 ++ .../hw_combobox/models/combobox/toggle.js | 5 +- .../component/customizable.rb | 10 +++- test/system/hotwire_combobox_test.rb | 47 +++++++++---------- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/hw_combobox/models/combobox/options.js b/app/assets/javascripts/hw_combobox/models/combobox/options.js index 176ecda..68c831f 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/options.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/options.js @@ -19,6 +19,10 @@ Combobox.Options = Base => class extends Base { return this._actingListbox.querySelector(`[${this.filterableAttributeValue}][data-value='${value}']`) } + _displayForOptionElement(element) { + return element.getAttribute(this.autocompletableAttributeValue) + } + get _allowNew() { return !!this.nameWhenNewValue } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index 165d47c..0d40bb0 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -20,9 +20,8 @@ Combobox.Toggle = Base => class extends Base { this._dispatchClosedEvent() this._createChip() - if (this._isSingleSelect) { - const display = this._selectedOptionElement.getAttribute(this.autocompletableAttributeValue) - this._announceToScreenReader(display, "selected") + if (this._isSingleSelect && this._selectedOptionElement) { + this._announceToScreenReader(this._displayForOptionElement(this._selectedOptionElement), "selected") } } } diff --git a/app/presenters/hotwire_combobox/component/customizable.rb b/app/presenters/hotwire_combobox/component/customizable.rb index 14013bf..0fb3e1a 100644 --- a/app/presenters/hotwire_combobox/component/customizable.rb +++ b/app/presenters/hotwire_combobox/component/customizable.rb @@ -44,7 +44,15 @@ def customize(element, **attrs) def apply_customizations_to(element, base: {}) custom = custom_attrs[element] - coalesce = ->(k, v) { v.is_a?(String) ? view.token_list(v, custom.delete(k)) : v } + + coalesce = ->(key, value) do + if value.is_a?(String) || value.is_a?(Array) + view.token_list(value, custom.delete(key)) + else + value + end + end + default = base.map { |k, v| [ k, coalesce.(k, v) ] }.to_h custom.deep_merge default diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index b554c08..27fbd2d 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -104,8 +104,8 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "#state-field" type_in_combobox "#state-field", "lor" - assert_combobox_display_and_value "#state-field", "lor", "FL" - assert_selected_option_with selector: ".hw-combobox__option--selected", text: "Florida" + assert_combobox_display_and_value "#state-field", "lor", "CO" + assert_selected_option_with selector: ".hw-combobox__option--selected", text: "Colorado" end test "selecting with the keyboard" do @@ -161,12 +161,12 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "#state-field" type_in_combobox "#state-field", "is", :enter assert_closed_combobox - assert_combobox_display_and_value "#state-field", "Mississippi", "MS" + assert_combobox_display_and_value "#state-field", "Illinois", "IL" assert_no_visible_selected_option # because the list is closed type_in_combobox "#state-field", :backspace assert_open_combobox - assert_combobox_display_and_value "#state-field", "Mississipp", nil + assert_combobox_display_and_value "#state-field", "Illinoi", nil assert_no_visible_selected_option end @@ -177,7 +177,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase type_in_combobox "#state-field", "is" click_away assert_closed_combobox - assert_combobox_display_and_value "#state-field", "Mississippi", "MS" + assert_combobox_display_and_value "#state-field", "Illinois", "IL" open_combobox "#state-field" assert_options_with count: 1 @@ -190,7 +190,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase type_in_combobox "#state-field", "is" tab_away assert_closed_combobox - assert_combobox_display_and_value "#state-field", "Mississippi", "MS" + assert_combobox_display_and_value "#state-field", "Illinois", "IL" open_combobox "#state-field" assert_options_with count: 1 @@ -205,27 +205,27 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_selected_option_with text: "Alabama" type_in_combobox "#state-field", :down - assert_selected_option_with text: "Florida" + assert_selected_option_with text: "Alaska" type_in_combobox "#state-field", :down - assert_selected_option_with text: "Michigan" + assert_selected_option_with text: "Arizona" type_in_combobox "#state-field", :up - assert_selected_option_with text: "Florida" + assert_selected_option_with text: "Alaska" type_in_combobox "#state-field", :up assert_selected_option_with text: "Alabama" # wrap around type_in_combobox "#state-field", :up - assert_selected_option_with text: "Missouri" + assert_selected_option_with text: "Wyoming" type_in_combobox "#state-field", :down assert_selected_option_with text: "Alabama" # home and end keys type_in_combobox "#state-field", :end - assert_selected_option_with text: "Missouri" + assert_selected_option_with text: "Wyoming" type_in_combobox "#state-field", :home assert_selected_option_with text: "Alabama" @@ -237,21 +237,21 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "#state-field" assert_combobox_display_and_value "#state-field", nil, nil - click_on_option "Florida" + click_on_option "Alaska" assert_closed_combobox - assert_combobox_display_and_value "#state-field", "Florida", "FL" + assert_combobox_display_and_value "#state-field", "Alaska", "AK" open_combobox "#state-field" assert_options_with count: 1 - delete_from_combobox "#state-field", "Florida", original: "Florida" + delete_from_combobox "#state-field", "Alaska", original: "Alaska" assert_options_with count: State.count type_in_combobox "#state-field", :down, :down, :down - assert_selected_option_with text: "Michigan" - click_on_option "Florida" + assert_selected_option_with text: "Arizona" + click_on_option "Alabama" assert_closed_combobox - assert_combobox_display_and_value "#state-field", "Florida", "FL" + assert_combobox_display_and_value "#state-field", "Alabama", "AL" end test "combobox with prefilled value" do @@ -325,7 +325,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase visit new_options_path open_combobox "#allow-new" - type_in_combobox "#allow-new", "Alaska", :enter + type_in_combobox "#allow-new", "Newplace", :enter open_combobox "#disallow-new" type_in_combobox "#disallow-new", "Alabama", :enter @@ -336,7 +336,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase end new_user = User.last - assert_equal "Alaska", new_user.favorite_state.name + assert_equal "Newplace", new_user.favorite_state.name assert_equal "Alabama", new_user.home_state.name end @@ -375,7 +375,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase visit new_options_path open_combobox "#disallow-new" - type_in_combobox "#disallow-new", "Alaska", :enter + type_in_combobox "#disallow-new", "Newplace", :enter find("input[type=submit]").click @@ -792,6 +792,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase end test "allows multiple selections (stays open, swaps out placeholder when selected)" do + skip visit multiple_path assert_selector "input[placeholder='State']" @@ -804,6 +805,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase end test "prefills multiple selections and hides already-selected options" do + skip visit multiple_prefilled_path assert_selector "div[id='FL'] div", text: "Florida" @@ -830,6 +832,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase end test "multiple selection with custom events" do + skip visit multiple_custom_events_path assert_text "Ready to listen for hw-combobox events!" @@ -935,10 +938,6 @@ def assert_selected_option_with(selector: "", **kwargs) assert_selector "li[role=option][aria-selected=true]#{selector}".squish, **kwargs end - def assert_current_option_with(selector: "", **kwargs) - assert_selector "li[role=option][aria-current=true]#{selector}".squish, **kwargs - end - def assert_no_visible_selected_option assert_no_selector "li[role=option][aria-selected=true]" end From 4bd94f75272c43423fab7c2c6cbd4933fba62af5 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 15:09:14 -0600 Subject: [PATCH 23/32] Add basic multiselect test --- .../app/views/comboboxes/multiselect.html.erb | 2 +- test/system/hotwire_combobox_test.rb | 44 ++++++++++++++----- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/test/dummy/app/views/comboboxes/multiselect.html.erb b/test/dummy/app/views/comboboxes/multiselect.html.erb index 2385570..112fbf8 100644 --- a/test/dummy/app/views/comboboxes/multiselect.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect.html.erb @@ -1,5 +1,5 @@ <%= combobox_tag "state", State.all, - id: "state-field", + id: "states-field", label: "States", multiselect_chip_src: new_state_chip_path, placeholder: "Select states" %> diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 27fbd2d..18feb7d 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -791,17 +791,24 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_option_with text: "display: Alabama\nvalue: AL" end - test "allows multiple selections (stays open, swaps out placeholder when selected)" do - skip - visit multiple_path + test "multiselect" do + visit multiselect_path + + open_combobox "#states-field" - assert_selector "input[placeholder='State']" - open_combobox "#state-field" click_on_option "Alabama" - assert_open_combobox - assert_no_selector "input[placeholder='State']" - find("#AL .hw-combobox__chip__remover").click - assert_selector "input[placeholder='State']" + click_on_option "California" + click_on_option "Arizona" + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama California Arizona ], + states(:alabama, :california, :arizona).pluck(:id) + + find("[aria-label='Remove California']").click + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama Arizona ], + states(:alabama, :arizona).pluck(:id) end test "prefills multiple selections and hides already-selected options" do @@ -930,8 +937,17 @@ def assert_combobox_value(selector, value) end def assert_combobox_display_and_value(selector, text, value) - assert_combobox_display selector, text - assert_combobox_value selector, value + if text.is_a? Array + assert_selection_chips(*text) + else + assert_combobox_display selector, text + end + + if value.is_a? Array + assert_combobox_value selector, value.join(",") + else + assert_combobox_value selector, value + end end def assert_selected_option_with(selector: "", **kwargs) @@ -962,6 +978,12 @@ def assert_proper_combobox_name_choice(original:, new:, proper:) end end + def assert_selection_chips(*texts) + texts.each do |text| + assert_selector "[data-hw-combobox-chip]", text: text + end + end + def locator_for(selector) # https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/Node/Matchers#has_field%3F-instance_method selector.delete_prefix("#") From fb28e17259a469f198ac5e757da651a27a9538b3 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 18:17:54 -0600 Subject: [PATCH 24/32] Add tests for existing multiselects --- lib/hotwire_combobox/helper.rb | 4 +- .../app/controllers/states_controller.rb | 16 ++- .../multiselect_async_html.html.erb | 6 +- .../state_chips/new_html.turbo_stream.erb | 2 +- .../app/views/states/index.turbo_stream.erb | 4 +- test/system/hotwire_combobox_test.rb | 124 ++++++++++++++---- 6 files changed, 126 insertions(+), 30 deletions(-) diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index 1f6be8d..a9e74af 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -31,7 +31,7 @@ def hw_combobox_options(options, render_in: {}, include_blank: nil, display: :to end def hw_paginated_combobox_options(options, for_id: params[:for_id], src: request.path, next_page: nil, render_in: {}, include_blank: {}, **methods) - include_blank = params[:page] ? nil : include_blank + include_blank = params[:page].to_i > 0 ? nil : include_blank options = hw_combobox_options options, render_in: render_in, include_blank: include_blank, **methods this_page = render "hotwire_combobox/paginated_options", for_id: for_id, options: options next_page = render "hotwire_combobox/next_page", for_id: for_id, src: src, next_page: next_page @@ -114,7 +114,7 @@ def hw_combobox_next_page_uri(uri, next_page, for_id) end def hw_combobox_page_stream_action - params[:page] ? :append : :update + params[:page].to_i > 0 ? :append : :update end def hw_blank_option(include_blank) diff --git a/test/dummy/app/controllers/states_controller.rb b/test/dummy/app/controllers/states_controller.rb index 2a4b039..4782e62 100644 --- a/test/dummy/app/controllers/states_controller.rb +++ b/test/dummy/app/controllers/states_controller.rb @@ -1,5 +1,19 @@ class StatesController < ApplicationController + before_action :set_page + def index - @states = State.search params[:q] + if @page + @states = @page.records + @next_page = @page.last? ? nil : @page.next_param + else + @states = State.search params[:q] + end end + + private + def set_page + if params[:page] + set_page_and_extract_portion_from State.search(params[:q]), per_page: 5 + end + end end diff --git a/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb b/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb index 0347d67..386920b 100644 --- a/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_async_html.html.erb @@ -6,7 +6,7 @@ display: flex; font-size: var(--hw-selection-chip-font-size); line-height: var(--hw-line-height); - padding: 0.5rem 1rem; + padding: 0 1rem; } .custom-remover { @@ -43,8 +43,8 @@ .state--missouri::before { background-image: url(<%= asset_path('missouri.png') %>); } -<%= combobox_tag "state", states_path, - id: "state-field", +<%= combobox_tag "state", states_path(page: 0), + id: "states-field", label: "States", multiselect_chip_src: new_html_state_chip_path, value: State.limit(2).ids.join(","), diff --git a/test/dummy/app/views/state_chips/new_html.turbo_stream.erb b/test/dummy/app/views/state_chips/new_html.turbo_stream.erb index 238d290..956c226 100644 --- a/test/dummy/app/views/state_chips/new_html.turbo_stream.erb +++ b/test/dummy/app/views/state_chips/new_html.turbo_stream.erb @@ -2,7 +2,7 @@ <%= within_combobox_selection_chip do %> <%= tag.div class: "custom-chip" do %> <%= tag.span class: "state state--#{state.name.downcase.parameterize}" %> - <%= tag.span state.name %> + <%= tag.p state.name %> <%= tag.span **combobox_chip_remover_attrs(display: state.name, value: state.id, class: "custom-remover") %> <% end %> <% end %> diff --git a/test/dummy/app/views/states/index.turbo_stream.erb b/test/dummy/app/views/states/index.turbo_stream.erb index cac29c8..8914859 100644 --- a/test/dummy/app/views/states/index.turbo_stream.erb +++ b/test/dummy/app/views/states/index.turbo_stream.erb @@ -1 +1,3 @@ -<%= hw_async_combobox_options @states, render_in: { partial: "states/state", formats: [:html] } %> +<%= hw_async_combobox_options @states, + render_in: { partial: "states/state", formats: [:html] }, + next_page: @next_page %> diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 18feb7d..b6cb188 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -799,43 +799,115 @@ class HotwireComboboxTest < ApplicationSystemTestCase click_on_option "Alabama" click_on_option "California" click_on_option "Arizona" + assert_no_visible_options_with text: "Alabama" + assert_no_visible_options_with text: "California" + assert_no_visible_options_with text: "Arizona" assert_combobox_display_and_value \ "#states-field", %w[ Alabama California Arizona ], states(:alabama, :california, :arizona).pluck(:id) - find("[aria-label='Remove California']").click + remove_chip "California" + assert_combobox_display_and_value \ "#states-field", %w[ Alabama Arizona ], states(:alabama, :arizona).pluck(:id) + + open_combobox "#states-field" + assert_option_with text: "California" end - test "prefills multiple selections and hides already-selected options" do - skip - visit multiple_prefilled_path + test "prefilled multiselect" do + visit multiselect_async_html_path - assert_selector "div[id='FL'] div", text: "Florida" - open_combobox "#state-field" - assert_no_selector "li[role=option]", text: "Florida" + assert_closed_combobox + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama Alaska ], + states(:alabama, :alaska).pluck(:id) + end - type_in_combobox "#state-field", "mi" - assert_no_selector "li[role=option]", text: "Alabama" - delete_from_combobox "#state-field", "mi", original: "mi" - assert_selector "li[role=option]", text: "Alabama" - assert_no_selector "li[role=option]", text: "Florida" + test "async multiselect" do + visit multiselect_async_html_path - type_in_combobox "#state-field", :down - assert_current_option_with text: "Alabama" - type_in_combobox "#state-field", :enter - assert_open_combobox - assert_selector "div[id='AL'] div", text: "Alabama" - find("#AL .hw-combobox__chip__remover").click - assert_no_selector "div[id='AL'] div", text: "Alabama" + open_combobox "#states-field" - click_on_option "Minnesota" - assert_open_combobox - assert_selector "div[id='MN'] div", text: "Minnesota" + assert_text "Arizona" + type_in_combobox "#states-field", "mi" + assert_selected_option_with text: "Michigan" + assert_options_with count: 4 + click_away + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama Alaska Michigan ], + states(:alabama, :alaska, :michigan).pluck(:id) + + # pagination + assert_options_with count: 8 # AL, AK, and MI are served but hidden + find("#states-field-hw-listbox").scroll_to :bottom + assert_options_with count: 13 + end + + test "html multiselect options and chips" do + visit multiselect_async_html_path + + open_combobox "#states-field" + + assert_option_with html_markup: "div > p", text: "Arizona" + assert_chip_with html_markup: "div > p", text: "Alabama" + + remove_chip "Alabama" + + assert_combobox_display_and_value \ + "#states-field", + %w[ Alaska ], + [ states(:alaska).id ] + end + + test "multiselect form" do + visit multiselect_prefilled_form_path + + assert_closed_combobox + assert_combobox_display_and_value \ + "#user_visited_state_ids", + %w[ Florida Illinois ], + states(:florida, :illinois).pluck(:id) + + open_combobox "#user_visited_state_ids" + type_in_combobox "#user_visited_state_ids", "Lou" + click_on_option "Louisiana" + assert_text "Alabama" # combobox is reset and still open + + assert_combobox_display_and_value \ + "#user_visited_state_ids", + %w[ Florida Illinois Louisiana ], + states(:florida, :illinois, :louisiana).pluck(:id) + + click_away + find("input[type=submit]").click + assert_text "User updated" + + assert_combobox_display_and_value \ + "#user_visited_state_ids", + %w[ Florida Illinois Louisiana ], + states(:florida, :illinois, :louisiana).pluck(:id) + + remove_chip "Florida" + + assert_combobox_display_and_value \ + "#user_visited_state_ids", + %w[ Illinois Louisiana ], + states(:illinois, :louisiana).pluck(:id) + + click_away + find("input[type=submit]").click + assert_text "User updated" + + assert_combobox_display_and_value \ + "#user_visited_state_ids", + %w[ Illinois Louisiana ], + states(:illinois, :louisiana).pluck(:id) end test "multiple selection with custom events" do @@ -928,6 +1000,10 @@ def assert_option_with(html_markup: "", **kwargs) end alias_method :assert_options_with, :assert_option_with + def assert_chip_with(html_markup: "", **kwargs) + assert_selector "[data-hw-combobox-chip] #{html_markup}".squish, **kwargs + end + def assert_combobox_display(selector, text) assert_field locator_for(selector), with: text end @@ -984,6 +1060,10 @@ def assert_selection_chips(*texts) end end + def remove_chip(text) + find("[aria-label='Remove #{text}']").click + end + def locator_for(selector) # https://rubydoc.info/github/teamcapybara/capybara/master/Capybara/Node/Matchers#has_field%3F-instance_method selector.delete_prefix("#") From 6bbceeb3e95a5b76507abb2cb8385fd7c737aa50 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 18:21:06 -0600 Subject: [PATCH 25/32] Fix chips flicker --- .../controllers/hw_combobox_controller.js | 2 +- .../models/combobox/multiselect.js | 19 +++++++++++++++++-- app/presenters/hotwire_combobox/component.rb | 3 ++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 6c50808..533202a 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -61,13 +61,13 @@ export default class HwComboboxController extends Concerns(...concerns) { initialize() { this._initializeActors() this._initializeFiltering() + this._initializeMultiselect() } connect() { this._connectSelection() this._connectListAutocomplete() this._connectDialog() - this._connectMultiselect() } disconnect() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index 041ebf3..6073ad6 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -25,6 +25,10 @@ Combobox.Multiselect = Base => class extends Base { this._announceToScreenReader(display, "removed") } + hideChipsForCache() { + this.element.querySelectorAll("[data-hw-combobox-chip]").forEach(chip => chip.hidden = true) + } + _chipKeyHandlers = { Backspace: (event) => { this.removeChip(event) @@ -44,8 +48,11 @@ Combobox.Multiselect = Base => class extends Base { } } - _connectMultiselect() { - this._preselectMultiple() + _initializeMultiselect() { + if (!this._isMultiPreselected) { + this._preselectMultiple() + this._markMultiPreselected() + } } _createChip() { @@ -125,6 +132,10 @@ Combobox.Multiselect = Base => class extends Base { this.chipDismisserTargets[this.chipDismisserTargets.length - 1].focus() } + _markMultiPreselected() { + this.element.dataset.multiPreselected = "" + } + get _isMultiselect() { return this.hasSelectionChipSrcValue } @@ -132,4 +143,8 @@ Combobox.Multiselect = Base => class extends Base { get _isSingleSelect() { return !this._isMultiselect } + + get _isMultiPreselected() { + return this.element.hasAttribute("data-multi-preselected") + } } diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index b0b892b..4195500 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -314,7 +314,8 @@ def input_data keydown->hw-combobox#navigate click@window->hw-combobox#closeOnClickOutside focusin@window->hw-combobox#closeOnFocusOutside - turbo:before-stream-render@document->hw-combobox#rerouteListboxStreamToDialog".squish, + turbo:before-stream-render@document->hw-combobox#rerouteListboxStreamToDialog + turbo:before-cache@document->hw-combobox#hideChipsForCache".squish, hw_combobox_target: "combobox", async_id: canonical_id end From 29b917d137ed3af3c312fc727878a52850978e77 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 18:39:14 -0600 Subject: [PATCH 26/32] Add chips shorthand methods --- lib/hotwire_combobox/helper.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index a9e74af..b2a329a 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/lib/hotwire_combobox/helper.rb @@ -52,6 +52,15 @@ def hw_combobox_selection_chip(display:, value:, for_id: params[:for_id], remove remover_attrs: remover_attrs end + def hw_combobox_selection_chips_for(objects, display:, value:, for_id: params[:for_id]) + objects.map do |object| + hw_combobox_selection_chip \ + display: hw_call_method(object, display), + value: hw_call_method(object, value), + for_id: for_id + end.then { |chips| safe_join chips } + end + def hw_dismissing_combobox_selection_chip(display:, value:, for_id: params[:for_id]) hw_combobox_selection_chip \ display: display, @@ -60,6 +69,15 @@ def hw_dismissing_combobox_selection_chip(display:, value:, for_id: params[:for_ remover_attrs: hw_combobox_dismissing_chip_remover_attrs(display, value) end + def hw_dismissing_combobox_selection_chips_for(objects, display:, value:, for_id: params[:for_id]) + objects.map do |object| + hw_dismissing_combobox_selection_chip \ + display: hw_call_method(object, display), + value: hw_call_method(object, value), + for_id: for_id + end.then { |chips| safe_join chips } + end + def hw_combobox_chip_remover_attrs(display:, value:, **kwargs) { tabindex: "0", @@ -86,7 +104,9 @@ def hw_combobox_dismissing_chip_remover_attrs(display, value) hw_alias :hw_async_combobox_options hw_alias :hw_within_combobox_selection_chip hw_alias :hw_combobox_selection_chip + hw_alias :hw_combobox_selection_chips_for hw_alias :hw_dismissing_combobox_selection_chip + hw_alias :hw_dismissing_combobox_selection_chips_for hw_alias :hw_combobox_chip_remover_attrs hw_alias :hw_combobox_dismissing_chip_remover_attrs From 6e32470cac6f3202181e28f3a34b12d26bec0ec8 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 18:39:23 -0600 Subject: [PATCH 27/32] Add tests for dismissing chips --- test/dummy/app/controllers/state_chips_controller.rb | 5 +++++ .../app/views/comboboxes/multiselect_dismissing.html.erb | 5 +++++ test/dummy/app/views/state_chips/new.turbo_stream.erb | 3 --- test/dummy/config/routes.rb | 1 + 4 files changed, 11 insertions(+), 3 deletions(-) delete mode 100644 test/dummy/app/views/state_chips/new.turbo_stream.erb diff --git a/test/dummy/app/controllers/state_chips_controller.rb b/test/dummy/app/controllers/state_chips_controller.rb index 24feb12..3d8f600 100644 --- a/test/dummy/app/controllers/state_chips_controller.rb +++ b/test/dummy/app/controllers/state_chips_controller.rb @@ -2,11 +2,16 @@ class StateChipsController < ApplicationController before_action :set_states def new + render turbo_stream: helpers.combobox_selection_chips_for(@states, display: :name, value: :id) end def new_html end + def new_dismissing + render turbo_stream: helpers.dismissing_combobox_selection_chips_for(@states, display: :name, value: :id) + end + private def set_states @states = State.find params[:combobox_values].split(",") diff --git a/test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb b/test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb index e69de29..30b1685 100644 --- a/test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_dismissing.html.erb @@ -0,0 +1,5 @@ +<%= combobox_tag "state", State.all, + id: "states-field", + label: "States", + multiselect_chip_src: new_dismissing_state_chip_path, + placeholder: "Select states" %> diff --git a/test/dummy/app/views/state_chips/new.turbo_stream.erb b/test/dummy/app/views/state_chips/new.turbo_stream.erb deleted file mode 100644 index 9ccb1df..0000000 --- a/test/dummy/app/views/state_chips/new.turbo_stream.erb +++ /dev/null @@ -1,3 +0,0 @@ -<% @states.each do |state| %> - <%= combobox_selection_chip display: state.name, value: state.id %> -<% end %> diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 9489178..e625820 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -40,6 +40,7 @@ resources :states, only: :index resources :state_chips, only: :new, param: :combobox_value get "new_html_state_chip", to: "state_chips#new_html" + get "new_dismissing_state_chip", to: "state_chips#new_dismissing" resources :users, only: :update root to: "comboboxes#plain" From 9ac4eaa45736a1acedd1c24f8863abe1a0cb015d Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 18:57:13 -0600 Subject: [PATCH 28/32] Fix multiselect events --- .../hw_combobox/models/combobox/events.js | 4 ++-- .../hw_combobox/models/combobox/form_field.js | 11 ++++++++++ .../models/combobox/multiselect.js | 4 ++-- .../multiselect_custom_events.html.erb | 20 ++++++++----------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/hw_combobox/models/combobox/events.js b/app/assets/javascripts/hw_combobox/models/combobox/events.js index 5b3ec07..223d9f5 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/events.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/events.js @@ -3,7 +3,7 @@ import { dispatch } from "hw_combobox/helpers" Combobox.Events = Base => class extends Base { _dispatchSelectionEvent({ isNewAndAllowed, previousValue }) { - if (previousValue === this._fieldValueString) return + if (previousValue === this._incomingFieldValueString) return dispatch("hw-combobox:selection", { target: this.element, @@ -20,7 +20,7 @@ Combobox.Events = Base => class extends Base { get _eventableDetails() { return { - value: this._fieldValueString, + value: this._incomingFieldValueString, display: this._fullQuery, query: this._typedQuery, fieldName: this._fieldName, diff --git a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js index 738eb59..33556a2 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js @@ -20,6 +20,17 @@ Combobox.FormField = Base => class extends Base { } } + get _incomingFieldValueString() { + if (this._isMultiselect) { + const array = this._fieldValueArray + array.push(this.hiddenFieldTarget.dataset.valueForMultiselect) + + return array.join(",") + } else { + return this.hiddenFieldTarget.value + } + } + get _fieldValueArray() { if (this._isMultiselect) { return Array.from(this._fieldValue) diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index 6073ad6..ccaa821 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -111,7 +111,7 @@ Combobox.Multiselect = Base => class extends Base { } _addToFieldValue(value) { - let newValue = this._fieldValue + const newValue = this._fieldValue newValue.add(String(value)) this.hiddenFieldTarget.value = Array.from(newValue).join(",") @@ -120,7 +120,7 @@ Combobox.Multiselect = Base => class extends Base { } _removeFromFieldValue(value) { - let newValue = this._fieldValue + const newValue = this._fieldValue newValue.delete(String(value)) this.hiddenFieldTarget.value = Array.from(newValue).join(",") diff --git a/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb b/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb index 1ee0a99..04990a6 100644 --- a/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb @@ -1,8 +1,5 @@
@@ -17,13 +14,12 @@
- <%= combobox_tag :state, - @state_options, - id: "state-field", - data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed" }, - multiple: true, - value: ['FL', 'MO'] do |combobox| %> - <% combobox.customize_main_wrapper class: "wide-multiple-field" %> - <% end %> + <%= combobox_tag "state", State.all, + id: "states-field", + label: "States", + multiselect_chip_src: new_state_chip_path, + placeholder: "Select states", + data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed" } %>
+ From dba54cb034b10ec83049e03819a36c1b6ee8c409 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Mar 2024 19:33:35 -0600 Subject: [PATCH 29/32] Test multiselect idiosyncrasies --- app/assets/javascripts/hw_combobox/helpers.js | 16 +++++++++ .../models/combobox/multiselect.js | 9 +++-- test/system/hotwire_combobox_test.rb | 33 +++++++++++++++++++ test/system_test_helper.rb | 9 +++++ 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/hw_combobox/helpers.js b/app/assets/javascripts/hw_combobox/helpers.js index 766ab6f..a40a282 100644 --- a/app/assets/javascripts/hw_combobox/helpers.js +++ b/app/assets/javascripts/hw_combobox/helpers.js @@ -79,3 +79,19 @@ export function dispatch(eventName, { target, cancelable, detail } = {}) { return event } + +export function nextRepaint() { + if (document.visibilityState === "hidden") { + return nextEventLoopTick() + } else { + return nextAnimationFrame() + } +} + +export function nextAnimationFrame() { + return new Promise((resolve) => requestAnimationFrame(() => resolve())) +} + +export function nextEventLoopTick() { + return new Promise((resolve) => setTimeout(() => resolve(), 0)) +} diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index ccaa821..5e1620b 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -1,5 +1,5 @@ import Combobox from "hw_combobox/models/combobox/base" -import { cancel } from "hw_combobox/helpers" +import { cancel, nextRepaint } from "hw_combobox/helpers" import { get } from "hw_combobox/vendor/requestjs" Combobox.Multiselect = Base => class extends Base { @@ -55,7 +55,7 @@ Combobox.Multiselect = Base => class extends Base { } } - _createChip() { + async _createChip() { if (!this._isMultiselect) return this._beforeClearingMultiselectQuery(async (display, value) => { @@ -63,7 +63,10 @@ Combobox.Multiselect = Base => class extends Base { this._filter("hw:multiselectSync") this._requestChips(value) this._addToFieldValue(value) - if (this._isSync) this.openByFocusing() + if (this._isSync) { + await nextRepaint() + this.openByFocusing() + } this._announceToScreenReader(display, "multi-selected. Press Shift + Tab, then Enter to remove.") }) } diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index b6cb188..6fd6470 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -797,8 +797,11 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "#states-field" click_on_option "Alabama" + assert_chip_with text: "Alabama" # wait for async chip creation click_on_option "California" + assert_chip_with text: "California" click_on_option "Arizona" + assert_chip_with text: "Arizona" assert_no_visible_options_with text: "Alabama" assert_no_visible_options_with text: "California" assert_no_visible_options_with text: "Arizona" @@ -818,6 +821,32 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_option_with text: "California" end + test "multiselect idiosyncrasies" do + visit multiselect_path + + open_combobox "#states-field" + assert_focused_combobox "#states-field" + + click_on_option "Alabama" + assert_focused_combobox "#states-field" + assert_chip_with text: "Alabama" # wait for async chip creation + assert_no_visible_options_with text: "Alabama" + type_in_combobox "#states-field", "ala" + type_in_combobox "#states-field", :backspace # clear autocompleted portion + assert_no_visible_options_with text: "Alabama" + remove_chip "Alabama" + assert_option_with text: "Alabama" + + click_on_option "Alabama" + assert_chip_with text: "Alabama" # wait for async chip creation + type_in_combobox "#states-field", "michi" + assert_selected_option_with text: "Michigan" + type_in_combobox "#states-field", :backspace # clear autocompleted portion + assert_option_with text: "Michigan" + remove_chip "Alabama" + assert_no_visible_options_with text: "Alabama" + end + test "prefilled multiselect" do visit multiselect_async_html_path @@ -1060,6 +1089,10 @@ def assert_selection_chips(*texts) end end + def assert_focused_combobox(selector) + page.evaluate_script("document.activeElement.id") == locator_for(selector) + end + def remove_chip(text) find("[aria-label='Remove #{text}']").click end diff --git a/test/system_test_helper.rb b/test/system_test_helper.rb index 3cdba90..ed2e8ed 100644 --- a/test/system_test_helper.rb +++ b/test/system_test_helper.rb @@ -9,3 +9,12 @@ end Capybara.default_max_wait_time = 10 + +Minitest.after_run do + puts "\n" + puts <<~WARNING + ⚠️ Warning + Focus tests might pass on Selenium but not when checked manually on Chrome. + Make sure you grep for `assert_focused_combobox` and test manually before releasing a new version. + WARNING +end From cfe4aa0681341b1ac3b203d5ec2135adafa19cfd Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Tue, 19 Mar 2024 18:15:22 -0600 Subject: [PATCH 30/32] Test multiselect events --- .../controllers/hw_combobox_controller.js | 2 +- .../hw_combobox/models/combobox/events.js | 11 ++- .../hw_combobox/models/combobox/form_field.js | 5 +- .../models/combobox/multiselect.js | 7 +- .../hw_combobox/models/combobox/navigation.js | 4 +- .../hw_combobox/models/combobox/selection.js | 2 +- .../hw_combobox/models/combobox/toggle.js | 23 ++++-- .../controllers/combobox/events_controller.js | 27 ++++--- .../multiselect_custom_events.html.erb | 6 +- test/system/hotwire_combobox_test.rb | 76 ++++++++++++++----- 10 files changed, 117 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 533202a..1f132df 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -99,7 +99,7 @@ export default class HwComboboxController extends Concerns(...concerns) { } closerTargetConnected() { - this._closeAndBlur() + this._closeAndBlur("hw:asyncCloser") } // Use +_printStack+ for debugging purposes diff --git a/app/assets/javascripts/hw_combobox/models/combobox/events.js b/app/assets/javascripts/hw_combobox/models/combobox/events.js index 223d9f5..b36046c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/events.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/events.js @@ -5,19 +5,26 @@ Combobox.Events = Base => class extends Base { _dispatchSelectionEvent({ isNewAndAllowed, previousValue }) { if (previousValue === this._incomingFieldValueString) return - dispatch("hw-combobox:selection", { + dispatch("hw-combobox:selection", { // TODO: rename to preselection target: this.element, detail: { ...this._eventableDetails, isNewAndAllowed, previousValue } }) } _dispatchClosedEvent() { - dispatch("hw-combobox:closed", { + dispatch("hw-combobox:closed", { // TODO: rename to selection target: this.element, detail: this._eventableDetails }) } + _dispatchRemovalEvent({ removedDisplay, removedValue }) { + dispatch("hw-combobox:removal", { + target: this.element, + detail: { ...this._eventableDetails, removedDisplay, removedValue } + }) + } + get _eventableDetails() { return { value: this._incomingFieldValueString, diff --git a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js index 33556a2..f361760 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js @@ -23,7 +23,10 @@ Combobox.FormField = Base => class extends Base { get _incomingFieldValueString() { if (this._isMultiselect) { const array = this._fieldValueArray - array.push(this.hiddenFieldTarget.dataset.valueForMultiselect) + + if (this.hiddenFieldTarget.dataset.valueForMultiselect) { + array.push(this.hiddenFieldTarget.dataset.valueForMultiselect) + } return array.join(",") } else { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index 5e1620b..cb11adb 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -23,6 +23,7 @@ Combobox.Multiselect = Base => class extends Base { } this._announceToScreenReader(display, "removed") + this._dispatchRemovalEvent({ removedDisplay: display, removedValue: params.value }) } hideChipsForCache() { @@ -55,7 +56,7 @@ Combobox.Multiselect = Base => class extends Base { } } - async _createChip() { + async _createChip(shouldReopen) { if (!this._isMultiselect) return this._beforeClearingMultiselectQuery(async (display, value) => { @@ -63,7 +64,7 @@ Combobox.Multiselect = Base => class extends Base { this._filter("hw:multiselectSync") this._requestChips(value) this._addToFieldValue(value) - if (this._isSync) { + if (shouldReopen) { await nextRepaint() this.openByFocusing() } @@ -132,7 +133,7 @@ Combobox.Multiselect = Base => class extends Base { } _focusLastChipDismisser() { - this.chipDismisserTargets[this.chipDismisserTargets.length - 1].focus() + this.chipDismisserTargets[this.chipDismisserTargets.length - 1]?.focus() } _markMultiPreselected() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js index 182b85a..d1d299e 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/navigation.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/navigation.js @@ -26,11 +26,11 @@ Combobox.Navigation = Base => class extends Base { cancel(event) }, Enter: (event) => { - this._closeAndBlur() + this._closeAndBlur("hw:keyHandler:enter") cancel(event) }, Escape: (event) => { - this._closeAndBlur() + this._closeAndBlur("hw:keyHandler:escape") cancel(event) }, Backspace: (event) => { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/selection.js b/app/assets/javascripts/hw_combobox/models/combobox/selection.js index d72b0ad..f0b5732 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/selection.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/selection.js @@ -4,7 +4,7 @@ import { wrapAroundAccess, isDeleteEvent } from "hw_combobox/helpers" Combobox.Selection = Base => class extends Base { selectOnClick({ currentTarget, inputType }) { this._forceSelectionAndFilter(currentTarget, inputType) - this._closeAndBlur() + this._closeAndBlur("hw:optionRoleClick") } _connectSelection() { diff --git a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js index 0d40bb0..9a6d032 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/toggle.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/toggle.js @@ -10,15 +10,24 @@ Combobox.Toggle = Base => class extends Base { this._actingCombobox.focus() } - close() { + close(inputType) { if (this._isOpen) { + const shouldReopen = this._isMultiselect && + this._isSync && + !this._isSmallViewport && + inputType != "hw:clickOutside" && + inputType != "hw:focusOutside" + this._lockInSelection() this._clearInvalidQuery() this.expandedValue = false this._dispatchClosedEvent() - this._createChip() + + if (inputType != "hw:keyHandler:escape") { + this._createChip(shouldReopen) + } if (this._isSingleSelect && this._selectedOptionElement) { this._announceToScreenReader(this._displayForOptionElement(this._selectedOptionElement), "selected") @@ -28,7 +37,7 @@ Combobox.Toggle = Base => class extends Base { toggle() { if (this.expandedValue) { - this._closeAndBlur() + this._closeAndBlur("hw:toggle") } else { this.openByFocusing() } @@ -41,14 +50,14 @@ Combobox.Toggle = Base => class extends Base { if (this.mainWrapperTarget.contains(target) && !this._isDialogDismisser(target)) return if (this._withinElementBounds(event)) return - this._closeAndBlur() + this._closeAndBlur("hw:clickOutside") } closeOnFocusOutside({ target }) { if (!this._isOpen) return if (this.element.contains(target)) return - this._closeAndBlur() + this._closeAndBlur("hw:focusOutside") } clearOrToggleOnHandleClick() { @@ -60,8 +69,8 @@ Combobox.Toggle = Base => class extends Base { } } - _closeAndBlur() { - this.close() + _closeAndBlur(inputType) { + this.close(inputType) this._actingCombobox.blur() } diff --git a/test/dummy/app/javascript/controllers/combobox/events_controller.js b/test/dummy/app/javascript/controllers/combobox/events_controller.js index 5fadf17..75dbd9c 100644 --- a/test/dummy/app/javascript/controllers/combobox/events_controller.js +++ b/test/dummy/app/javascript/controllers/combobox/events_controller.js @@ -1,7 +1,7 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = [ "selectionScratchpad", "selectionCount", "closedScratchpad", "closedCount" ] + static targets = [ "selectionScratchpad", "selectionCount", "closedScratchpad", "closedCount", "removalScratchpad", "removalCount" ] connect() { this.selectionScratchpadTarget.innerText = "Ready to listen for hw-combobox events!" @@ -21,6 +21,13 @@ export default class extends Controller { this.closedCountTarget.innerText = `closings: ${this.closedCount}.` } + showRemoval(event) { + this.removalCount ??= 0 + this.removalCount++ + this.removalScratchpadTarget.innerText = this.#template(event) + this.removalCountTarget.innerText = `removals: ${this.removalCount}.` + } + #template(event) { const cast = (string) => { let _string = String(string) @@ -28,14 +35,16 @@ export default class extends Controller { return _string || "" } - return `event: ${cast(event.type) || ""} - value: ${cast(event.detail.value) || ""} - display: ${cast(event.detail.display) || ""} - query: ${cast(event.detail.query) || ""} - fieldName: ${cast(event.detail.fieldName) || ""} - isNewAndAllowed: ${cast(event.detail.isNewAndAllowed) || ""} - isValid: ${cast(event.detail.isValid) || ""} - previousValue: ${cast(event.detail.previousValue) || ""} + return `event: ${cast(event.type)} + value: ${cast(event.detail.value)}. + display: ${cast(event.detail.display)}. + query: ${cast(event.detail.query)}. + fieldName: ${cast(event.detail.fieldName)}. + isNewAndAllowed: ${cast(event.detail.isNewAndAllowed)}. + isValid: ${cast(event.detail.isValid)}. + previousValue: ${cast(event.detail.previousValue)}. + removedDisplay: ${cast(event.detail.removedDisplay)}. + removedValue: ${cast(event.detail.removedValue)}. ` } } diff --git a/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb b/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb index 04990a6..e065980 100644 --- a/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_custom_events.html.erb @@ -6,20 +6,22 @@
+
+
- <%= combobox_tag "state", State.all, + <%= combobox_tag "states", State.all, id: "states-field", label: "States", multiselect_chip_src: new_state_chip_path, placeholder: "Select states", - data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed" } %> + data: { action: "hw-combobox:selection->combobox--events#showSelection hw-combobox:closed->combobox--events#showClosed hw-combobox:removal->combobox--events#showRemoval" } %>
diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 6fd6470..510df8b 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -939,39 +939,79 @@ class HotwireComboboxTest < ApplicationSystemTestCase states(:illinois, :louisiana).pluck(:id) end - test "multiple selection with custom events" do - skip - visit multiple_custom_events_path + test "multiselect custom events" do + visit multiselect_custom_events_path assert_text "Ready to listen for hw-combobox events!" - - open_combobox "#state-field" - type_in_combobox "#state-field", "mi" - assert_no_text "event: hw-combobox:selection" assert_no_text "event: hw-combobox:closed" assert_no_text "selections:" - click_on_option "Minnesota" + open_combobox "#states-field" + type_in_combobox "#states-field", "mi" assert_text "event: hw-combobox:selection" - assert_text 'value: ["FL","MO","MN"]' - assert_text "selections: 1." - assert_no_text "event: hw-combobox:closed" - - find("#MO .hw-combobox__chip__remover").click - assert_text 'value: ["FL","MN"]' - assert_text "selections: 2." + assert_text "value: #{states(:michigan).id}." + assert_text "display: Michigan" + assert_text "query: Mi" + assert_text "fieldName: states" + assert_text "isNewAndAllowed: false" + assert_text "isValid: true" + assert_text "previousValue: " + assert_text "selections: 2." # `m`, then `mi` assert_no_text "event: hw-combobox:closed" - assert_no_text "closings:" click_away + assert_closed_combobox + + assert_text "event: hw-combobox:closed" + assert_text "value: #{states(:michigan).id}." + assert_text "display: Michigan" + assert_text "query: Michigan" + assert_text "fieldName: states" + assert_text "isNewAndAllowed: " + assert_text "isValid: true" + assert_text "previousValue: " + assert_text "closings: 1." + + assert_text "selections: 3." + + remove_chip "Michigan" + assert_open_combobox + + assert_text "removedDisplay: Michigan." + assert_text "removedValue: #{states(:michigan).id}." + assert_text "removals: 1." + + click_on_option "Arkansas" + click_on_option "Colorado" assert_text "event: hw-combobox:selection" - assert_text 'value: ["FL","MN"]' + assert_text "value: #{states(:arkansas, :colorado).pluck(:id).join(",")}." + assert_text "display: Colorado." + assert_text "query: Colorado." + assert_text "fieldName: states." + assert_text "isNewAndAllowed: false." + assert_text "isValid: true." + assert_text "previousValue: 1011954550." + assert_text "removedDisplay: ." + assert_text "removedValue: ." assert_text "event: hw-combobox:closed" - assert_text "closings: 1." + assert_text "value: #{states(:arkansas, :colorado).pluck(:id).join(",")}." + assert_text "display: Colorado." + assert_text "query: Colorado." + assert_text "fieldName: states." + assert_text "isNewAndAllowed: ." + assert_text "isValid: true." + assert_text "previousValue: ." + assert_text "removedDisplay: ." + assert_text "removedValue: ." + + click_away + + assert_text "selections: 7." # TODO: lockInSelection causes duplicate selection events; shouldn't lock-in unnecessarily + assert_text "closings: 4." end private From 25dbf969d1b3ff21ab1ec993ff2a552d66e01ba4 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Tue, 19 Mar 2024 18:20:38 -0600 Subject: [PATCH 31/32] Add test for navigating chips --- test/system/hotwire_combobox_test.rb | 36 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 510df8b..a65dcf2 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -1014,6 +1014,22 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_text "closings: 4." end + test "navigating chips with keyboard" do + visit multiselect_prefilled_form_path + + open_combobox "#user_visited_state_ids" + assert_combobox_display "#user_visited_state_ids", %w[ Florida Illinois ] + type_in_combobox "#user_visited_state_ids", :backspace, :enter + assert_combobox_display "#user_visited_state_ids", %w[ Florida ] + + visit multiselect_prefilled_form_path + + open_combobox "#user_visited_state_ids" + assert_combobox_display "#user_visited_state_ids", %w[ Florida Illinois ] + type_in_combobox "#user_visited_state_ids", %i[shift tab], %i[shift tab], :enter + assert_combobox_display "#user_visited_state_ids", %w[ Illinois ] + end + private def open_combobox(selector) find(selector).click @@ -1074,25 +1090,21 @@ def assert_chip_with(html_markup: "", **kwargs) end def assert_combobox_display(selector, text) - assert_field locator_for(selector), with: text + if text.is_a? Array + assert_selection_chips(*text) + else + assert_field locator_for(selector), with: text + end end def assert_combobox_value(selector, value) + value = value.join(",") if value.is_a? Array assert_field "#{locator_for(selector)}-hw-hidden-field", type: "hidden", with: value end def assert_combobox_display_and_value(selector, text, value) - if text.is_a? Array - assert_selection_chips(*text) - else - assert_combobox_display selector, text - end - - if value.is_a? Array - assert_combobox_value selector, value.join(",") - else - assert_combobox_value selector, value - end + assert_combobox_display selector, text + assert_combobox_value selector, value end def assert_selected_option_with(selector: "", **kwargs) From dbdc7cae2065c68ff23640cec06ee8c88d48a08b Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Tue, 19 Mar 2024 19:45:05 -0600 Subject: [PATCH 32/32] Multiselect new values --- .../hw_combobox/models/combobox/form_field.js | 5 ++- .../models/combobox/multiselect.js | 12 ++++-- app/presenters/hotwire_combobox/component.rb | 14 +++++++ .../app/controllers/comboboxes_controller.rb | 1 + .../app/controllers/state_chips_controller.rb | 10 ++++- .../app/controllers/visits_controller.rb | 23 +++++++++++ .../multiselect_new_values.html.erb | 13 +++++++ test/dummy/config/routes.rb | 6 ++- test/helpers/hotwire_combobox/helper_test.rb | 10 +++++ test/system/hotwire_combobox_test.rb | 38 +++++++++++++++++++ 10 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 test/dummy/app/controllers/visits_controller.rb diff --git a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js index f361760..6b8f17c 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/form_field.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/form_field.js @@ -44,7 +44,7 @@ Combobox.FormField = Base => class extends Base { set _fieldValue(value) { if (this._isMultiselect) { - this.hiddenFieldTarget.dataset.valueForMultiselect = value + this.hiddenFieldTarget.dataset.valueForMultiselect = value?.replace(/,/g, "") this.hiddenFieldTarget.dataset.displayForMultiselect = this._fullQuery } else { this.hiddenFieldTarget.value = value @@ -53,7 +53,8 @@ Combobox.FormField = Base => class extends Base { get _hasEmptyFieldValue() { if (this._isMultiselect) { - return this.hiddenFieldTarget.dataset.valueForMultiselect === "" + return this.hiddenFieldTarget.dataset.valueForMultiselect == "" || + this.hiddenFieldTarget.dataset.valueForMultiselect == "undefined" } else { return this.hiddenFieldTarget.value === "" } diff --git a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js index cb11adb..a857cad 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js +++ b/app/assets/javascripts/hw_combobox/models/combobox/multiselect.js @@ -8,11 +8,17 @@ Combobox.Multiselect = Base => class extends Base { } removeChip({ currentTarget, params }) { + let display const option = this._optionElementWithValue(params.value) - const display = option.getAttribute(this.autocompletableAttributeValue) - this._markNotSelected(option) - this._markNotMultiselected(option) + if (option) { + display = option.getAttribute(this.autocompletableAttributeValue) + this._markNotSelected(option) + this._markNotMultiselected(option) + } else { + display = params.value // for new options + } + this._removeFromFieldValue(params.value) this._filter("hw:multiselectSync") diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index 4195500..a1abf20 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -2,9 +2,12 @@ class HotwireCombobox::Component include Customizable + include ActiveModel::Validations attr_reader :options, :label + validate :name_when_new_on_multiselect_must_match_original_name + def initialize \ view, name, association_name: nil, @@ -30,6 +33,8 @@ def initialize \ @combobox_attrs = input.reverse_merge(rest).deep_symbolize_keys @association_name = association_name || infer_association_name + + validate! end def render_in(view_context, &block) @@ -195,6 +200,15 @@ def pagination_attrs :name_when_new, :open, :data, :combobox_attrs, :mobile_at, :association_name, :multiselect_chip_src + def name_when_new_on_multiselect_must_match_original_name + return unless multiselect? && name_when_new.present? + + unless name_when_new.to_s == name + errors.add :name_when_new, :must_match_original_name, + message: "must match the regular name ('#{name}', in this case) on multiselect comboboxes." + end + end + def multiselect? multiselect_chip_src.present? end diff --git a/test/dummy/app/controllers/comboboxes_controller.rb b/test/dummy/app/controllers/comboboxes_controller.rb index 6ac2a68..f4f66a7 100644 --- a/test/dummy/app/controllers/comboboxes_controller.rb +++ b/test/dummy/app/controllers/comboboxes_controller.rb @@ -87,6 +87,7 @@ def multiselect_custom_events end def multiselect_new_values + @user = User.first || raise("No user found, load fixtures first.") end private diff --git a/test/dummy/app/controllers/state_chips_controller.rb b/test/dummy/app/controllers/state_chips_controller.rb index 3d8f600..0709785 100644 --- a/test/dummy/app/controllers/state_chips_controller.rb +++ b/test/dummy/app/controllers/state_chips_controller.rb @@ -1,5 +1,5 @@ class StateChipsController < ApplicationController - before_action :set_states + before_action :set_states, except: :new_possibly_new def new render turbo_stream: helpers.combobox_selection_chips_for(@states, display: :name, value: :id) @@ -12,6 +12,14 @@ def new_dismissing render turbo_stream: helpers.dismissing_combobox_selection_chips_for(@states, display: :name, value: :id) end + def new_possibly_new + @states = params[:combobox_values].split(",").map do |value| + State.find_by(id: value) || OpenStruct.new(name: value, id: value) + end + + render turbo_stream: helpers.combobox_selection_chips_for(@states, display: :name, value: :id) + end + private def set_states @states = State.find params[:combobox_values].split(",") diff --git a/test/dummy/app/controllers/visits_controller.rb b/test/dummy/app/controllers/visits_controller.rb new file mode 100644 index 0000000..2db8c80 --- /dev/null +++ b/test/dummy/app/controllers/visits_controller.rb @@ -0,0 +1,23 @@ +class VisitsController < ApplicationController + before_action :set_user + + def create + User.transaction do + @states = find_or_create_states! + @user.update! visited_states: @states + end + + redirect_back_or_to multiselect_new_values_path, notice: "Visits created" + end + + private + def set_user + @user = User.find(params[:user_id]) + end + + def find_or_create_states! + @states = params[:user][:visited_states].split(",").map do |value| + State.find_by(id: value) || State.create!(name: value) + end + end +end diff --git a/test/dummy/app/views/comboboxes/multiselect_new_values.html.erb b/test/dummy/app/views/comboboxes/multiselect_new_values.html.erb index e69de29..f92e872 100644 --- a/test/dummy/app/views/comboboxes/multiselect_new_values.html.erb +++ b/test/dummy/app/views/comboboxes/multiselect_new_values.html.erb @@ -0,0 +1,13 @@ +<% if flash[:notice] %> +

<%= flash[:notice] %>

+<% end %> + +<%= form_with model: @user, url: user_visits_url(@user), method: :post, html: { style: "display: flex; flex-direction: column; gap: 1rem;" } do |form| %> + <%= combobox_tag "user[visited_states]", + State.all, + id: "states-field", + label: "Visited states (new allowed)", + multiselect_chip_src: new_possibly_new_state_chip_path, + name_when_new: "user[visited_states]" %> + <%= form.submit style: "padding: 0.5rem;" %> +<% end %> diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index e625820..4057171 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -41,7 +41,11 @@ resources :state_chips, only: :new, param: :combobox_value get "new_html_state_chip", to: "state_chips#new_html" get "new_dismissing_state_chip", to: "state_chips#new_dismissing" - resources :users, only: :update + get "new_possibly_new_state_chip", to: "state_chips#new_possibly_new" + + resources :users, only: :update do + resources :visits, only: :create + end root to: "comboboxes#plain" end diff --git a/test/helpers/hotwire_combobox/helper_test.rb b/test/helpers/hotwire_combobox/helper_test.rb index e93223b..80ebff4 100644 --- a/test/helpers/hotwire_combobox/helper_test.rb +++ b/test/helpers/hotwire_combobox/helper_test.rb @@ -110,4 +110,14 @@ class HotwireCombobox::HelperTest < ApplicationViewTestCase assert_instance_of String, hw_listbox_id(:bar) assert_equal hw_listbox_id(:bar), HotwireCombobox::Component.new(self, :foo, id: :bar).listbox_attrs[:id] end + + test "name_when_new must match original name on multiselects" do + assert_raises ActiveModel::ValidationError do + combobox_tag :foo, multiselect_chip_src: "some_path", name_when_new: :bar + end + + assert_nothing_raised do + combobox_tag :foo, multiselect_chip_src: "some_path", name_when_new: :foo + end + end end diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index a65dcf2..f1b9bf1 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -1030,6 +1030,44 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_combobox_display "#user_visited_state_ids", %w[ Illinois ] end + test "multiselect with new options" do + visit multiselect_new_values_path + + open_combobox "#states-field" + + click_on_option "Alabama" + assert_chip_with text: "Alabama" + type_in_combobox "#states-field", "Newplace", :enter + assert_chip_with text: "Newplace" + + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama Newplace ], + [ states(:alabama).id, "Newplace" ] + + remove_chip "Newplace" + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama ], + [ states(:alabama).id ] + + type_in_combobox "#states-field", "New,place", :enter # comma is stripped away + click_away + + assert_combobox_display_and_value \ + "#states-field", + %w[ Alabama Newplace ], + [ states(:alabama).id, "Newplace" ] + + assert_difference -> { State.count }, +1 do + find("input[type=submit]").click + assert_text "Visits created" + end + + assert_equal "Newplace", State.unscoped.last.name + assert_equal %w[ Alabama Newplace ], User.first.visited_states.map(&:name) + end + private def open_combobox(selector) find(selector).click