The point of issues: a wrong handling of exposed filters with multi values and with defaulkt values

CommentFileSizeAuthor
#12 interdiff-3083093-11-12.diff1.18 KBcolan
#12 views_autocomplete_filters-handle_dependent_filters-3083093-12.patch4.17 KBcolan
#11 views_autocomplete_filters-3083093-11.patch4.08 KBcolan
#11 interdiff-3083093-9-11.diff3.53 KBcolan
#11 interdiff-3083093-6-9.diff898 bytescolan
#9 views_autocomplete_filters-autocomplete_default_filters-3083093-9-D8.patch2.06 KBnikita_tt
#6 autocomplete-dependent-3083093-6.patch1.7 KBuzlov
#2 autocomplete-dependent-3083093-1.patch3.23 KBuzlov

Comments

uzlov created an issue. See original summary.

uzlov’s picture

uzlov commented
StatusFileSize
new autocomplete-dependent-3083093-1.patch3.23 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 8.x-1.2 » 8.x-1.x-dev
Status: Needs review » Needs work 
  1. +++ b/src/Controller/ViewsAutocompleteFiltersController.php
@@ -100,7 +100,7 @@ class ViewsAutocompleteFiltersController implements ContainerInjectionInterface
-  
+

@@ -200,7 +207,7 @@ class ViewsAutocompleteFiltersController implements ContainerInjectionInterface
-  
+

@@ -210,7 +217,7 @@ class ViewsAutocompleteFiltersController implements ContainerInjectionInterface
-  
+

@@ -231,7 +238,7 @@ class ViewsAutocompleteFiltersController implements ContainerInjectionInterface
-  
+

    better keep as is 

  2. +++ b/src/Controller/ViewsAutocompleteFiltersController.php
@@ -172,6 +172,13 @@ class ViewsAutocompleteFiltersController implements ContainerInjectionInterface
+      // Need to reset the default values for exposed filters.

    better to tell why

uzlov’s picture

uzlov commented

1. it's just empty spaces (I gess, beetter do this cleanup, no?)
2. the answer in comment and code before

    // Collect exposed filter values and set them to the view.
    if (!empty($expose_options['autocomplete_dependent'])) {
      $exposed_input = $view->getExposedInput() ;
    }
    else {
      $exposed_input = [];
...

If disabled option "autocomplete_dependent" (Suggestions depend on other filter fields), it means need to reset possible default falues for exposed filters.

uzlov’s picture

uzlov commented
StatusFileSize
new autocomplete-dependent-3083093-6.patch1.7 KB

Restored the empty spaces.

uzlov’s picture

uzlov commented
Status: Needs work » Needs review
andypost’s picture

andypost
he/him
Primary language Russian
commented
Assigned: uzlov » Unassigned
Status: Needs review » Reviewed & tested by the community
nikita_tt’s picture

nikita_tt
Primary language Ukrainian
commented
StatusFileSize
new views_autocomplete_filters-autocomplete_default_filters-3083093-9-D8.patch2.06 KB

Thanks for the patch. I added a few lines for grouped filters with a default value.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/js/views-autocomplete-filters-dependent.js
@@ -149,7 +149,13 @@
+            if(!data_string.data[value['name']]) data_string.data[value['name']] = []

Missing whitespace after `if`.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-3083093-6-9.diff898 bytes
new interdiff-3083093-9-11.diff3.53 KB
new views_autocomplete_filters-3083093-11.patch4.08 KB

Actually, ViewsAutocompleteFiltersController->autocomplete() is way too long already. Let's not make it worse.

Also, here's the missing interdiff from #9.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
StatusFileSize
new views_autocomplete_filters-handle_dependent_filters-3083093-12.patch4.17 KB
new interdiff-3083093-11-12.diff1.18 KB

Sorry; I forgot to pass a variable into the new method.

vasike’s picture

vasike
Location Ramnicu Valcea
commented
Related issues: +#3117099: Incompatibility with search autocomplete module, +#2986201: "Add some fields to view" option is always empty

It seems there could be some related issues and their patches ...

So added as related issues to be taken into account ...

vasike’s picture

vasike
Location Ramnicu Valcea
commented
Status: Needs review » Needs work

i put it back in "Needs work" to make sure about related issues

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Status: Needs work » Needs review

Please don't create deadlock by setting a collection of independent issues to NW.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented

  • colan committed ee8bc06 on 8.x-1.x authored by uzlov 
    Issue #3083093 by colan, uzlov, nikita_tt: Added correct handling for...
colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Status: Needs review » Fixed

Thanks all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.