fix: drag strategy only clear group id set by us #8355
Conversation
cpcallen
left a comment
There was a problem hiding this comment.
Hello HolowMan6, and thanks for this PR. This seems like a sensible fix, and the code looks great—I have only a couple of minor nits re: documentation and organisation.
|
Hi @cpcallen! Thanks for reviewing, I just updated the code according to your change request! |
|
@HollowMan6: test failures are due to issues with node.js v22.5.0 (see #8392 and #8393). Can you rebase this PR onto latest |
We should add a condition check so that we don't unset the group ID that is not set by us, otherwise, the multi-select plugin undo/redo will be broken (apply individually instead of all together) Signed-off-by: Hollow Man <[email protected]>
Done! |
|
Hi @HollowMan6 and @ewpatton This change has caused some issues with ending event groups when you drag a block out of the toolbox (#8764). I'm not very familiar with the multi-select plugin, so I'm not sure what options there are for an alternative fix. Would you like to take a look before I revert this change and file an issue to follow up on the plugin? |
The basics
The details
Resolves
Fixes mit-cml/workspace-multiselect#62 (comment)
Proposed Changes
We should add a condition check so that we don't unset the group ID that is not set by us, otherwise, the multi-select plugin undo/redo will be broken (apply individually instead of all together)
Reason for Changes
If not, then a monkey patch will be needed on the multi-selection plugin side.
Test Coverage
Documentation
Additional Information