Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TinyMCE Comments Plugin #473

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

domwhewell-sage
Copy link
Contributor

@domwhewell-sage domwhewell-sage commented Jul 2, 2024

Draft PR for a comments plugin mentioned in #425
Added a new tinymce plugin to the folder called ghostwriter-comments

  • The plugin adds a Comment context menu item to all wysiwyg editors
image
  • It'll allow you to highlight some text and add a comment.
  • Once a comment is added it'll appear with a red background until you select resolve.
image
  • Once a comment is resolved the red background disappears but the comment remains in the source code of the editor the comment thread can also be viewed again by highlighting the text and selecting comment
<p>This is a <span class="comment-resolved" comment-data="%5B%7B%22author%22%3A%2261646d696e%22%2C%22comment%22%3A%22Test%22%7D%2C%7B%22author%22%3A%22646f6d2e77686577656c6c%22%2C%22comment%22%3A%22Hello%22%7D%5D">test</span> vulnerability</p>
  • Multiple collaborators can comment on findings. The comment from the current user will always appear on the left
image

The comment thread currently appears with the commenters username avatar and then their comment.

<div class="${selfclass}">
    <div class="comment-header">
        <h1>${tinymce.DOM.encode(hex2ascii(author))}</h1>
        <img class="comment-avatar" src="/users/${tinymce.DOM.encode(hex2ascii(author))}/avatar" alt="Avatar">
    </div>
    <p>${tinymce.DOM.encode(comment)}</p>
</div>

The comments dont currently transfer over to the docx but Im not sure they should, the reportwriter is currently just adding the paragraph element into the docx correctly without the comment so we might not want to change that

@domwhewell-sage
Copy link
Contributor Author

I'm trying to think of a way to notify the author which findings / fields have comments on them sort of like the "Previous" / "Next" functionality in word

image

We could potentially send an ajax request to set the finding as "Needs Editing" once a reviewer has posted a comment on a finding but I think sending a list of urls to the user that take them directly to each comment would be more useful. We might be able to use the websockets notifications for the user but I don't know how long these stay available to the user

Copy link
Collaborator

@ColonelThirtyTwo ColonelThirtyTwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your patch! I've added a few notes - let me know if you're still interested in working on this.

@domwhewell-sage
Copy link
Contributor Author

domwhewell-sage commented Jul 31, 2024

Thanks @ColonelThirtyTwo I have made the requested changes. And added a border around each individual comment, moved the avatar to the top left of the comment.

I am still interested in working on this however I am still trying to think of a way to solve the notification problem. After your report has been QA'd (and comments have been left) it might be time consuming for the author to go back through each individual finding and check if all comments have been addressed in the report.

@domwhewell-sage
Copy link
Contributor Author

Thinking about it I don't think the notifications should be part of this plugin as I did have it sending websocket messages once a comment had been added but if the user didn't submit the form the comment wouldn't save therefore notifications should probably be done server side once the form is submitted.

Something like this: If the updated form contains any rich text fields with 1 or more <span class="comment"> tags in the source send a notification.

Marking this as ready for review now.

@domwhewell-sage domwhewell-sage marked this pull request as ready for review August 9, 2024 10:33
Copy link
Collaborator

@ColonelThirtyTwo ColonelThirtyTwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to support nested comments (which I think we should), I think this this may need to be split up into a "View Existing Comment" button and a "Create New Comment" button. View would traverse up the node tree and open the editor for an existing comment if found and replace that span specifically when edited, whereas Create would create a comment for the highlighted span. This would also allow one to edit the content of a commented span, which is impossible with the current design.

This would mean you could comment on an entire paragraph while also being able to comment on individual words or sentences.

Styling may be a pain though to indicate nested comments... you'd have to have .comment { color: onecolor; } and .comment .comment { color: anothercolor; } and so on.

@domwhewell-sage
Copy link
Contributor Author

Hi @ColonelThirtyTwo, I have reworked the comment plugin to allow for nested comments, There is now a "Comments" submenu in the context menu, which allows you to "Create comment" and "View comments". "Create" is a simple dialog to create a brand new comment (Which can be nested (There is CSS to support up to three levels of comment nesting)) and "View" opens a dialog displaying the existing comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants