refactor(picker-starter): place query and filters logic directly into the picker.config.ts file #91
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
picker-starter/src/picker.config.ts
Outdated
| label: 'Categories', | ||
| name: 'categoryMulti', | ||
| defaultValue: [], | ||
| options: categoryMockAssets.map((category) => ({ |
There was a problem hiding this comment.
what if categoryMockAssets is just categories? like items above.
Or we could name them something like:
sampleItemssampleCategories
"mock" feels like unit testing to me.
picker-starter/src/picker.config.ts
Outdated
| const filteredItems = items | ||
| .filter(matchCategories(filterSelection['categoryMulti'] as string[])) | ||
| .filter(matchItem(searchTerm)) |
There was a problem hiding this comment.
matchItem sounds a bit unintuitive to me. It can be something like matchSearchTerm...?
Or we could rename them altogether like:
const filteredItems = items
.filter(filterByCategories(filterSelection['categoryMulti'] as string[]))
.filter(filterBySearchTerm(searchTerm))What do you think? 🤔
There was a problem hiding this comment.
matchItem sounds a bit unintuitive to me. It can be something like matchSearchTerm...?
Yeah, you're totally right, matchSearchTerm is much better.
About the filterByCategories and filterBySearchTerm renaming, I'm not too sure.
These names suggest to me this function is going to perform the filtering action while it doesn't.
I don't know... somehow using match... was more meaningful to me.
What do you think, @eunjae-lee?
There was a problem hiding this comment.
okay, then it sounds good. let's merge!
There was a problem hiding this comment.
Thanks for you feedback, @eunjae-lee.
Great suggestions 💯
I've applied them already ✅
picker-starter/src/picker.config.ts
Outdated
| const filteredItems = items | ||
| .filter(matchCategories(filterSelection['categoryMulti'] as string[])) | ||
| .filter(matchItem(searchTerm)) |
There was a problem hiding this comment.
matchItem sounds a bit unintuitive to me. It can be something like matchSearchTerm...?
Yeah, you're totally right, matchSearchTerm is much better.
About the filterByCategories and filterBySearchTerm renaming, I'm not too sure.
These names suggest to me this function is going to perform the filtering action while it doesn't.
I don't know... somehow using match... was more meaningful to me.
What do you think, @eunjae-lee?
demetriusfeijoo
deleted the
EXT-2135-place-query-and-query-options-logic-directly-into-picker-config-ts-file
branch
Issue: EXT-2135
What?
This PR places all query and filter logic directly into the picker.config.ts file
Why?
The idea of this change is to provide a better experience for developers using this starter by placing the
core logicfor fetchingitemsandfiltersdirectly into the main file of this project which is thepicker.config.tsfile.