Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(react-grid): correct calculating start index of loading row for Infinite Scrolling #2256

Merged
merged 11 commits into from
Aug 27, 2019
Merged

fix(react-grid): correct calculating start index of loading row for Infinite Scrolling #2256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f96c158
Fix bug
LazyLahtak Aug 21, 2019
78d22eb
Correct tests
LazyLahtak Aug 21, 2019
6769228
Update API
LazyLahtak Aug 21, 2019
43f978d
Refactor
LazyLahtak Aug 22, 2019
11af8be
Fix
LazyLahtak Aug 22, 2019
672a0e8
Correct calculation and add test
LazyLahtak Aug 23, 2019
4878aef
Update API
LazyLahtak Aug 23, 2019
9fb6479
Refactor
LazyLahtak Aug 23, 2019
37d1f00
Refactor
LazyLahtak Aug 26, 2019
89301ed
Refactor
LazyLahtak Aug 27, 2019
bdce541
Merge branch 'master' into virtual-table-index-bug
LazyLahtak Aug 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/dx-grid-core/src/plugins/virtual-table-state/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ export const calculateRequestedRange: CalculateRequestedRangeFn = (
) => {
const loadedInterval = intervalUtil.getRowsInterval(virtualRows);
const isAdjacentPage = Math.abs(loadedInterval.start - newRange.start) < 2 * pageSize;
if (isAdjacentPage) {
const calculatedRange = intervalUtil.difference(newRange, loadedInterval);
const calculatedRange = intervalUtil.difference(newRange, loadedInterval);
if (isAdjacentPage && calculatedRange !== intervalUtil.empty) {
if (calculatedRange.start - referenceIndex > pageSize / 2) {
calculatedRange.start -= pageSize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In non-infinite scrolling mode request range shouldn't be affected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'll fix it.

calculatedRange.end -= pageSize;
Expand Down Expand Up @@ -143,7 +143,7 @@ export const needFetchMorePages: PureComputed<[VirtualRows, number, number], boo
const { start, end } = intervalUtil.getRowsInterval(virtualRows);
const loadCount = end - start;
const topTriggerIndex = start > 0 ? start + pageSize! : 0;
const bottomTriggerIndex = end - pageSize!;
const bottomTriggerIndex = Math.max(topTriggerIndex + pageSize, end - pageSize! * 1.5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a 1.5 value?

Copy link
Contributor Author

@LazyLahtak LazyLahtak Aug 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Half of page.
Sometimes difference between bottomTriggerIndex and topTriggerIndex more than one pageSize and we can get index, than no trigger update. This is index in interval between end - pageSize! and end - pageSize! * 1.5
For example, in master try after load press End button twice (lazy loading => infinite scrolling demo)
You will stuck at string 150 as last with bottomTriggerIndex === 150 and index ~ 140

Copy link
Contributor Author

@LazyLahtak LazyLahtak Aug 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes we may catch this bug not in beginning of table. It is quite difficult to catch, but can. This check fixes it.


if (loadCount <= 0) {
return false;
Expand Down
5 changes: 2 additions & 3 deletions packages/dx-react-grid/src/plugins/virtual-table/virtual-table-state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,13 @@ class VirtualTableStateBase extends React.PureComponent<VirtualTableStateProps,
{ virtualRows }: Getters,
) => {
const { pageSize, totalRowCount } = this.props;
const { requestedStartIndex, availableRowCount } = this.state;
const { requestedStartIndex } = this.state;
const actualVirtualRows = forceReload ? emptyVirtualRows : virtualRows;
const { requestedRange, actualBounds } = getRequestMeta(
referenceIndex, virtualRows, pageSize!, totalRowCount, forceReload,
);
const needCheckLimit = requestedStartIndex < availableRowCount;

if (forceReload || needCheckLimit || shouldSendRequest(requestedRange, requestedStartIndex)) {
if (forceReload || shouldSendRequest(requestedRange, requestedStartIndex)) {
this.requestNextPage(requestedRange, actualVirtualRows, actualBounds);
}
}
Expand Down