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

react-query for blocklist fetching and caching #128

Merged
merged 8 commits into from
Nov 24, 2024

Conversation

noahm
Copy link
Collaborator

@noahm noahm commented Nov 24, 2024

This switches over to react-query (aka tanstack query) to manage fetch/cache behavior of blocklists. Only one page will load initially. If more pages are available, the next will begin loading after a user scrolls to the bottom of the current list.

image

This also removes the search functionality, as it will be inaccurate as long as we aren't loading the complete list of blocks in the client. In the future we can restore search behavior somehow, ideally via server-side call, but perhaps conditionally whenever all pages have been loaded?

Closes #80
Closes #59
Begins progress on #122

@noahm noahm marked this pull request as ready for review November 24, 2024 04:33
Copy link
Member

@thieflord06 thieflord06 left a comment

Choose a reason for hiding this comment

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

Great! I'm going to just approve and let someone that's technical sign off.

@AshishDhama
Copy link
Contributor

@noahm Please check what's breaking Ag grid view

Screen.Recording.2024-11-24.at.10.43.04.AM.mov

@AshishDhama AshishDhama self-requested a review November 24, 2024 06:37
@noahm
Copy link
Collaborator Author

noahm commented Nov 24, 2024

@AshishDhama Nice catch. Oddly the table view seems to completely break when the search header is taken out of the layout... Looks like something weird about a flex or grid based layout quirk.

@noahm
Copy link
Collaborator Author

noahm commented Nov 24, 2024

Actually... my auto-loading approach doesn't work at all in the table view. Gotta work on this more tomorrow if that's also important to keep enabled.

@AshishDhama
Copy link
Contributor

@thieflord06 Could you please give more context for the ag-grid view, the current feature doesn’t justify ag-grid usage. It's a big library.

@thieflord06 thieflord06 self-requested a review November 24, 2024 13:18
@thieflord06
Copy link
Member

@mihailik Can you give context?

@noahm
Copy link
Collaborator Author

noahm commented Nov 24, 2024

After discussing in the discord a bit, i've gone ahead and removed the table view in this PR. We can explore restoring it in the future.

@AshishDhama AshishDhama merged commit 4bcb078 into ClearskyApp06:dev Nov 24, 2024
1 check passed
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.

Load lists dynamically on scroll instead of consistently calling until done Double API calls
3 participants