Skip to content

Conversation

@budnix
Copy link
Member

@budnix budnix commented Mar 28, 2025

Context

The PR fixes the TypeError that could occur when removing the row from the bottom overlay. It turned out that there is a more general problem. The issue sits in the overlays table's methods that incorrectly returned the first/last row/column indexes. For example, for the top overlay, when there were no rows rendered (only headers), the getFirstRenderedRow method returned 0 where it should -1, the same issues I fixed for other overlays.

Secondly, after fixing the bug reported by the origin issue, I found another one with the same TypeError and the same scenario (removing a row from the bottom overlay). It's also fixed within PR's changes. In that case, there was a problem with the selection update being in the wrong order compared to the tableMeta update.

How has this been tested?

I tested the changes locally and I covered the fix with new tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Related issue(s):

  1. fixes https://github.com/handsontable/dev-handsontable/issues/2351

Affected project(s):

  • handsontable

Checklist:

@budnix budnix self-assigned this Mar 28, 2025
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2d1a3ce:

Sandbox Source
handsontable-pr-javascript-demo Configuration
handsontable-ts-demo Configuration
handsontable-pr-react-demo Configuration

@budnix budnix marked this pull request as ready for review March 28, 2025 10:21
@budnix budnix requested review from jansiegel and qunabu March 28, 2025 10:21
@budnix budnix merged commit 1823be4 into develop Apr 2, 2025
32 checks passed
@budnix budnix deleted the feature/dev-issue-2351 branch April 2, 2025 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants