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

fix(utilities): move findOrDefault to a function #29

Merged
merged 3 commits into from
Mar 11, 2018

Conversation

jmzagorski
Copy link
Collaborator

Having it on the Array.prototype causes issues with jquery extend and overall is risky. Also, and I am not sure how I missed this, but I was still using the filter property in the collectionFormatter. So i fixed that to make sure this new findOrDefault works.

closes #27

having it on the Array.prototype causes issues with jquery extend and
overall is risky

closes #27
@ghiscoding
Copy link
Owner

I think you missed this filter.collection as well, though it's just a comment.

I was wondering what does the collectionFormatter does, so I went in the index.ts to see the description. Doesn't this look a lot like the csvFormatter that I made? At first glance, the description doesn't tell me the difference between the 2. Oh wait, actually nevermind, it takes the collection from the params, not from the value, that is the difference I guess.

@jmzagorski
Copy link
Collaborator Author

the collectionFormatter is a lookup since the structure of the collection is { value: .., label... }. For example if you had a collection like this [{ value: 1, label: 'foo'}, {value: 2, label: 'bar' }] and your dataset item on the grid had [{ value: 1 },{ value: 2 }] then the grid would display foo and bar and not 1 and 2

@ghiscoding
Copy link
Owner

Oh that is nice then, would you mind putting that example within the comment description of that formatter. I think it would be useful

@ghiscoding
Copy link
Owner

I've put all the description within the index.ts because those are the ones that show up with intellisense

@jmzagorski
Copy link
Collaborator Author

I really need to get some form on intellisense or popup in my vim editor so I can see these jsdocs too. Not sure if what I just committed will look as you want it in your editor.

@ghiscoding
Copy link
Owner

ghiscoding commented Mar 11, 2018

You might want to try Vim for VSCode extension, you could gain a lot from it without sacrificing your shortcuts and keys. Obviously the first thing you would gain is intellisense, which is 1 of the top 3 biggest reasons that I like TypeScript.

Side note, I would also be happy with a 1 liner example in the intellisense jsdoc, just like we do in error thrown. It just helps to see a quick example sometime.

Here's the intellisense for the previous code that was in place.

code - insiders_2018-03-11_17-11-35

@ghiscoding ghiscoding merged commit 3d21548 into master Mar 11, 2018
@jmzagorski
Copy link
Collaborator Author

I used VS Code and VIM for VSCode a little over a year ago. I liked it a lot, but when I started getting more comfortable with VIM I found some quirks so I just rid of VSCode. I will use VsVim in Visual Studio every now and then since I need it for work, but I find myself always going back to vim. VIM probably has something out there for typescript that I need to go find now that I am developing more in that language. Most of my plugins are javascript/c#.

@jmzagorski jmzagorski deleted the bugfix/array_findOrDefault branch March 19, 2018 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: findOrDefault shows in mergeGridOptions which uses jQuery extend
2 participants