fix(links): link styles should be removed when tracked change is rejected#2465
fix(links): link styles should be removed when tracked change is rejected#2465
Conversation
superdoc-bot
bot
added
risk: critical
risk: sensitive
and removed
risk: critical
labels
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8289f71cdc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markSnapshotHelpers.js
Show resolved
Hide resolved
…jecting tracked changes
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@palmer-cl nice fix — removing "after" marks before restoring "before" marks is clean and the overlap matcher fills a real gap. no correctness issues. tests are solid across all layers. left a couple inline suggestions on duplicated code and one note on visual test coverage.
| newTr.addMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), step.mark); | ||
|
|
||
| const allowedMarks = ['bold', 'italic', 'strike', 'underline', 'textStyle', 'highlight']; | ||
| const allowedMarks = ['bold', 'italic', 'strike', 'underline', 'textStyle', 'highlight', 'link']; |
There was a problem hiding this comment.
this same list exists in removeMarkStep.js:39 — move it to constants.js so there's one place to update. this PR already had to change both.
| const text = 'text' in run && typeof run.text === 'string' ? run.text : ''; | ||
|
|
||
| // Include formatting marks that affect measurement (mirroring paragraph approach) | ||
| // Include formatting marks that affect visual output (mirroring paragraph approach). |
There was a problem hiding this comment.
this block is copy-pasted at line ~300. a small helper would mean future additions (like the underline/strike/link ones in this PR) only need one edit. worth it?
| @@ -0,0 +1,68 @@ | |||
| import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js'; | |||
There was a problem hiding this comment.
the behavior test checks state, but since the bug was about link styling staying visible, a visual screenshot test (like the existing reject-mixed-track-format.spec.ts) would catch pixel-level regressions too. worth adding?
2 participants
Fixed tracked-change reject for hyperlink suggestions so rejecting a suggested link removes both the link mark and transient hyperlink styling
Hardened layout invalidation by including hyperlink/underline/strike in diff and cache keys, so reject/apply formatting changes always trigger correct re-rendering instead of stale cached runs.