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

Authorship Module does not account for History Module Undo Redo #1371

Closed
sferoze opened this issue Mar 20, 2017 · 9 comments
Closed

Authorship Module does not account for History Module Undo Redo #1371

sferoze opened this issue Mar 20, 2017 · 9 comments

Comments

@sferoze
Copy link
Contributor

sferoze commented Mar 20, 2017

This applies when using both the history and authorship module. When undoing or redoing a change, the original author of the change is not preserved. You become the new author when undoing and redoing any change.

Here is the fix I am currently using. It checks if the delta already has an author attribute. If so it uses that instead of its own property for authorId.

this.quill.on(Quill.events.EDITOR_CHANGE, (eventName, delta, oldDelta, source) => {
      if (eventName == Quill.events.TEXT_CHANGE && source == 'user') {
        let authorDelta = new Delta();
        delta.ops.forEach((op) => {
          if(op.delete) {
            return;
          }
          if(op.insert || (op.retain && op.attributes)) {
            // Add authorship to insert/format
            op.attributes = op.attributes || {};

            // These 2 lines below are the fix I implemented
            op.attributes.author = op.attributes.author || this.options.authorId;
            let authorFormat = { author: op.attributes.author };

            // Apply authorship to our own editor
            authorDelta.retain(op.retain || op.insert.length || 1, authorFormat);
          } else {
            authorDelta.retain(op.retain);
          }
        });
        this.quill.updateContents(authorDelta, Quill.sources.SILENT);
      }
    });
@WinMinTun
Copy link

@sferoze I v just written a plugin for this.. History (Ctr+Z & Ctr+Y) is working well together with Authorship. Pls check and comment https://github.com/WinMinTun/quill-authorship

@sferoze
Copy link
Contributor Author

sferoze commented Jun 6, 2017

@WinMinTun

I looked at your code and there is one bug you missed. I originally missed this too when trying to get the history module to work....

Basically if you undo few steps.... and then start editing text that a previous collaborator added, the text you add is tagged as the previous collaborator and not you.

In my code I have something like this

           if (!!Session && !!Session.get('historyActive')) {
              op.attributes.author = op.attributes.author || this.options.authorId;
            } else {
              op.attributes.author = this.options.authorId;
            }

as you can see if I am undoing and redoing it will use the original author. But whenever I start typing it is fored to use my own authorId.

@WinMinTun
Copy link

@sferoze I tried to remake the bug you had mentioned: Undo a few steps -> edit other author's text. But still it is ok. Tested Quill version is 1.2.4. Could u pls tell me more so that I can remake exactly the bug you mentioned? Which Quill version u find the bug on? Thanks

@sferoze
Copy link
Contributor Author

sferoze commented Jun 7, 2017

@WinMinTun Ok I looked at the code again and realized that I described the wrong situation... it should work when you undo and then edit another authors text.

but here is the correct bug description.

if you have a document with multiple authors. and then undo to the beginning. and then redo all the undo steps again you will notice that you loose the history of all the previous authors. It will insert the operations again under your own authorId. I believe the history undo and redo should preserve the original authors

@sferoze
Copy link
Contributor Author

sferoze commented Jun 7, 2017

@WinMinTun

here is how I solved it

this.quill.on(Quill.events.EDITOR_CHANGE, (eventName, delta, oldDelta, source) => {
      if (eventName == Quill.events.TEXT_CHANGE && source == 'user') {
        let authorDelta = new Delta();
        delta.ops.forEach((op) => {
          if(op.delete) {
            return;
          }
          if(op.insert || (op.retain && op.attributes)) {
            // Add authorship to insert/format
            op.attributes = op.attributes || {};
            if (youAreUsingAnUndoOrRedoStep) {
              op.attributes.author = op.attributes.author || this.options.authorId;
            } else {
              op.attributes.author = this.options.authorId;
            }
            let authorFormat = { author: op.attributes.author };
            // Apply authorship to our own editor
            authorDelta.retain(op.retain || op.insert.length || 1, authorFormat);
          } else {
            authorDelta.retain(op.retain);
          }
        });
        this.quill.updateContents(authorDelta, Quill.sources.SILENT);
      }
    });

@sferoze
Copy link
Contributor Author

sferoze commented Jun 8, 2017

@WinMinTun

I am wondering why in your code you have this line

    this.styleElement.innerHTML = css; // bug fix
    // this.styleElement.sheet.insertRule(css, 0);

why did you use this.styleElement.innerHTML = css; instead of this.styleElement.sheet.insertRule(css, 0); ?

@seanippolito
Copy link

Is this complete? Would like to see the Authorship module implemented so the Cursors module can be updated.

@WinMinTun
Copy link

@seanippolito I m encountering a problem with Chinese keyboard which show dropdown to choose Chinese characters. Let you update on it.

@benbro
Copy link
Contributor

benbro commented Feb 2, 2024

This is related to #1049 and not core Quill

@benbro benbro closed this as completed Feb 2, 2024
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

4 participants