SD-2974 - Bug: image resizing in suggesting mode duplicates the image#3238
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
andrii-harbour
left a comment
andrii-harbour
left a comment
There was a problem hiding this comment.
Nice job! Here are a few notes:
- Rebase needed: the branch is ~1,100 commits behind main, and main's
ImageResizeOverlay.test.js:17mockstras{ setNodeMarkup }only — nosetMeta— so this PR as-is throwstr.setMeta is not a functionin main's unit tests. (Good news: verified the bug still reproduces on current main, so the fix is still needed.) - Simpler mechanism worth considering:
tr.setNodeAttribute(imagePos, 'size', …)instead ofsetNodeMarkup+skipTrackChanges— it also fixes an edge case the current approach misses (details inline). - Test hardening: the behavior test can false-pass if the drag no-ops, and two of the ticket's acceptance criteria aren't covered yet (details inline).
…ting-mode-duplicates-the-image
…-2974) Address review feedback: - Replace setNodeMarkup + skipTrackChanges with tr.setNodeAttribute, which emits an AttrStep. AttrSteps pass through the track-changes machinery untracked and never trip the protect-tracked-review-state guard, so the resize applies in place even when the image is itself a pending tracked insertion. - Bump the containing blocks' sdBlockRev in the same transaction: AttrSteps have no changed range, so blockNodePlugin never increments the rev and the layout engine would reuse the cached FlowBlock, leaving the paint stale (same pattern as numberingPlugin's bumpBlockRev). - Harden the behavior test against false-greens by asserting the image's size attrs actually changed after the drag. - Add coverage for repeated resizes (AC #3) and for resizing an image that is itself a pending tracked insertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@andrii-harbour Thanks for the review! All addressed, including the rebase. 1. Mechanism - switched to 2. Tests - all three added: before/after size assertions, repeated resizes, pending-insertion resize. Two findings from the pending-insertion test:
|
|
🎉 This PR is included in superdoc-cli v0.16.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.15.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.11.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.39.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.10.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.11.0 |
6 participants
Issue
Images resized in suggesting mode creates a duplicate.
Proposed solution
Since MS Word does not track changes when the image is resized, the simplest way to fix the issue is by simply ignoring track changes on the event of an image resize.
Just for future reference, the original issue happens around here.
This
replaceRangecall adds the new (resized) image but keeps the old one. If we ever want to show the old and new image (and show the tracked change for resized) images, we might have to fix this.Created a behavior test that ensures this doesn't happen in the future (tested both before & after the fix).