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

Inline commenting #3

Closed
kishorenc opened this issue Feb 6, 2012 · 15 comments
Closed

Inline commenting #3

kishorenc opened this issue Feb 6, 2012 · 15 comments

Comments

@kishorenc
Copy link

Any plans for supporting document commenting in the roadmap? Although you can't strictly classify such a feature under "track changes", while editing a document, you often have the need to add annotations/comments.

@briantully
Copy link

Yes, would love to see this feature as well.

@benesch
Copy link
Contributor

benesch commented Mar 4, 2012

This is absolutely essential for a project I'm working on. I'll let you know about my progress!

@benesch
Copy link
Contributor

benesch commented Mar 4, 2012

@kishorenc
Copy link
Author

Looks neat! A few years back when I was trying to do this whole thing in Flash, one issue we came up against was overlapping comments. What's your idea for tackling that? Right now, new comments in an overlapping area of text overrides the text's previous comment. This is the easiest approach to take. But, it would help if two comments next to each other were in a different color so users can differentiate between them.

@briantully
Copy link

Nikhil -- this is an amazing start! Thanks so much for initiating this.

As Kishore mentioned above, my gut tells me that people will want to use the comments/annotations functionality to engage in discussions. In other words, at least allow the writer and editor to leave annotations that can be edited and clarified through simple means.

I envision this looking something like the following rough screenshot: http://dl.dropbox.com/u/16531332/Photos/ICE-comments-mockup.png

The only issue I can foresee is how will the comments/markup inferfere with what ICE perceives as changes. So in your example, the comment requests that the writer rephrase. When the editor rephrases the text "happy to talk to reporters", ICE correctly recognizes it as a change and inserts the proper markup. However if the "Resolve Comment" button is pressed afterwards, it removes the comment but also removes the ICE markup to indicate that changes have been made.

Again, great start. Let me know what I can do to help!

@benesch
Copy link
Contributor

benesch commented Mar 5, 2012

Thank you! Glad to help out—I'm really impressed with ICE.

Discussions are a primary design goal, not to worry! I'm envisioning a two-part system. The discussion system (what you see here) allows any user to mark any phrase for discussion. Then, users can comment upon that discussion in an entirely separate system, a la Google Docs comment replies. I think this follows the most common use case: content is stored in one table, and comments in another. Note I was initially referring to discussions as comments.

Splitting the system also allows us to squeeze out the bugs of the range selection. ICE's logic for tracking changes is horrifically complex, and I'm positive the mature discussion plugin will need some of that logic. And the commenting code can stay clean and pretty and use the discussion and discussionResolved hooks.

I think I squashed the bug where resolving a comment also resolves changes. At least, I can't reproduce it today, and I know I saw it last night. Edit: again related to #11. If you try with a comment you create yourself, resolving the comment works as expected.

@kishorenc, good call on those overlapping comments. Turns out it's [potentially] a bug in ICE (#11). ICE won't register the default comment hard-coded in the source, but if you add two entirely new comments it works pretty well. I'm thinking we follow the Google Docs comment flow model. If comments overlap, no bigs—just use the sidebar. Thoughts?

I also added an opacity to the highlight, so you can vaguely see where one comment begins and one ends.

@benesch
Copy link
Contributor

benesch commented Mar 5, 2012

Currently reworking my repository structure to separate my fork of ICE itself and this plugin. I'll post updated demos tonight--I accidentally stranded my latest fixes on my home computer. Oops.

@briantully
Copy link

Nikhil -

Good call on the Google Docs comment flow model. Very exciting stuff!

@benesch
Copy link
Contributor

benesch commented Mar 6, 2012

@briantully, thank you for your mock-up, too. I've always felt Google Docs comments were a bit too narrow for good discussion — your layout looks great.

@benesch
Copy link
Contributor

benesch commented Mar 6, 2012

Github Pages isn't updating properly, but here's the latest round of bug fixes: http://designbynikhil.com/ice/ice-discussions/demo/

Please try to break it!

@pkbarbiedoll
Copy link

Nikhil, thanks so much for your work on ice-discussions. I'm wondering if you have done any work on this project since March? What you describe with the google-docs style comments (as replies to document comments) sounds great.

@benesch
Copy link
Contributor

benesch commented Aug 2, 2012

In case anyone else is following, I haven't done any additional work on this plugin. @pkbarbiedoll or anyone else, please feel free to submit pull requests with any changes. I'll probably look into packaging this properly soon. Ideally, we'd have a separate commenting library that hooked into this plugin to provide full-featured, Google Docs style commenting.

@pkbarbiedoll I'll take a look at the errors with TinyMCE later today!

@ghost
Copy link

ghost commented Sep 6, 2012

I really like your plugin mate! Yet i have a hard time integrating the ice plugin along with your discussion plugin into drupal. It took me a day to figure out why your tinymce.html demo didn't work. As it turned out, I took the "wrong" ice editor_plugin.js because there where no plugins in the demo folder. Instead I took the ones from the ice github project which didn't contain your tweak to expose the inlineChangeEditor. I'm new to github and didn't know that the green folder icon is a link to another repo. So just in case somebody has the same problem, a short note in the readme file would be great.

Keep up the good work!

Cheers

@delambo
Copy link
Member

delambo commented Nov 13, 2012

Thanks, all. Nikhil's implementation looks good, but I don't think this is going to fit into ice proper. It would probably be better as a tinymce plugin.

@delambo delambo closed this as completed Nov 13, 2012
@benesch
Copy link
Contributor

benesch commented Nov 13, 2012

@DAWG22, so sorry for the confusion. Development has stalled from my end; if someone else wants to take the helm, I'm happy to grant commit access to the repo. Forking encouraged as always.

@delambo, works for me. I'll see if I can implement #23 soon.

vivekfantain pushed a commit to vivekfantain/ice that referenced this issue Oct 16, 2014
Update to latest NYTimes/ice's master.
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

No branches or pull requests

5 participants