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.

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

Make the source argument for setDataAtCell in the Merge Cells plugin more specific. #10857

Merged
merged 5 commits into from Mar 19, 2024

Conversation

jansiegel
Copy link
Member

Context

The Merge Cells plugin utilizes populateFromArray and setDataAtCell methods to fill the cells under the merged cells with nulls.

Currently, the source argument given in the beforeChange/afterChange hooks differed between the following use cases:

  • When the merged cells were created at initialization, the source was defined as edit (used to be populateFromArray).
  • When the merged cells were created after initialization, the source was defined as MergeCells.

After the change introduced in this PR, both cases will be passing the source argument as MergeCells.

I think in both cases, the source should be more specific, like MergeCells.clearCells, but the change could be considered breaking, so I think we should maybe introduce this change with the next major release. WDYT @evanSe?

How has this been tested?

Added a test case.

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#1763
  2. handsontable/dev-handsontable#1764

Affected project(s):

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

Checklist:

jansiegel and others added 5 commits March 19, 2024 11:05
- Remove the ObserveChanges entry from the Events and Hooks docs page
- Add the Merge Cells entry to the Events and Hooks docs page
@jansiegel jansiegel requested a review from budnix March 19, 2024 10:33
Copy link

Launch the local version of documentation by running:

npm run docs:review df79d62832de5c11005e4c7f27e4a52407435cba

@jansiegel jansiegel merged commit 923e108 into develop Mar 19, 2024
21 checks passed
@jansiegel jansiegel deleted the feature/dev-issue-1764 branch March 19, 2024 12:08
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.

None yet

3 participants