fix: track changes comment text for formatting changes #2013
Conversation
Visual diffs detectedPixel differences were found in visual tests. This is not blocking — reproduce locally with |
The visual test that failed is related to this PR which hasn't been merged yet: #1995 |
luccas-harbour
changed the title
luccas-harbour
changed the title
luccas-harbour
requested review from
VladaHarbour,
artem-harbour,
caio-pizzol and
harbournick
![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: d53d326547
ℹ️ 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/addMarkStep.js
Outdated
Show resolved
Hide resolved
luccas-harbour
force-pushed
the
luccas/sd-1742-bug-style-changes-in-specific-document-from-sd-webpage-apply
branch
from
d53d326 to
85da4cf
Compare
luccas-harbour
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85da4cf907
ℹ️ 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/comment/comments-plugin.js
Outdated
Show resolved
Hide resolved
luccas-harbour
force-pushed
the
luccas/sd-1742-bug-style-changes-in-specific-document-from-sd-webpage-apply
branch
2 times, most recently
from
73d9d03 to
ab2d2b1
Compare
luccas-harbour
force-pushed
the
luccas/sd-1742-bug-style-changes-in-specific-document-from-sd-webpage-apply
branch
from
ab2d2b1 to
b24dd07
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b24dd078d4
ℹ️ 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".
| const wasTextStyleRemoved = nodes.some((node) => { | ||
| const hasTextStyleMark = node.marks.find((mark) => mark.type.name === 'textStyle'); | ||
| return !hasTextStyleMark; | ||
| }); |
There was a problem hiding this comment.
wasTextStyleRemoved currently flips to true if any node in the tracked range lacks a textStyle mark, but format changes can span mixed content where some nodes never had textStyle at all. In that case, a non-textStyle change (for example, adding bold) can be normalized as if text style was removed, so translateFormatChangesToEnglish emits incorrect “Removed …” text for unchanged style attrs. This is especially visible when one node in the range has before.textStyle and another node does not; removal detection should be derived from the tracked before/after payload for this change, not from nodes.some(...) over heterogeneous nodes.
Useful? React with 👍 / 👎.
1 participant
This PR tightens tracked-formatting behavior and comment generation for style changes.
removeMarkStepso before/after payloads are built from actual existing mark attrs and avoid duplicate/incorrect entries.textStyleattrs before translation, preventing"undefined"values in the generated message. It also an adds support for describing removed style properties.comments-plugin.test.js.Before:
After:
