-
Notifications
You must be signed in to change notification settings - Fork 63
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
Features/822 pagination #1127
base: master
Are you sure you want to change the base?
Features/822 pagination #1127
Conversation
7843704
to
86b4b23
Compare
86b4b23
to
6e26342
Compare
<template v-else> | ||
{{ odataFilter == null ? $t('submission.emptyTable') : $t('noMatching') }} | ||
</template> | ||
<p v-show="emptyTableMessage" class="empty-table-message"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Show this only after transition is finished
@@ -111,6 +115,8 @@ following cases in mind: | |||
*/ | |||
watch(() => props.data, removeHoverClass); | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove these extra blank lines.
showsTable() { | ||
if (!this.odataEntities.dataExists) return false; | ||
const { length } = this.odataEntities.value; | ||
return length !== 0 && length !== this.odataEntities.removedCount; | ||
return this.odataEntities.dataExists; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to template to make it consistent with SubmissionsList
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! 🎉
Closes getodk/central#822
What has been done to verify that this works as intended?
Wrote some component tests and manually verified it. I am thinking to pass this to QA team even before we review/merge this PR.
Why is this the best possible solution? Were any other approaches considered?
We talked about what to do when a row is deleted, should we just remove the row from the UI or we fetch the next available row and update the count/page size. We decided that it is better to just remove the row.
Transition Group
I have used Vue's Transition Group (TG) component in our table-freeze component. I think this has simplified our code because model and view are alway in sync; and we no longer keep deleted row in the DOM hidden.
I am not using css class provided by TG component and kept our transition logic in markRowChanged and markRowDeleted. Because TG doesn't help with row modified, so thought might as well keep both change/delete consistent. The only magic of TG is that it waits for
transitionend
event before removing row from the table, that event is raised whentransition
started by css property is completed.Snapshot Filters
I am passing filters to odata endpoint to fetch the data for a single point in time so that submission/entity created or deleted after fetching first page don't effect our total count and number of pages.
For alive records, those filters are records created before
now
and not deleted or deleted afternow
.For deleted records, the filter is to fetch records that are deleted before
now
.I am keeping a set of removed rows (instanceId or UUID), so I don't add those rows in resource store when page is changed.
Only weird case is where a user is looking at alive records and some other user undeletes a record, then next odata request will have different
count
and number of pages can change. But I think it's not super problematic.How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
No
Does this change require updates to user documentation? If so, please file an issue here and include the link below.
Before submitting this PR, please make sure you have:
npm run test
andnpm run lint
and confirmed all checks still pass OR confirm CircleCI build passes