-
Notifications
You must be signed in to change notification settings - Fork 104
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
Implemented virtual scrolling for Shoot List #1674
Conversation
# Conflicts: # frontend/src/views/GShootList.vue
Currently, we need to manually set |
there is a bug with the table when you scroll to the bottom and then filter the table using the search input field. As long as you do not scroll, only a subset of the result will be displayed |
This seems to be a bug in Vuetify: vuetifyjs/vuetify#20566 and vuetifyjs/vuetify#18926 |
- Added workaround for vuetify rendering issue when virtual table scroll position is not initial
@grolu You need rebase this pull request with latest master branch. Please check. |
The issue has been fixed in the upstream vuetify project so we can continue testing this PR |
frontend/src/views/GShootList.vue
Outdated
class="g-table" | ||
height="calc(100vh - 240px)" | ||
item-height="50px" |
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.
check if needed
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.
yes. definitely needed. Added a test to check that behavior doesn't change.
aria-label="Last page" | ||
@click="setPage(pageCount)" | ||
/> | ||
<template v-if="pageCount"> |
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.
can we use the default footer?
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.
No. The default footer is empty, but I think it makes sense to display the number of clusters
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.
Minor change request, otherwise /lgtm
# Conflicts: # frontend/__tests__/components/Vuetify.spec.js
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #1363
Special notes for your reviewer:
Release note: