Page MenuHomePhabricator

Paginate longitudinal views
ClosedPublic

Authored by sashank on Mar 30 2020, 5:37 PM.

Details

Summary

Depends on D2365.

Test Plan

manual

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sashank created this revision.Mar 30 2020, 5:37 PM
sashank retitled this revision from Update longitudinal views to Paginate longitudinal views.Mar 30 2020, 5:44 PM
sashank edited the summary of this revision. (Show Details)Mar 31 2020, 5:37 PM
sashank added a reviewer: prha.
prha accepted this revision.Apr 1 2020, 6:38 PM

overall looks good.

js_modules/dagit/src/schedules/ScheduleRoot.tsx
67

nit: maybe extract 100 as a constant?

91

what's this + 1 about?

466–468

should we reset the cursor if we change the page size?

This revision is now accepted and ready to land.Apr 1 2020, 6:38 PM
sashank added inline comments.Apr 5 2020, 6:49 AM
js_modules/dagit/src/schedules/ScheduleRoot.tsx
67

will do

91

We query for just one additional run to tell whether we have a next page or not. See [1]

110–111

[1]

466–468

hm not sure - what do you think. i originally didn't because it would be weird if you were on the last page, changed the page size, and then were taken all the way back to the start

sashank updated this revision to Diff 11534.Apr 7 2020, 11:24 PM

rerun tests

This revision was automatically updated to reflect the committed changes.