Make the table trigger "fast" render instead of a full one, when the table is outside of the viewport.

[jansiegel]

Context

As described in https://github.com/handsontable/dev-handsontable/issues/983, since 12.1.0, the table triggers a "full" render with every scroll when the table is outside of the viewport.

This PR aims to restore the state from <12.1.0 - when outside the viewport, the table renders only using the "fast" mode.

This should also fix the problem described in handsontable/dev-handsontable#561.

Additional to the added logic and tests, I removed the lastSelectedColumn property from the Filters plugin because I didn't find anything referencing it.

How has this been tested?

Added test cases.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature or improvement (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Additional language file or change to the existing one (translations)

Related issue(s):

1. handsontable/dev-handsontable#561
2. partially resolves https://github.com/handsontable/dev-handsontable/issues/983

Affected project(s):

• handsontable
• @handsontable/angular
• @handsontable/react
• @handsontable/vue
• @handsontable/vue3

Checklist:

• I have reviewed the guidelines about Contributing to Handsontable and I confirm that my code follows the code style of this project.
• I have signed the Contributor License Agreement
• My change requires a change to the documentation.

[github-actions]

Launch the local version of documentation by running:

npm run docs:review eb9cb44057f00173d44c21d6919a83cc1ba921ea

[budnix]

I tested the changes, and here are my thoughts:

1. Generally, I think that it's a good idea to test if the table is visible in the viewport or not - so good direction. However, I'd propose merging the areAnyRowsFullyInViewport and areAnyColumnsFullyInViewport methods to one that uses IntersectionObserver API. The code would be much more simplified and bullet-proof. For example, code for both axes:

    this.visibilityObserver = new IntersectionObserver((entries) => {
      entries.forEach((entry) => {
        this.isOutsideOfViewport = !entry.isIntersecting;
      });
    }, {
      root: null,
    });

   Right now, there is a bug in those methods which appears in resulting wrong true/false results. See the gif
   
2. The main issue seems to be solved (that with the checkboxes in the list), but I think it's some sort of lucky code combination related to the bug in the above methods 😄 I've tried using the IntersectionObserver API, and the fast/slow render in and outside the viewport work as expected but the main issue still did not work.
3. The last thing. For viewport stuff, I think the best place for that code would be the viewport.js file. Thanks to that, you won't have to juggle arguments back and forth in the method wtViewport.createRenderCalculators within the WoT Table class.

[jansiegel]

@budnix

As for 1, there are a few things worth mentioning here:

1. The names I gave those functions are oversimplified and thus misleading, I'll need to address that. What they're supposed to check is if all the rows of the table except for the last one are in the viewport. So the first issue you're showing on the gif is actually a feature 😆 (The second one isn't, it's clearly a bug)
   If I remember correctly (it's been some time since I created those methods), it needs to check it that way because of when the calculators were re-triggered when scrolling the table. Without that, the issue with the checkboxes would not be fixed - with the last row in the viewport, there would still be a full render. Which brings me to...

2. ... the intersection observer idea. In general, I totally agree - the intersection observer would be a perfect thing to use here if it wasn't for the problem with the last row being visible in the viewport. I'm guessing that's probably the reason why your proof of concept didn't solve the original issue.

[budnix]

2. ... the intersection observer idea. In general, I totally agree - the intersection observer would be a perfect thing to use here if it wasn't for the problem with the last row being visible in the viewport. I'm guessing that's probably the reason why your proof of concept didn't solve the original issue.

I'd be for more deeply investigating why there is a need for a gap for the last row and try to use the intersection observer idea. The current implementation seems to be covering some other bug.

[jansiegel]

I'd be for more deeply investigating why there is a need for a gap for the last row and try to use the intersection observer idea. The current implementation seems to be covering some other bug.

I'll get to it and try to elaborate more on the exact reason why this is happening 👍

[jansiegel]

@budnix
The logic underneath calculating the viewport without the last row/column stems from the change introduced in #9542, namely:

The fix adds conditions that check whether only the partially visible records are rendered in the viewport and if true then the full table's render is performed.

This, as a result, forces the table to perform full renders if the table is outside of the viewport or only the last row/column is visible.
That, in turn, causes the checkboxes in the dropdown menu of the Filters plugin to get re-rendered and lose their state.

Removing the logic from #9542 would break instances with oversized cells taking up more than the width/height of the viewport - leaving it in and using the intersection observer would not resolve the issue from handsontable/dev-handsontable#561.

Do you have any suggestions on how to move forward?

[aninde]

I checked and confirmed that the 'Walkontable' tests pass in every supported environment while waiting for the review to end.

[budnix]

Do you have any suggestions on how to move forward?

I investigated the issue, and I think that the best idea is to use rows/columns calculator to detect if the row or column is in or out of the viewport. Then, with my PoC, I used that flag to improve the changes that you've mentioned (#9542). WDYT?

[jansiegel]

Do you have any suggestions on how to move forward?

I investigated the issue, and I think that the best idea is to use rows/columns calculator to detect if the row or column is in or out of the viewport. Then, with my PoC, I used that flag to improve the changes that you've mentioned (#9542). WDYT?

Looks great! 🏅 Much cleaner than my initial idea. Would you make a PR of that branch so that we could merge it into this one?

[budnix]

Would you make a PR of that branch so that we could merge it into this one?

Something like that #10224?

[jansiegel]

As discussed, handsontable/dev-handsontable#1106 (marked as a blocker) is being postponed and is not fixed in this PR.

handsontable/dev-handsontable#1107, however, should work all right after merging this PR.

@budnix, could you re-review?

[budnix]

We are almost there 👏 I think there is missing logic in the FULLY_VISIBLE_TYPE calculator that returns the wrong isVisibleInTrimmingContainer value when the table disappears at the bottom of the browser viewport.

For the browsers top edge, it works correctly:

For the browsers bottom edge, it's not:

[budnix]

👍