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

Multiple Cursors Module #918

Closed
Joeao opened this issue Sep 6, 2016 · 27 comments
Closed

Multiple Cursors Module #918

Joeao opened this issue Sep 6, 2016 · 27 comments
Labels

Comments

@Joeao
Copy link
Contributor

Joeao commented Sep 6, 2016

Multi cursor support was a feature I used on my project pre 1.0 and is great for use with frameworks such as YJS which allows concurrent editing by multiple users at the same time.

I look forward to the feature to be reintroduced now that 1.0 has officially launched and have set up this issue to track progress and so that people can offer their support to the reintroduction of a great feature.

@jhchen jhchen added the feature label Sep 7, 2016
@benbro
Copy link
Contributor

benbro commented Sep 10, 2016

Initial untested version of the multi-cursor module updated from 0.2:
https://github.com/benbro/quill/tree/multi-cursor

To be useful we probably need the authorship module too.

@sferoze
Copy link
Contributor

sferoze commented Sep 29, 2016

@benbro I remember the authorship module from previous Quill.

I used it to show which edits where made by which person when 2 people having access to edit a specific document.

I lock it down so only one person can make an edit at a time though. works for what I need to do, so I don't need multiple cursors.

But still showing which edit was made by which user would be cool.

I remember the authorship module would add a class to contain the elements that were added by a user.

I wonder if the authorship module will be introduced as a separate plugin?

@mmacfadden
Copy link

I am interested in this as well. We don't have a ton of cycles this month, but we might be interested in contributing to bringing this module back, if it hasn't been picked up yet.

@mmacfadden
Copy link

@benbro Do you need any help?

@nateps
Copy link

nateps commented Oct 7, 2016

Very excited about this feature being reimplemented.

As an improvement, would also be excellent if the feature supported displaying a selection range per cursor, similar to Google Docs.

@benbro
Copy link
Contributor

benbro commented Oct 7, 2016

@mmacfadden the multi-cursor module depends on the authorship module so you might want to update it to work with Quill 1.0.0
https://github.com/benbro/quill/blob/multi-cursor/modules/multi-cursor.js#L82
https://github.com/quilljs/quill/blob/0.20.1/src/modules/authorship.coffee

@nateps getBounds gives you a rectangle. To be able to display a selection range you need to break the selection into multiple rows and get the bounds of each part.

@mmacfadden
Copy link

@benbro I will take a look...

@nateps Agree. I think cursor, selection, and authorship make a nice collaborative experience. I am completely willing to look at all three. I am finishing up a few more tasks over the next week or two, but then I this becomes high on my priority list. Willing to dig in myself or help others.

@jhchen
Copy link
Member

jhchen commented Oct 7, 2016

It's worth mentioning the reason cursors depended on authorship is because of this #746. Basically, if the user inserts a character next to the same character ie inserting a 'a' into 'aaaa', Quill does not know which position the insertion happened. Authorship can solve this because those existing "a"s would have an author associated, but the new "a" you just inserted would not (yet).

@sferoze
Copy link
Contributor

sferoze commented Oct 7, 2016

Looks like first step is to update authorship module to work with 1.0

I am looking at the code here:

https://github.com/quilljs/quill/blob/0.20.1/src/modules/authorship.coffee

and comparing it with the docs here:

https://quilljs.com/docs/modules/

Changes I notice that the update will need.

Instead of Quill.registerModule its just Quill.register

We don't need to export the module.

Not really sure what else...

Am I looking at the right place in the docs on how to create a module for Quill 1.0?

@benbro
Copy link
Contributor

benbro commented Oct 7, 2016

Module guide:
http://quilljs.com/guides/building-a-custom-module/

You'll also need to make Quill handle the 'author-' class.

@sferoze
Copy link
Contributor

sferoze commented Oct 7, 2016

@benbro ok cool thanks missed that guide.

So it looks like this is the minimum that's needed to make a Quill module

// Implement and register module
Quill.register('modules/counter', function(quill, options) {
  var container = document.querySelector('#counter');
  quill.on('text-change', function() {
    var text = quill.getText();
    // There are a couple issues with counting words
    // this way but we'll fix these later
    container.innerHTML = text.split(/\s+/).length;
  });
});

so after looking at the code here

https://github.com/quilljs/quill/blob/0.20.1/src/modules/authorship.coffee

It looks like everything in the constructor and all the functions will go into the register modules functions. The options are passed in so we can use that.

After quick looks looks like I could use the authorship code pretty much as is, and just adjust it to fit the new format.

@kminehart
Copy link

How's this looking? Haven't seen any commits for a while; I'd love to contribute.

@sferoze
Copy link
Contributor

sferoze commented Nov 27, 2016

@kminehart the first step to get Multiple Cursors working is to get the authorship module to work.

#1049

@benbro has already ported the code from quill 0.2 to be compatible with the new Quill

But it has issues.

I've been working a lot with this issue because I am using the authorship module in my app and I have added a hack fix to patch the issue, but I am not sure if it will have unwanted side effects, although I haven't noticed any yet.

You can track the issue here: #1057

Basically the authorship module keeps track of who edited what content by wrapping the text in a span element with a class unique to that author.

But an issue with html5 contenteditables is typing in span elements makes the cursor disappear. So we need to think of a solution. I posted mine fix where the issue is being tracked (#1057).

Let me know if you have any ideas or suggestions.

@pedrosanta
Copy link

pedrosanta commented Apr 18, 2017

Hi all, @jhchen, been following this thread with close attention, as I'm currently trying to wrap up multi-cursors as well, and published the Quill cursors module we've been using over at Reedsy - that we initially based of 0.20 cursors module.

It's available at https://github.com/reedsy/quill-cursors (or on NPM as quill-cursors).

I'm testing this out on a simple scenario, that I'll be publishing soon on an own repo (including a simple Node/ShareDB server handling the sync), and I'm still getting a few cursor un-sync issues when used collaboratively - I suspect it's a nasty racing condition of sorts, more to that later.

The module already handles the selection display and all (mentioned by @nateps).

Anyway, just to put this out here, I'll be more than glad to help integrate it on Quill. 🙂 (I'll ping back when I got a repo with the server+client test scenario.)

PS: Although I've been following the discussion, and really, also got issues with what's described on #746, I've taken a different approach than resorting to an author attribute (as I wasn't particularly keen into adding that level of metadata do the documents) but taken a rather exotic approach of implementing a 'character look back' mechanism to update/shift all cursors present in a character series. It isn't an approach I like still, but it's the compromise I made at this point, but, open to anything.

@jhchen
Copy link
Member

jhchen commented Apr 18, 2017

I do think multi-cursor and authorship modules can and should be independent from one another. And to be clear I'm talking about the modules -- both will need to know who wrote what from the server, but one should be able to use one or the other.

It was previously the case that Quill incorrectly emitted the insert location as described in #746 so the authorship module was used to correct this. I think this was the wrong approach and this root issue was fixed with the addition of an edit location suggestion in the diff library that Delta and thus Quill used. If you are saying @pedrosanta Quill still reports insertions in the wrong location, then please post details and reproductions steps in a separate issue and it will be treated as a bug and fixed.

@pedrosanta
Copy link

pedrosanta commented Apr 18, 2017

Hey @jhchen , yep that's exactly what I'm working to put up at the moment - a repo that mimics the test scenario I'm using to reproduce the issue with a very simple Node/ShareJS backend to sync document and cursors. Still gathering all the information to save everyone's time. 🙂 Just pinged in to share the module though.

Also, still need to look closer to jhchen/fast-diff#2 too.

@deepakjtrigent
Copy link

Hi @jhchen

Any idea on when this module might get released?
I am dependent on the multi cursor support which was a great feature and looking forward for it to be back as soon as possible.
Awesome editor by the way. Thanks.

@pedrosanta
Copy link

pedrosanta commented Apr 26, 2017

@deepakjtrigent I don't think there is an estimated date for that. Just keep following this issue. I'm sure any news will be posted here.

@pedrosanta
Copy link

pedrosanta commented Apr 26, 2017

@jhchen:

I do think multi-cursor and authorship modules can and should be independent from one another. And to be clear I'm talking about the modules -- both will need to know who wrote what from the server, but one should be able to use one or the other.

Indeed, agree. IMO, one of the issues with having to use authorship module to use cursors was (1) the additional metadata it added to the document/delta (the author attribute) and (2) the <span> overuse it added to the contents as well. Which leads me to...

It was previously the case that Quill incorrectly emitted the insert location as described in #746 so the authorship module was used to correct this. I think this was the wrong approach and this root issue was fixed with the addition of an edit location suggestion in the diff library that Delta and thus Quill used.

Yep, authorship also seemed a 'weird' work around to fix this to me. I've taken the time to look to jhchen/fast-diff#2 and I'm very very happy that it was fixed in an pretty elegant way (kudos to @jhchen and @dmonad, thanks! 👍). I've ran some tests and removed the 'lookback' fix/hack I had on reedsy/quill-cursors - this was in place because this module was in use with Quill 0.20.x in the past.

I'm still working to publish a working test scenario of multi-cursors using ShareDB, will run a few more tests with it and I'll get back with news.

@pedrosanta
Copy link

pedrosanta commented May 19, 2017

Hi all, just to ping in that I've recently finished to put up a working example of multi cursor sync using a ShareDB backend and that is available at:

https://quill-sharedb-cursors.herokuapp.com

I'm still trying to document it better, but you can follow up the repository over at https://github.com/pedrosanta/quill-sharedb-cursors. Note, I'm using the reedsy/quill-cursors for this example - and also, I'm entirely open to contributing with code for a 1.x cursors module for Quill if you think it might be worth it.

Still has a couple of glitches sometimes (leaves cursor behind, cursors skips to a few positions from where the other client has it), but I think that could be a racing condition between the cursor update/sync through its own socket and the updates that the cursors module does based off the operations/text-change deltas it receives, and (definitely) other causes I haven't managed to pinpoint just yet.

One thing that it seems to me that could benefit this is the 'concept' of ephemeral data @nateps is suggesting on share/sharedb#138, as this way all cursor update info would be sent through the same pipe/at the same time.

Meanwhile, I'll continue to try to find and debug those glitches the best and can and continue to follow up the discussion.

@sferoze
Copy link
Contributor

sferoze commented May 19, 2017

@pedrosanta wow this is cool work. I will be following the progress. I have quill running in my meteor app and would like to integrate but still a little lost on how I would set this up. Maybe if I have more time I can get it working. If you ever get around to making some rough documentation that would be helpful as well.

@pedrosanta
Copy link

pedrosanta commented May 19, 2017

@sferoze That repository is the example with all the pieces joined together. Have you checked the actual reedsy/quill-cursors? It is fairly documented and can be made to work with any backend, I think.

Although I would say, that it seems to me this might come sooner than later on Quill, and if you can wait probably it's best to rely on it then. (I know I will when it's available. 🙂)

@seanippolito
Copy link

Is there an update on this? I still don't see a multi-cursors module incorporated in Quill.

@jeffbryner
Copy link

FYI: Not a multi-cursors module, but this does support reactive, realtime editing in quill via meteor:
http://quill-realtime-demo.jeffbryner.com:3000/

Source: https://github.com/jeffbryner/quill-reactive

Posted here since this issue is the most comprehensive docs I found while looking for something like this ;-]

@Jbithell
Copy link

Also checkout https://docs.pusher.com/textsync which is based on Quill

@Joeao
Copy link
Contributor Author

Joeao commented Aug 1, 2018

@seanippolito I'm having success with the quill-cursors module

@quill-bot
Copy link

Quill 2.0 has been released (announcement post) with many changes and fixes. If this is still an issue please create a new issue after reviewing our updated Contributing guide 🙏

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

No branches or pull requests