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

[Feature request]: Asset index - store searchParams #1065

Closed
DonKoko opened this issue Jun 14, 2024 · 9 comments · Fixed by #1211
Closed

[Feature request]: Asset index - store searchParams #1065

DonKoko opened this issue Jun 14, 2024 · 9 comments · Fixed by #1211

Comments

@DonKoko
Copy link
Contributor

DonKoko commented Jun 14, 2024

Contact Details

No response

Is your feature request related to a problem? Please describe?

When I have some filters active on the asset asset and then I navigate to a different page, I want those filters to persist when I come back to the asset index.

Describe the solution you'd like

Store the searchParams in a localStorage, the question is how do we make this smooth. The reason I ask is because localStorage is only accessible on the client, so it will be a very ugly flickr if we trigger the loader and once we get localStorage params it will reload basically.

Describe alternatives you've considered

No response

Additional context

No response

@DonKoko DonKoko changed the title [Feature request]: Asset index - store searchParams in cookie [Feature request]: Asset index - store searchParams Jun 14, 2024
@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 16, 2024

hey @rphlmr. I know you used that a lot so I wanted to hear your opinion. Could we maybe use the clientLoader to prevent this issue from happening when we are storing this data on the client?

@rphlmr
Copy link
Contributor

rphlmr commented Jul 16, 2024

Hey 👋
If you want to reuse them in the loader when you come back to the page it will not work.
We can't modify the request, even before await serverLoader().

What about a cookie (not necessary a session cookie, a basic one)?
You scope it to the page where it is needed and in the loader you check for searchParams or cookie.

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 17, 2024

@rphlmr yeah I think we might have to go the cookie way. I like the idea of scoping it based on the path of the assets index so we don't send it with each request but only for that url.

@rajeshj11 is this an issue you might want to work on?

@rajeshj11
Copy link
Contributor

@rphlmr yeah I think we might have to go the cookie way. I like the idea of scoping it based on the path of the assets index so we don't send it with each request but only for that url.

@rajeshj11 is this an issue you might want to work on?

yeah, i would like to give it a try.

@rajeshj11
Copy link
Contributor

@DonKoko please assign it to me. I will also check other approaches and will let you know about them before start the development

@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 17, 2024

Thanks @rajeshj11 . I think the cookie approach is still the best as we can handle it on the server this way. Let me know and lets get started on this.

@DonKoko DonKoko linked a pull request Jul 26, 2024 that will close this issue
@DonKoko
Copy link
Contributor Author

DonKoko commented Jul 30, 2024

@jurrejansen @carlosvirreira this is deployed to staging and ready to be tested. Needs to be tested carefully as it might break some filters.
Some things to note:

  • it is scoped per org, so changing orgs is something to test
  • All filters need to be tested

PLS let me know if all is good and we can release it.

@jurrejansen
Copy link

jurrejansen commented Jul 30, 2024

@DonKoko seems to work smoothly, tested all filters (present on asset index). I did notice if you switch through different accounts (logging out / in) that have access to the same workspace the filter config gets stored as well. This is expected I guess cause it works with cookies, not a bad thing and not really a realistic case.

Anyways, didn't come across any issues

@carlosvirreira
Copy link
Contributor

Happy to release @DonKoko

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants