-
Notifications
You must be signed in to change notification settings - Fork 13
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
Pagination in query string #1102
Comments
The length likely matters less in the case of query params. It's hard to really do anything about having a shorter cursor given it generally maps to a UUID. Still, it's more likely the case that they'd just copy+paste that. Given the overall length of our URLs anyway, I doubt many folks will be typing stuff out by hand 😅. Need a shortcode service, haha. |
The other reason we haven't done this yet is that we can't get to the previous page. So if you have a cursor in the query string and you refresh, you can only go forward, not back. Not very useful. |
We need to capture the various bits of API work on pagination in a tracking issue. To me this is pretty bad right now and very important to the basic usage of the console, but we'll need to prioritize that API work if we want to make progress. |
Switched to MVP+1 because I think fixing pagination is very important, but I'm not sure putting it in the query string, as one part of that, is necessary for MVP. |
An odd note on pagination. Dropshot is designed such that the pagination token contains all the necessary query parameters to look up a given page. In practice this means that selectors in the new api (.e.g.
Previously when the hierarchy was encoded into the route this wasn't really a big deal. Given that everything is now query parameters it does change how the API is used. This doesn't really mean anything for how pagination works in console. It does mean that the pagination token will be longer than we originally anticipated. You aren't going to be typing this in by hand. |
You can still leave in the redundant |
I don't think it'll error out, but my understanding is that that's invalid. It will, at best, be ignored. |
We could do this today. I think the main reason we haven't is that the cursor is really long. But maybe we should just do it and then think about making it shorter on the API side if we really care.
The text was updated successfully, but these errors were encountered: