SD-1760 - feat: allow custom accept/reject handlers for TC bubbles#1921
SD-1760 - feat: allow custom accept/reject handlers for TC bubbles#1921mattConnHarbour wants to merge 1 commit intomainfrom
Conversation
![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: 6410f8173b
ℹ️ 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/superdoc/src/components/CommentsLayer/CommentDialog.vue
Outdated
Show resolved
Hide resolved
harbournick
left a comment
harbournick
left a comment
There was a problem hiding this comment.
I think its a cool idea and looks like a clean implementation. @mattConnHarbour please add unit tests to this covering the new api, as well as documentation
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
clean implementation - just address the comments (incld Codex's) and feel free to merge after that
| }; | ||
|
|
||
| const handleReject = () => { | ||
| // Check for custom handler that replaces default behavior entirely |
There was a problem hiding this comment.
should we add tests for the custom handler paths? the existing tests at only cover default behavior
we could add a case where config.onTrackedChangeBubbleAccept is set and verify it's called instead of the default
| onFontsResolved: null, | ||
|
|
||
| // Tracked change bubble handlers - replace default accept/reject behavior | ||
| // Only fires from bubble buttons, not toolbar or context menu |
There was a problem hiding this comment.
should we document that the default behavior includes resolving the comment and clearing active state?
users might not realize they need to handle these themselves - maybe add an example showing the full flow?
|
|
||
| const handleReject = () => { | ||
| // Check for custom handler that replaces default behavior entirely | ||
| const customHandler = proxy.$superdoc.config.onTrackedChangeBubbleReject; |
There was a problem hiding this comment.
new handlers should be added to apps/docs/core/superdoc/configuration.mdx, e.g.
<ParamField path="onTrackedChangeBubbleAccept" type="function">
Custom handler for accepting tracked changes from comment bubbles.
Replaces default behavior entirely when provided.
```javascript
onTrackedChangeBubbleAccept: (comment, editor) => {
// Your custom logic here
editor.commands.acceptTrackedChangeById(comment.commentId);
await saveDocument(); // e.g., custom save after accept
}
Only fires from bubble buttons, not toolbar or context menu
mattConnHarbour
force-pushed
the
SD-1760
branch
from
6410f81 to
6cc9c06
Compare
3 participants
No description provided.