Feat 1065 filter persistance on asset index #1211
Conversation
|
|
||
| function handleValueChange(value: string) { | ||
| setSearchParams((prev) => { | ||
| /** If the value is "ALL", we just remove the param */ | ||
| if (value === "ALL") { | ||
| destoryCookieValues(["status"]); |
There was a problem hiding this comment.
I am a bit concerned about this approach, because that requires us to never forget to add this when having functions that clear filters and so on.
A suggestion, what if we create our own abstraction of useSearchParams() that handles this and makes sure that every time a value is removed from the search params, we also remove it from the cookie.
There was a problem hiding this comment.
I could not able to figure out a way to do it due to some scenarios.
- Not able to differentiate if the user has cleared the filters in the asset page or if the user is directly going to the asset page after some navigations. (in both cases search parameters are empty.)
There was a problem hiding this comment.
If the user has search params saved in the cookie, they wont be empty as they come with the server response and useSearchParams checks on the client so I am not sure what you mean here.
…com:rajeshj11/shelf.nu into feat-1065-filter-persistance-on-asset-index
…com:rajeshj11/shelf.nu into feat-1065-filter-persistance-on-asset-index
There was a problem hiding this comment.
Okey we still have quite some stuff to handle here. A few of the filters are currently broken because of this change. For example:
- manually clearing an item from the dropdown filters doesnt work anymore. I assume its because of what i mentioned before, that if we don't manage the
destroyCookieValues(["s"]);somehow globally, it will create sitiations like this where we forgot to add it in 1 place and it causes a bug.
So I did something to help out. I created the abstraction of the setSearchParams that automatically removed search params values that were removed from the cookie.
However this still needs to be further tested and the import of useSearchParams needs to be replaced. Can you please take care of that.
I also placed one inline comment to address.
ok will check |
…om useSreachParams hook for handling the search params deleting and updation
…rited in useSearchParams
No description provided.