From d640ab8428591d0da7544eaeed98af8edf1dc104 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 15 Jul 2024 11:10:34 +0200 Subject: [PATCH 1/2] fix(combobox): dispatch change event on multiple combo changes --- app/javascript/components/ComboBox.tsx | 23 +++++++--- app/javascript/components/react-aria/hooks.ts | 43 +++++++++++++++---- .../champs/multiple_drop_down_list_champ.rb | 2 +- .../multiple_drop_down_list_champ_spec.rb | 6 ++- 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/app/javascript/components/ComboBox.tsx b/app/javascript/components/ComboBox.tsx index 63dffa015..10d184497 100644 --- a/app/javascript/components/ComboBox.tsx +++ b/app/javascript/components/ComboBox.tsx @@ -147,6 +147,7 @@ export function MultiComboBox(maybeProps: MultiComboBoxProps) { formValue, allowsCustomValue, valueSeparator, + className, ...props } = useMemo(() => s.create(maybeProps, MultiComboBoxProps), [maybeProps]); @@ -174,7 +175,7 @@ export function MultiComboBox(maybeProps: MultiComboBoxProps) { const formResetRef = useOnFormReset(onReset); return ( -
+
{selectedItems.length > 0 ? ( @@ -203,16 +204,26 @@ export function MultiComboBox(maybeProps: MultiComboBoxProps) { {name ? ( - {hiddenInputValues.map((value, i) => ( + {hiddenInputValues.length == 0 ? ( - ))} + ) : ( + hiddenInputValues.map((value, i) => ( + + )) + )} ) : null}
diff --git a/app/javascript/components/react-aria/hooks.ts b/app/javascript/components/react-aria/hooks.ts index ede4825b6..df940e0f0 100644 --- a/app/javascript/components/react-aria/hooks.ts +++ b/app/javascript/components/react-aria/hooks.ts @@ -23,6 +23,7 @@ export interface ComboBoxProps } const inputMap = new WeakMap(); +const inputCountMap = new WeakMap(); export function useDispatchChangeEvent() { const ref = useRef(null); @@ -30,12 +31,15 @@ export function useDispatchChangeEvent() { ref, dispatch: () => { requestAnimationFrame(() => { - const input = ref.current?.querySelector('input'); - if (input) { - const value = input.value; - const prevValue = inputMap.get(input) || ''; - if (value != prevValue) { - inputMap.set(input, value); + if (ref.current) { + const container = ref.current; + const inputs = Array.from(container.querySelectorAll('input')); + const input = inputs.at(0); + if (input && inputChanged(container, inputs)) { + inputCountMap.set(container, inputs.length); + for (const input of inputs) { + inputMap.set(input, input.value.trim()); + } input.dispatchEvent(new Event('change', { bubbles: true })); } } @@ -44,6 +48,23 @@ export function useDispatchChangeEvent() { }; } +// I am not proude of this code. We have to tack values and number of values to deal with multi select combobox. +// I have a plan to remove this code. Soon. +function inputChanged(container: HTMLSpanElement, inputs: HTMLInputElement[]) { + const prevCount = inputCountMap.get(container) ?? 0; + if (prevCount != inputs.length) { + return true; + } + for (const input of inputs) { + const value = input.value.trim(); + const prevValue = inputMap.get(input); + if (prevValue == null || prevValue != value) { + return true; + } + } + return false; +} + export function useSingleList({ defaultItems, defaultSelectedKey, @@ -174,9 +195,13 @@ export function useMultiList({ const filteredItems = useMemo( () => inputValue.length == 0 - ? items - : matchSorter(items, inputValue, { keys: ['label'] }), - [items, inputValue] + ? items.filter((item) => !selectedKeys.has(item.value)) + : matchSorter( + items.filter((item) => !selectedKeys.has(item.value)), + inputValue, + { keys: ['label'] } + ), + [items, inputValue, selectedKeys] ); const selectedItems = useMemo(() => { const selectedItems: Item[] = []; diff --git a/app/models/champs/multiple_drop_down_list_champ.rb b/app/models/champs/multiple_drop_down_list_champ.rb index cd55dbc0c..46c2d5d2d 100644 --- a/app/models/champs/multiple_drop_down_list_champ.rb +++ b/app/models/champs/multiple_drop_down_list_champ.rb @@ -73,7 +73,7 @@ class Champs::MultipleDropDownListChamp < Champ end def value=(value) - return super(nil) if value.nil? + return super(nil) if value.blank? values = if value.is_a?(Array) value diff --git a/spec/models/champs/multiple_drop_down_list_champ_spec.rb b/spec/models/champs/multiple_drop_down_list_champ_spec.rb index 744637431..407347098 100644 --- a/spec/models/champs/multiple_drop_down_list_champ_spec.rb +++ b/spec/models/champs/multiple_drop_down_list_champ_spec.rb @@ -41,8 +41,6 @@ describe Champs::MultipleDropDownListChamp do expect(champ.value).to eq("[\"val1\"]") champ.value = 'val2' expect(champ.value).to eq("[\"val1\",\"val2\"]") - champ.value = '' - expect(champ.value).to eq("[\"val1\",\"val2\"]") champ.value = "[brackets] val4" expect(champ.value).to eq("[\"val1\",\"val2\",\"[brackets] val4\"]") champ.value = nil @@ -51,6 +49,10 @@ describe Champs::MultipleDropDownListChamp do expect(champ.value).to eq("[\"val1\"]") champ.value = [] expect(champ.value).to be_nil + champ.value = ["val1"] + expect(champ.value).to eq("[\"val1\"]") + champ.value = '' + expect(champ.value).to be_nil } end end From 888be8df89c69d87f043f44bf1a15e5cb51a98f9 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 15 Jul 2024 18:41:20 +0200 Subject: [PATCH 2/2] feat(champs): use new combobox for champs selection multiple --- .../drop_down_list_component.rb | 4 +++- .../multiple_drop_down_list_component.rb | 11 ++++++++-- ...ultiple_drop_down_list_component.html.haml | 10 ++------- app/controllers/champs/options_controller.rb | 10 --------- .../champs/options_controller_spec.rb | 22 ------------------- spec/system/users/brouillon_spec.rb | 13 +++++++---- .../shared/dossiers/_edit.html.haml_spec.rb | 2 +- 7 files changed, 24 insertions(+), 48 deletions(-) delete mode 100644 app/controllers/champs/options_controller.rb delete mode 100644 spec/controllers/champs/options_controller_spec.rb diff --git a/app/components/editable_champ/drop_down_list_component.rb b/app/components/editable_champ/drop_down_list_component.rb index 04f901550..ec5fd536e 100644 --- a/app/components/editable_champ/drop_down_list_component.rb +++ b/app/components/editable_champ/drop_down_list_component.rb @@ -30,6 +30,8 @@ class EditableChamp::DropDownListComponent < EditableChamp::EditableChampBaseCom name: @form.field_name(:value), selected_key: @champ.selected, items: @champ.enabled_non_empty_options(other: true).map { _1.is_a?(Array) ? _1 : [_1, _1] }, - empty_filter_key: @champ.drop_down_other? ? Champs::DropDownListChamp::OTHER : nil) + empty_filter_key: @champ.drop_down_other? ? Champs::DropDownListChamp::OTHER : nil, + 'aria-describedby': @champ.describedby_id, + 'aria-labelledby': @champ.labelledby_id) end end diff --git a/app/components/editable_champ/multiple_drop_down_list_component.rb b/app/components/editable_champ/multiple_drop_down_list_component.rb index 55e3d4ba4..5c6265e14 100644 --- a/app/components/editable_champ/multiple_drop_down_list_component.rb +++ b/app/components/editable_champ/multiple_drop_down_list_component.rb @@ -9,7 +9,14 @@ class EditableChamp::MultipleDropDownListComponent < EditableChamp::EditableCham @champ.render_as_checkboxes? ? :fieldset : :div end - def update_path(option) - champs_options_path(@champ.dossier, @champ.stable_id, row_id: @champ.row_id, option:) + def react_props + react_input_opts(id: @champ.input_id, + class: 'fr-mt-1w', + name: @form.field_name(:value, multiple: true), + selected_keys: @champ.selected_options, + items: @champ.enabled_non_empty_options, + 'aria-label': @champ.libelle, + 'aria-describedby': @champ.describedby_id, + 'aria-labelledby': @champ.labelledby_id) end end diff --git a/app/components/editable_champ/multiple_drop_down_list_component/multiple_drop_down_list_component.html.haml b/app/components/editable_champ/multiple_drop_down_list_component/multiple_drop_down_list_component.html.haml index 609e0d690..65136be64 100644 --- a/app/components/editable_champ/multiple_drop_down_list_component/multiple_drop_down_list_component.html.haml +++ b/app/components/editable_champ/multiple_drop_down_list_component/multiple_drop_down_list_component.html.haml @@ -9,11 +9,5 @@ = b.text - else - %div{ 'data-turbo-focus-group': true } - - if @champ.selected_options.present? - .fr-mb-2w.fr-mt-2w{ "data-turbo": "true" } - - @champ.selected_options.each do |option| - = render NestedForms::OwnedButtonComponent.new(formaction: update_path(option), http_method: :delete, opt: { aria: {pressed: true }, class: 'fr-tag fr-tag-bug fr-mb-1w fr-mr-1w', id: @champ.checkbox_id(option) }) do - = option - - if @champ.unselected_options.present? - = @form.select :value, @champ.unselected_options, { selected: '', include_blank: false, prompt: t('.prompt') }, id: @champ.input_id, aria: { describedby: @champ.describedby_id }, class: 'fr-select fr-mt-2v' + %react-fragment + = render ReactComponent.new "ComboBox/MultiComboBox", **react_props diff --git a/app/controllers/champs/options_controller.rb b/app/controllers/champs/options_controller.rb deleted file mode 100644 index 61e53e05c..000000000 --- a/app/controllers/champs/options_controller.rb +++ /dev/null @@ -1,10 +0,0 @@ -class Champs::OptionsController < Champs::ChampController - include TurboChampsConcern - - def remove - @champ.remove_option([params[:option]].compact, true) - @dossier = @champ.private? ? nil : @champ.dossier - champs_attributes = { @champ.public_id => {} } - @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_attributes, @champ.dossier.champs) - end -end diff --git a/spec/controllers/champs/options_controller_spec.rb b/spec/controllers/champs/options_controller_spec.rb deleted file mode 100644 index ca24b6805..000000000 --- a/spec/controllers/champs/options_controller_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -describe Champs::OptionsController, type: :controller do - let(:user) { create(:user) } - let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :multiple_drop_down_list }]) } - - describe '#remove' do - let(:dossier) { create(:dossier, user:, procedure:) } - let(:champ) { dossier.champs.first } - - before { - sign_in user - champ.update(value: ['toto', 'tata'].to_json) - } - - context 'with stable_id' do - subject { delete :remove, params: { dossier_id: dossier, stable_id: champ.stable_id, option: 'tata' }, format: :turbo_stream } - - it 'remove option' do - expect { subject }.to change { champ.reload.selected_options.size }.from(2).to(1) - end - end - end -end diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index 05bc3aabc..d1c3e9b71 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -34,8 +34,13 @@ describe 'The user' do find('.fr-checkbox-group label', text: 'val1').click find('.fr-checkbox-group label', text: 'val3').click select('bravo', from: form_id_for('simple_choice_drop_down_list_long')) - select('alpha', from: form_id_for('multiple_choice_drop_down_list_long')) - select('charly', from: form_id_for('multiple_choice_drop_down_list_long')) + + scroll_to(find_field('multiple_choice_drop_down_list_long'), align: :center) + fill_in('multiple_choice_drop_down_list_long', with: 'alpha') + find('.fr-menu__item', text: 'alpha').click + fill_in('multiple_choice_drop_down_list_long', with: 'charly') + find('.fr-menu__item', text: 'charly').click + wait_until { champ_value_for('multiple_choice_drop_down_list_long') == ['alpha', 'charly'].to_json } select('Australie', from: form_id_for('pays')) select('Martinique', from: form_id_for('regions')) @@ -109,8 +114,8 @@ describe 'The user' do expect(page).to have_selected_value('regions', selected: 'Martinique') expect(page).to have_selected_value('departements', selected: '02 – Aisne') within("##{champ_for('multiple_choice_drop_down_list_long').input_group_id}") do - expect(page).to have_button('alpha') - expect(page).to have_button('charly') + expect(page).to have_text('alpha') + expect(page).to have_text('charly') end expect(page).to have_field('communes', with: 'Brétigny (60400)') expect(page).to have_selected_value('pays', selected: 'Australie') diff --git a/spec/views/shared/dossiers/_edit.html.haml_spec.rb b/spec/views/shared/dossiers/_edit.html.haml_spec.rb index f6ce8f5bf..931889088 100644 --- a/spec/views/shared/dossiers/_edit.html.haml_spec.rb +++ b/spec/views/shared/dossiers/_edit.html.haml_spec.rb @@ -103,7 +103,7 @@ describe 'shared/dossiers/edit', type: :view do let(:options) { ['peach', 'banana', 'pear', 'apricot', 'apple', 'grapefruit'] } it 'renders the list as a multiple-selection dropdown' do - expect(subject).to have_selector('select') + expect(subject).to have_selector('react-fragment > react-component[name="ComboBox/MultiComboBox"]') end end end