-
-
Notifications
You must be signed in to change notification settings - Fork 63
Refactor workflows list to use Task & SearchParamsValue
#2898
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
Conversation
Task & SearchParamsValue
- updates pagination component to add function allowing overwriting page url without pushing to history - updates searchParamsValue reactive controller to auto-determine property name on host to allow for more nuanced lifecycle updates in host when values change - cleans up and updates workflow list page to correctly reset page to 1 when changing filters in ui
it doesn't seem to be feasible without getting super hacky - i think i'll need another approach.
2d0e4d8 to
2d8f27f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This currently always calls this.paginationElement?.setPage(1) in the willUpdate callback, even if a page is specified in the URL as ?page=<number>, making it difficult to debug where and how the page is actually set and creating unnecessary aborted requests. I pushed some suggestions here for streamlining updates.
Co-authored-by: emma <hi@emma.cafe>
|
Per Discord convo, this seems like a good use case for preact signals with side effects, since signals would allow us to subscribe to slices of state rather than the entire search params object. Right now the behavior is a bit unexpected when updating state with a single |
Follows #2855
Changes
Refactors the workflows list page to:
Taskfor fetching workflowsSearchParamsValues for filters/sorting/search parametersAlso fixes a bug with the archived item list page where updating filters wouldn't reset the page.
setPagemethod that takes some optionsTesting
On the archived item page, test that all of the filters work as expected.
Test that pagination works as expected, and the page is reset when changing filter parameters.
Test that the browser's back and forward buttons work as expected when changing filters/search/sorting/pages.
Known Issues
When changing a filter when you're on a page other than the first, the page is reset to the first page in a separate step from the filter change, which means that if you use your browser's navigation buttons to go back you'll need to go back twice to get to where you were before. E.g.:
?page=3→ user sets a filter →?mine=true— this works as expected, but from here:?mine=true→ user presses back button in browser →?mine=true&page=3→ user presses back button again →?page=3I'm not totally sure what the best approach here is. Ideally I think we'd hook into Lit's reactive update cycle from the search values controller and only do a single
history.pushStatein theupdatestage, but I'm not sure there's an easy way to do that without digging pretty deep into Lit's internals. Open to ideas though!