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

[F] Use query params in admin #3653

Merged
merged 15 commits into from
Dec 12, 2024
Merged

[F] Use query params in admin #3653

merged 15 commits into from
Dec 12, 2024

Conversation

1aurend
Copy link
Contributor

@1aurend 1aurend commented Sep 6, 2023

This PR unifies how list pagination and filters are handled between the frontend and admin mode, using query params wherever a route's primary purpose is to display a single list. Pagination links are also changed to buttons rather than anchors where no valid href exists, e.g. on pages displaying multiple paginated lists.

@1aurend 1aurend marked this pull request as ready for review September 7, 2023 22:17
@1aurend 1aurend closed this Sep 7, 2023
@1aurend 1aurend reopened this Sep 7, 2023
@1aurend 1aurend changed the title [WIP] Use query params in admin [F] Use query params in admin Sep 7, 2023
@1aurend
Copy link
Contributor Author

1aurend commented Oct 31, 2024

@dananjohnson It looks to me like this is basically done, and the only conflicts are snapshots that can be deleted. I'd need to update the new spam mitigation list views to match this and do a little testing, but it seems worth getting in?

@dananjohnson
Copy link
Member

@1aurend that's great! Yeah I agree—it's worth merging and then utilizing in the new list views.

Copy link
Member

@dananjohnson dananjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good, @1aurend! Most things are working well, and the query params on the lists are a welcome change. I noticed a couple little issues that I'll describe below. I haven't looked at Manifold in awhile so I don't really know whether these relate to your PR or not. Feel free to suggest I open a separate Linear issue for any.


  • I noticed one weird thing with project collections. Here's how to reproduce:
  1. Toggle drawer to create a new PC
  2. Fill out whatever fields and click "Save Project Collection"
  3. Notice that behind the drawer, the new PC is created and we are routed to it (/backend/projects/project-collections/[slug]).
  4. Notice that the drawer doesn't close and a "You have unsaved changes" prompt appears.
Screenshot 2024-12-09 at 10 39 32 AM
  • Also, when I try to visit a project's Exports page, I get an exception with the project_exportations fetch that crashes the page:
useFetch tried to fetch data more than 25 times. This suggests that an input to
useFetch needs to be memoized.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@1aurend
Copy link
Contributor Author

1aurend commented Dec 12, 2024

@dananjohnson the project exports bug was related to this PR and was an easy fix. The PC drawer thing is an issue on Edge too, so I'm copying your notes into a separate Linear issue. I think it's an issue with how onDirty is being set on that form. It's one of the very old ones.

@scryptmouse scryptmouse merged commit 5eaaae7 into master Dec 12, 2024
2 of 3 checks passed
@scryptmouse scryptmouse deleted the ld/mnfld-569 branch December 12, 2024 19:13
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.

3 participants