osf.io icon indicating copy to clipboard operation
osf.io copied to clipboard

[Feature] Cursor based pagination [#OSF-8565]

Open mfraezz opened this issue 8 years ago • 4 comments

Purpose

Improve NodeLogList performance

Changes

  • Create JSONAPI-compliant subclass for rest_framework.pagination.CursorPagination
  • Use on NodeLogList
  • Update frontend

Side effects

None expected

Ticket

[OSF-8565]

mfraezz avatar Sep 15 '17 19:09 mfraezz

This is a pretty significant, backwards-incompatible change (worthy of more than a minor version bump). I wonder if we should keep the current pagination scheme to preserve compat but allow changing the pagination style via a query parameter. Looks like there's even a package for that.

Thoughts, @brianjgeiger ?

sloria avatar Sep 25 '17 14:09 sloria

The loading indicator for the log feed got lost with this change.

That said, the log feed was using the now-deprecated OSF loading spinner. Two options:

  • Good: Update the log feed to use a new spinner (.ball-pulse).
  • Better: Use a skeleton screen as a loading indicator. A quick-and-dirty way to achieve this is to reduce the opacity of the items when the widget is in the loading state. It's not exactly the same as a skeleton, but at least it doesn't change the height of the widget as much:

osf___small_project

sloria avatar Sep 25 '17 15:09 sloria

According to discussion with @brianjgeiger sometime over the past two weeks, this does not merit an API version bump. The values for linked references change, yes, but the rationale is that the client should only be following the resp.json['links'], the references for which do not change. That is, client-side url construction is not a supported use case; they should be using links provided in the response.

I'll fix the loading UI

mfraezz avatar Sep 25 '17 17:09 mfraezz

@mfraezz Ah ok, thanks for the clarification. I wasn't sure if the keys of the pagination object were changing here or not. If not, then I agree: no need to bump versions.

sloria avatar Sep 25 '17 17:09 sloria