From 0560c1b343e7128adea90391dbe69c898e6b8027 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Wed, 28 Aug 2024 13:52:51 +0300 Subject: [PATCH 1/3] [#57503] Time based filters like "Created on" don't trigger a result update while typing https://community.openproject.org/work_packages/57503 --- .../dynamic/filter/filters-form.controller.ts | 66 +++++++------------ 1 file changed, 22 insertions(+), 44 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts index 9bcf84db9ca7..bfb77a1b4515 100644 --- a/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts @@ -74,6 +74,8 @@ export default class FiltersFormController extends Controller { declare readonly singleDayTargets:HTMLInputElement[]; declare readonly simpleValueTargets:HTMLInputElement[]; + autoReloadTargets:HTMLElement[]; + static values = { displayFilters: { type: Boolean, default: false }, outputFormat: { type: String, default: 'params' }, @@ -89,6 +91,14 @@ export default class FiltersFormController extends Controller { initialize() { // Initialize runs anytime an element with a controller connected to the DOM for the first time this.sendForm = debounce(this.sendForm.bind(this), 300); + this.autoReloadTargets = [ + ...this.simpleValueTargets, + ...this.operatorTargets, + ...this.filterValueContainerTargets, + ...this.filterValueSelectTargets, + ...this.daysTargets, + ...this.singleDayTargets, + ]; } connect() { @@ -101,28 +111,12 @@ export default class FiltersFormController extends Controller { // Auto-register change event listeners for all fields // to keep DOM cleaner. if (this.performTurboRequestsValue) { - this.simpleValueTargets.forEach((simpleValue) => { - simpleValue.addEventListener('input', this.sendForm.bind(this)); - }); - - this.operatorTargets.forEach((operator) => { - operator.addEventListener('change', this.sendForm.bind(this)); - }); - - this.filterValueSelectTargets.forEach((select) => { - select.addEventListener('change', this.sendForm.bind(this)); - }); - - this.filterValueContainerTargets.forEach((container) => { - container.addEventListener('change', this.sendForm.bind(this)); - }); - - this.singleDayTargets.forEach((singleDay) => { - singleDay.addEventListener('change', this.sendForm.bind(this)); - }); - - this.daysTargets.forEach((days) => { - days.addEventListener('change', this.sendForm.bind(this)); + this.autoReloadTargets.forEach((target) => { + if (target instanceof HTMLInputElement) { + target.addEventListener('input', this.sendForm.bind(this)); + } else { + target.addEventListener('change', this.sendForm.bind(this)); + } }); } } @@ -134,28 +128,12 @@ export default class FiltersFormController extends Controller { // Auto-deregister change event listeners for all fields // to keep DOM cleaner. if (this.performTurboRequestsValue) { - this.simpleValueTargets.forEach((simpleValue) => { - simpleValue.removeEventListener('change', this.sendForm.bind(this)); - }); - - this.operatorTargets.forEach((operator) => { - operator.removeEventListener('change', this.sendForm.bind(this)); - }); - - this.filterValueSelectTargets.forEach((select) => { - select.removeEventListener('change', this.sendForm.bind(this)); - }); - - this.filterValueContainerTargets.forEach((container) => { - container.removeEventListener('change', this.sendForm.bind(this)); - }); - - this.singleDayTargets.forEach((singleDay) => { - singleDay.removeEventListener('change', this.sendForm.bind(this)); - }); - - this.daysTargets.forEach((days) => { - days.removeEventListener('change', this.sendForm.bind(this)); + this.autoReloadTargets.forEach((target) => { + if (target instanceof HTMLInputElement) { + target.removeEventListener('input', this.sendForm.bind(this)); + } else { + target.removeEventListener('change', this.sendForm.bind(this)); + } }); } } From f2be800d7da624ab87e36bd647f1d9d1c8754156 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Sun, 1 Sep 2024 18:42:52 +0300 Subject: [PATCH 2/3] Refactor target finding logic --- .../dynamic/filter/filters-form.controller.ts | 77 ++++++++++++------- 1 file changed, 50 insertions(+), 27 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts index bfb77a1b4515..880ffe72a279 100644 --- a/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts @@ -160,9 +160,9 @@ export default class FiltersFormController extends Controller { } toggleMultiSelect({ params: { filterName } }:{ params:{ filterName:string } }) { - const valueContainer = this.filterValueContainerTargets.find((filterValueContainer) => filterValueContainer.getAttribute('data-filter-name') === filterName); - const singleSelect = this.filterValueSelectTargets.find((selectField) => !selectField.multiple && selectField.getAttribute('data-filter-name') === filterName); - const multiSelect = this.filterValueSelectTargets.find((selectField) => selectField.multiple && selectField.getAttribute('data-filter-name') === filterName); + const valueContainer = this.findTargetByName(filterName, this.filterValueContainerTargets); + const singleSelect = this.findTargetByName(filterName, this.filterValueSelectTargets, (selectField) => !selectField.multiple); + const multiSelect = this.findTargetByName(filterName, this.filterValueSelectTargets, (selectField) => selectField.multiple); if (valueContainer && singleSelect && multiSelect) { if (valueContainer.classList.contains('multi-value')) { const valueToSelect = this.getValueToSelect(multiSelect); @@ -186,13 +186,13 @@ export default class FiltersFormController extends Controller { } addFilter(event:Event) { - const selectedFilterName = (event.target as HTMLSelectElement).value; - const selectedFilter = this.filterTargets.find((filter) => filter.getAttribute('data-filter-name') === selectedFilterName); + const filterName = (event.target as HTMLSelectElement).value; + const selectedFilter = this.findTargetByName(filterName, this.filterTargets); if (selectedFilter) { selectedFilter.classList.remove('hidden'); } - this.disableSelection(); - this.reselectPlaceholderOption(); + this.addFilterSelectTarget.selectedOptions[0].disabled = true; + this.addFilterSelectTarget.selectedIndex = 0; this.setSpacerVisibility(); if (this.performTurboRequestsValue) { @@ -200,16 +200,8 @@ export default class FiltersFormController extends Controller { } } - private disableSelection() { - this.addFilterSelectTarget.selectedOptions[0].setAttribute('disabled', 'disabled'); - } - - private reselectPlaceholderOption() { - this.addFilterSelectTarget.options[0].setAttribute('selected', 'selected'); - } - removeFilter({ params: { filterName } }:{ params:{ filterName:string } }) { - const filterToRemove = this.filterTargets.find((filter) => filter.getAttribute('data-filter-name') === filterName); + const filterToRemove = this.findTargetByName(filterName, this.filterTargets); filterToRemove?.classList.add('hidden'); const selectOptions = Array.from(this.addFilterSelectTarget.options); @@ -256,7 +248,7 @@ export default class FiltersFormController extends Controller { setValueVisibility({ target, params: { filterName } }:{ target:HTMLSelectElement, params:{ filterName:string } }) { const selectedOperator = target.value; - const valueContainer = this.filterValueContainerTargets.find((filterValueContainer) => filterValueContainer.getAttribute('data-filter-name') === filterName); + const valueContainer = this.findTargetByName(filterName, this.filterValueContainerTargets); if (valueContainer) { if (this.noValueOperators.includes(selectedOperator)) { valueContainer.classList.add('hidden'); @@ -360,10 +352,10 @@ export default class FiltersFormController extends Controller { const filters:InternalFilterValue[] = []; advancedFilters.forEach((filter) => { - const filterName = filter.getAttribute('data-filter-name'); + const filterName = filter.getAttribute('data-filter-name') as string; const filterType = filter.getAttribute('data-filter-type'); - const parsedOperator = this.operatorTargets.find((operator) => operator.getAttribute('data-filter-name') === filterName)?.value; - const valueContainer = this.filterValueContainerTargets.find((filterValueContainer) => filterValueContainer.getAttribute('data-filter-name') === filterName); + const parsedOperator = this.findTargetByName(filterName, this.operatorTargets)?.value; + const valueContainer = this.findTargetByName(filterName, this.filterValueContainerTargets); if (valueContainer && filterName && filterType && parsedOperator) { const parsedValue = this.parseFilterValue(valueContainer, filterName, filterType, parsedOperator) as string[]|null; @@ -404,13 +396,12 @@ export default class FiltersFormController extends Controller { private parseFilterValue(valueContainer:HTMLElement, filterName:string, filterType:string, operator:string) { const checkbox = valueContainer.querySelector('input[type="checkbox"]') as HTMLInputElement; - const isAutocomplete = valueContainer.dataset.filterAutocomplete === 'true'; if (checkbox) { return [checkbox.checked ? 't' : 'f']; } - if (isAutocomplete) { + if (valueContainer.dataset.filterAutocomplete === 'true') { return (valueContainer.querySelector('input[name="value"]') as HTMLInputElement)?.value.split(','); } @@ -426,7 +417,7 @@ export default class FiltersFormController extends Controller { return this.parseDateFilterValue(valueContainer, filterName); } - const value = this.simpleValueTargets.find((simpleValueInput) => simpleValueInput.getAttribute('data-filter-name') === filterName)?.value; + const value = this.findTargetByName(filterName, this.simpleValueTargets)?.value; if (value && value.length > 0) { return [value]; @@ -456,16 +447,16 @@ export default class FiltersFormController extends Controller { let value; if (valueContainer.classList.contains('days')) { - const dateValue = this.daysTargets.find((daysField) => daysField.getAttribute('data-filter-name') === filterName)?.value; + const dateValue = this.findTargetByName(filterName, this.daysTargets)?.value; value = _.without([dateValue], ''); } else if (valueContainer.classList.contains('on-date')) { - const dateValue = this.singleDayTargets.find((dateInput) => dateInput.id === `on-date-value-${filterName}`)?.value; + const dateValue = this.findTargetById(`on-date-value-${filterName}`, this.singleDayTargets)?.value; value = _.without([dateValue], ''); } else if (valueContainer.classList.contains('between-dates')) { - const fromValue = this.singleDayTargets.find((dateInput) => dateInput.id === `between-dates-from-value-${filterName}`)?.value; - const toValue = this.singleDayTargets.find((dateInput) => dateInput.id === `between-dates-to-value-${filterName}`)?.value; + const fromValue = this.findTargetById(`between-dates-from-value-${filterName}`, this.singleDayTargets)?.value; + const toValue = this.findTargetById(`between-dates-to-value-${filterName}`, this.singleDayTargets)?.value; value = [fromValue, toValue]; } @@ -474,4 +465,36 @@ export default class FiltersFormController extends Controller { } return null; } + + private findTargetByName( + filterName:string, + targets:T[], + targetFilter?:(target:T) => boolean, + ):T | undefined { + return this.findTargetBy( + filterName, + (target:T) => target.getAttribute('data-filter-name'), + targets, + targetFilter, + ); + } + + private findTargetById( + filterName:string, + targets:T[], + targetFilter?:(target:T) => boolean, + ):T | undefined { + return this.findTargetBy(filterName, (target:T) => target.id, targets, targetFilter); + } + + private findTargetBy( + attributeValue:string, + attributeGetter:(target:T) => string | null, + targets:T[], + targetFilter?:(target:T) => boolean, + ):T | undefined { + return targets.find((target) => { + return attributeGetter(target) === attributeValue && (!targetFilter || targetFilter(target)); + }); + } } From 4f4a229b36267900637a77b998ba1e63417d7a6b Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Mon, 2 Sep 2024 18:13:40 +0300 Subject: [PATCH 3/3] Add specs --- spec/features/projects/projects_index_spec.rb | 35 +++++++++++++ spec/support/pages/projects/index.rb | 51 ++++++++++++------- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/spec/features/projects/projects_index_spec.rb b/spec/features/projects/projects_index_spec.rb index ff395e1a6c93..76426bfee7ee 100644 --- a/spec/features/projects/projects_index_spec.rb +++ b/spec/features/projects/projects_index_spec.rb @@ -507,6 +507,16 @@ def load_and_open_filters(user) projects_page.remove_filter("name_and_identifier") projects_page.expect_projects_listed(project, development_project, public_project) + # Filter on model attribute 'name' triggered by keyboard input event instead of change + projects_page.filter_by_name_and_identifier("Plain", send_keys: true) + wait_for_reload + + projects_page.expect_projects_listed(project) + projects_page.expect_projects_not_listed(development_project, public_project) + + projects_page.remove_filter("name_and_identifier") + projects_page.expect_projects_listed(project, development_project, public_project) + # Filter on model attribute 'identifier' projects_page.filter_by_name_and_identifier("plain-project") wait_for_reload @@ -789,6 +799,18 @@ def load_and_open_filters(user) expect(page).to have_text(project_created_on_today.name) expect(page).to have_no_text(project_created_on_fixed_date.name) + # created on 'less than days ago' triggered by an input event + projects_page.remove_filter("created_at") + projects_page.set_filter("created_at", + "Created on", + "less than days ago", + ["1"], + send_keys: true) + wait_for_reload + + expect(page).to have_text(project_created_on_today.name) + expect(page).to have_no_text(project_created_on_fixed_date.name) + # created on 'more than days ago' projects_page.remove_filter("created_at") @@ -801,6 +823,19 @@ def load_and_open_filters(user) expect(page).to have_text(project_created_on_fixed_date.name) expect(page).to have_no_text(project_created_on_today.name) + # created on 'more than days ago' + projects_page.remove_filter("created_at") + + projects_page.set_filter("created_at", + "Created on", + "more than days ago", + ["1"], + send_keys: true) + wait_for_reload + + expect(page).to have_text(project_created_on_fixed_date.name) + expect(page).to have_no_text(project_created_on_today.name) + # created on 'between' projects_page.remove_filter("created_at") diff --git a/spec/support/pages/projects/index.rb b/spec/support/pages/projects/index.rb index 9f0933c88d73..c5c81901eeea 100644 --- a/spec/support/pages/projects/index.rb +++ b/spec/support/pages/projects/index.rb @@ -207,26 +207,26 @@ def filter_by_membership(value) wait_for_reload end - def filter_by_name_and_identifier(value) - set_name_and_identifier_filter([value]) + def filter_by_name_and_identifier(value, send_keys: false) + set_name_and_identifier_filter([value], send_keys:) wait_for_reload end - def set_filter(name, human_name, human_operator = nil, values = []) + def set_filter(name, human_name, human_operator = nil, values = [], send_keys: false) if name == "name_and_identifier" - set_simple_filter(name, values) + set_simple_filter(name, values, send_keys:) else - set_advanced_filter(name, human_name, human_operator, values) + set_advanced_filter(name, human_name, human_operator, values, send_keys:) end end - def set_simple_filter(_name, values) + def set_simple_filter(_name, values, send_keys: false) return unless values.any? - set_name_and_identifier_filter(values) # This is the only one simple filter at the moment. + set_name_and_identifier_filter(values, send_keys:) # This is the only one simple filter at the moment. end - def set_advanced_filter(name, human_name, human_operator = nil, values = []) + def set_advanced_filter(name, human_name, human_operator = nil, values = [], send_keys: false) select human_name, from: "add_filter_select" selected_filter = page.find("li[data-filter-name='#{name}']") select(human_operator, from: "operator") unless boolean_filter?(name) @@ -238,7 +238,7 @@ def set_advanced_filter(name, human_name, human_operator = nil, values = []) set_toggle_filter(values) elsif name == "created_at" select(human_operator, from: "operator") - set_created_at_filter(human_operator, values) + set_created_at_filter(human_operator, values, send_keys:) elsif /cf_\d+/.match?(name) select(human_operator, from: "operator") set_custom_field_filter(selected_filter, human_operator, values) @@ -269,21 +269,34 @@ def set_toggle_filter(values) end end - def set_name_and_identifier_filter(values) - fill_in "name_and_identifier", with: values.first + def set_name_and_identifier_filter(values, send_keys: false) + if send_keys + find_field("name_and_identifier").send_keys values.first + else + fill_in "name_and_identifier", with: values.first + end end - def set_created_at_filter(human_operator, values) + def set_created_at_filter(human_operator, values, send_keys: false) case human_operator when "on", "less than days ago", "more than days ago", "days ago" - fill_in "value", with: values.first + if send_keys + find_field("value").send_keys values.first + else + fill_in "value", with: values.first + end when "between" - fill_in "from_value", with: values.first - fill_in "to_value", with: values.second + if send_keys + find_field("from_value").send_keysvalues.first + find_field("to_value").send_keys values.second + else + fill_in "from_value", with: values.first + fill_in "to_value", with: values.second + end end end - def set_custom_field_filter(selected_filter, human_operator, values) + def set_custom_field_filter(selected_filter, human_operator, values, send_keys: false) if selected_filter[:"data-filter-type"] == "list_optional" if values.size == 1 value_select = find('.single-select select[name="value"]') @@ -291,7 +304,11 @@ def set_custom_field_filter(selected_filter, human_operator, values) end elsif selected_filter[:"data-filter-type"] == "date" if human_operator == "on" - fill_in "value", with: values.first + if send_keys + find_field("value").send_keys values.first + else + fill_in "value", with: values.first + end end end end