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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelogs/10857.json
@@ -0,0 +1,8 @@
{
"issuesOrigin": "private",
"title": "Unified the `source` arguments of the `beforeChange`/`afterChange` hooks when being triggered by the Merge Cells plugin.",
"type": "fixed",
"issueOrPR": 10857,
"breaking": false,
"framework": "none"
}
Expand Up @@ -170,7 +170,7 @@ It's worth mentioning that some Handsontable hooks are triggered from the Handso
| [`ContextMenu.rowAbove`](@/api/contextMenu.md) | Action triggered by the ContextMenu plugin after the "Insert row above" has been clicked. |
| [`ContextMenu.rowBelow`](@/api/contextMenu.md) | Action triggered by the ContextMenu plugin after the "Insert row below" has been clicked. |
| [`CopyPaste.paste`](@/api/copyPaste.md) | Action triggered by the CopyPaste plugin after the value has been pasted. |
| `ObserveChanges.change` | Action triggered by the ObserveChanges plugin after the changes have been detected. |
| `MergeCells` | Action triggered by the MergeCells plugin when clearing the merged cells' underlying cells. |
| [`UndoRedo.redo`](@/api/undoRedo.md) | Action triggered by the UndoRedo plugin after the change has been redone. |
| [`UndoRedo.undo`](@/api/undoRedo.md) | Action triggered by the UndoRedo plugin after the change has been undone. |
| [`ColumnSummary.set`](@/api/columnSummary.md) | Action triggered by the ColumnSummary plugin after the calculation has been done. |
Expand Down
30 changes: 30 additions & 0 deletions handsontable/src/plugins/mergeCells/__tests__/mergeCells.spec.js
Expand Up @@ -67,6 +67,36 @@ describe('MergeCells', () => {
]);
});

it('should provide information about the source of the change in the `beforeChange` and `afterChange` hooks', () => {
const beforeChange = jasmine.createSpy('beforeChange');
const afterChange = jasmine.createSpy('afterChange');

handsontable({
data: [
[1, 2, 3, 4],
[5, 6, 7, 8],
[9, 10, 11, 12],
[13, 14, 15, 16],
],
mergeCells: [{
row: 0,
col: 0,
rowspan: 2,
colspan: 2
}],
beforeChange,
afterChange,
});

expect(beforeChange.calls.mostRecent().args[1]).toEqual('MergeCells');
expect(afterChange.calls.mostRecent().args[1]).toEqual('MergeCells');

getPlugin('mergeCells').merge(2, 2, 3, 3);

expect(beforeChange.calls.mostRecent().args[1]).toEqual('MergeCells');
expect(afterChange.calls.mostRecent().args[1]).toEqual('MergeCells');
});

it('should merge cells on startup respecting indexes sequence changes', () => {
handsontable({
data: [
Expand Down
4 changes: 3 additions & 1 deletion handsontable/src/plugins/mergeCells/mergeCells.js
Expand Up @@ -313,7 +313,8 @@ export class MergeCells extends BasePlugin {
return;
}

this.hot.setDataAtCell(populatedNulls);
// TODO: Change the `source` argument to a more meaningful value, e.g. `${this.pluginName}.clearCells`.
this.hot.setDataAtCell(populatedNulls, undefined, undefined, this.pluginName);
}
}

Expand Down Expand Up @@ -451,6 +452,7 @@ export class MergeCells extends BasePlugin {
populationInfo = [mergeParent.row, mergeParent.col, clearedData];

} else {
// TODO: Change the `source` argument to a more meaningful value, e.g. `${this.pluginName}.clearCells`.
this.hot.populateFromArray(
mergeParent.row, mergeParent.col, clearedData, undefined, undefined, this.pluginName);
}
Expand Down