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

feat(idea/frontend) add query params #1701

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

Zewasik
Copy link
Member

@Zewasik Zewasik commented Dec 9, 2024

Resolves #1691 .

  • add query params for programs, messages, vouchers and mailbox

@osipov-mit

@osipov-mit
Copy link
Member

@Zewasik pls update branch
@nikitayutanov pls review

@nikitayutanov
Copy link
Member

Good solution so far, but I think the task is a little bit more complicated than it seems - I guess we need some generic solution, rather than implementing it over in each component that has filters functionality, otherwise it's gonna be hard to maintain.

The way I see it:

  • We have to handle default filter state values - merge default empty values with the ones from search params.
    Perhaps, it should be possible to define one generic hook for it - useFilterValues as a part of filters feature.
    It would accept default values as a parameter and perform necessary logic of combining it with URL params under the hood.

  • We have to set URL params on filter value submit. I believe would make sense to embed it's logic directly onto submit handlers - either common one in Filters component itself, or to each separate component such as filter Radio/StatusRadio/StatusCheckbox. This way we can eliminate the need to use side effects inside of useEffect, and generalize URL params set in one place to reuse everywhere.

  • Specified above principles should also apply to SearchForm component as well.

Apart from it, here's concerns to be aware of even in the current implementation:

  • Reset filters button should reset filter to it's empty state, even if URL params are present. To reproduce - open URL with defined params -> reset button will appear only on changed value, but should already be present, since custom value is provided from URL.

  • Some kind of check should be performed to determine whether provided value from URL is a valid one. To reproduce - open URL with defined params -> change param to a random value, that is not the one present in filters.

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.

feat(idea/frontend) add location query and history
3 participants