Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] collectionFilterBy documentation #63

Closed
jmzagorski opened this issue May 18, 2018 · 13 comments · Fixed by #66
Closed

[Question] collectionFilterBy documentation #63

jmzagorski opened this issue May 18, 2018 · 13 comments · Fixed by #66
Labels

Comments

@jmzagorski
Copy link
Collaborator

jmzagorski commented May 18, 2018

I'm submitting a feature request

  • Framework Version:
    Current

  • Browser:
    all

  • Language:
    TypeScript X.X

Current behavior:
the collectionFilterBy filters your collection for values not equal to the collectionFilterBy.value when operator is "EQ"

Expected/desired behavior:
Clarification, maybe documentation. I initially thought that if I passed 'Task2' in the collectionFilterBy.value (like in example 3), that the filter by service will filter for 'Task 2', but it seems the service "filters out" task 2 and shows everything else when the operator is "equal". Not sure if anyone else was confused by this, but I stumbled on it. First, i want to make sure this is not a bug and 2 if this is the design, I can add documentation that filter means filter out or exclude this value to potentially help others like me :-)

@ghiscoding
Copy link
Owner

It's the second time you tell me this, so I guess it is still confusing. Anyway when I coded that functionality I wanted to exclude 1 or 2 words from the collection which is why I coded it that way. However the Collection Service shouldn't care or know that it's a dropdown. This feature is not that old and there isn't much doc about it, so you can inverse it to be more common behavior if you wish to

@jmzagorski
Copy link
Collaborator Author

Sorry, probably was not that clear on my other issue with filtering, but this is separate from my other issue on filtering that I closed when I wanted to add functionality to the filter.

I found a way to use your collection service as is (in my other issues I was trying to work around it) and this does not have to do with dropdowns, but the javascript logic in the filter method that the collection service uses

I have no problem with how the service functions right now and I think we should leave it, I just wanted to make sure I was using it correctly.

@ghiscoding
Copy link
Owner

It wasn't related to yesterday's comment, it was about a month ago that you mentioned that. I don't have time to search for where you wrote that though. Since it was the second time you ask, I was thinking that perhaps the behavior is the inverse of what people are used to, and if so then we should recode it to fit usual behavior.

For a sample of how it works, it's really all within the Example 3 and the Single/Multiple Select Editors. There's no doc about that it seems, but we should probably add something in the Wiki - Editors page

@jmzagorski
Copy link
Collaborator Author

Okay, that's my fault, I thought you were relating it to yesterday (I didn't mean for you to search for it either. I've been away from this library while i was working on my server code so I am sure I will repeat myself sometimes :-) )

I saw you mentioned FilterBy/SortBy in the Select Filters page. My pending change is below. I think for now I will add a link to https://github.com/ghiscoding/aurelia-slickgrid/wiki/Select-Filter#collection-filterbysortby in the https://github.com/ghiscoding/aurelia-slickgrid/wiki/Editors#select-editors section if that is okay.

### Collection FilterBy/SortBy
You can also pre-sort or pre-filter the collection given to the multipleSelect/singleSelect Filters. 
Also note that if the `enableTranslateLabel` flag is set to `True`, it will use the translated value 
to filter or sort the collection. 
+++ The supported filters are `equal` and `notEqual`. To filter the `value` out of the collection 
++ use `OperatorType.equal` and to include only that `value` use `OperatorType.notEqual` 

@ghiscoding
Copy link
Owner

That's ok, you can repeat yourself many times if you want :-P

But I still wonder if it wouldn't be better to reverse the behavior in Collection Service. Is there a reason why you are backing up from this possible change?

@jmzagorski
Copy link
Collaborator Author

No reason, it does not matter to me as long as there is documentation to describe to API. I am fine either way.

Also, I will be submitting a PR to add in, notIn and contains to the filterBy logic if i understand the meaning of the OperatorTypes. If you want me to reverse the EQ and NEQ logic, just let me know.

-      if (operator === OperatorType.equal) {
-        filteredCollection = collection.filter((item) => item[property] !== value);
-      } else {
-        filteredCollection = collection.filter((item) => item[property] === value);
-      }
+      switch (operator) {
+       case OperatorType.equal:
+          filteredCollection = collection.filter((item) => item[property] !== value);
+          break;
+       case OperatorType.in:
+          filteredCollection = collection.filter((item) => item[property].indexOf(value) !== -1);
+          break;
+       case OperatorType.notIn:
+         filteredCollection = collection.filter((item) => item[property].indexOf(value) === -1);
+          break;
+        case OperatorType.contains:
+          filteredCollection = collection.filter((item) => value.indexOf(item[property]) !== -1);
+          break;
+        default:
+          filteredCollection = collection.filter((item) => item[property] === value);
+      }

@ghiscoding
Copy link
Owner

ghiscoding commented May 18, 2018

Yes please reverse it, I think it's better to do it now to avoid such confusions by other user. Nonetheless, we should still add a section in the Wiki with sample.

Adding more operator types is nice :)
By seeing all these new types, it makes more sense to reverse the EQ and NotEQ right away

@jmzagorski
Copy link
Collaborator Author

i'll have to finish this tomorrow. For some reason my npm is not downloading new packages on our works network (i deleted node_modules to get a fresh package list). My goal is to finish this so it makes your next release. I already have the changes, but I can't test with the examples since my node_modules folder is incomplete.

@ghiscoding
Copy link
Owner

Not a total rush, I'm planning on releasing a week later actually since I just got Angular-Slickgrid today to work with Multiple Grids in same View and I would like to get that done within the next release if possible. I will put more info in that ticket there later,

I use VPN for work and I get issues with NPM as well. Sometime need proxy setup, sometime it works without, but most of the time I simply need to disconnect from VPN to get it going

@jmzagorski
Copy link
Collaborator Author

yup same here...i will be very interested in multiple grids. My application uses multiple grids right now, but they are readonly (no filtering/sorting).

@ghiscoding
Copy link
Owner

ghiscoding commented May 18, 2018

I will need your help with the Multiple Grids ticket, I don't know how (I could search internet but you would be faster) to make all Services non-Singleton. I spent an entire day trying to figure out why action done on 1st grid was affecting 2nd grid, I was focused too much on the variables and not the Services, even posted a Stack Overflow question to just come back to it today and find out it was Service Singleton issues lol

So you're not in a rush for a release, right? we can postponed for another week?

@jmzagorski
Copy link
Collaborator Author

yes no rush. my app is in beta next week (fingers crossed) so i can point my package.json directly to your master branch instead of npm if i need something

jmzagorski added a commit that referenced this issue May 19, 2018
The default operator 'EQ' was switched to filter for the value instead of excluding it. Also, an explicit check for undefined in the `collectionFilterBy.value` was added in case the value is actual false, 0, blank or null.

New operator type filters:
`in`: supports `collectionFilterBy.property` as a nested array, and if the `collectionFilterBy.value` exists in the nested array, the parent item will NOT be filtered. For example: `collection: [{ foo: ['bar'] }, { foo: ['foo'] }]`, `collectionFilterBy.property: 'foo'`, `collectionFilterBy.value: 'bar'` will return the first item in the collection only
`notIn`: oppostie of `in`
`contains`: assumes the `collectionFilterBy.value` is an array and will check if any of those values exists in the `collectionFilterBy.property`. For example: `collection: [{ foo: 'bar' }, { foo: 'foo' }]`, `collectionFilterBy.property: 'foo'`, `collectionFilterBy.value: [ 'bar', 'foo' ]` will return both items

closes #63
BREAKING CHANGE: Reversing the 'EQ' filter logic. This was done now because the filter feature was new and it follows the javascript `filter` logic (since equal will filter values equal to the value provided)
ghiscoding pushed a commit that referenced this issue May 20, 2018
* feat(filters): add more operator types to collection svc filter

The default operator 'EQ' was switched to filter for the value instead of excluding it. Also, an explicit check for undefined in the `collectionFilterBy.value` was added in case the value is actual false, 0, blank or null.

New operator type filters:
`in`: supports `collectionFilterBy.property` as a nested array, and if the `collectionFilterBy.value` exists in the nested array, the parent item will NOT be filtered. For example: `collection: [{ foo: ['bar'] }, { foo: ['foo'] }]`, `collectionFilterBy.property: 'foo'`, `collectionFilterBy.value: 'bar'` will return the first item in the collection only
`notIn`: oppostie of `in`
`contains`: assumes the `collectionFilterBy.value` is an array and will check if any of those values exists in the `collectionFilterBy.property`. For example: `collection: [{ foo: 'bar' }, { foo: 'foo' }]`, `collectionFilterBy.property: 'foo'`, `collectionFilterBy.value: [ 'bar', 'foo' ]` will return both items

closes #63
BREAKING CHANGE: Reversing the 'EQ' filter logic. This was done now because the filter feature was new and it follows the javascript `filter` logic (since equal will filter values equal to the value provided)

* refactor(example): add example code to Doc and Client-CLI samples
@jmzagorski
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants