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

'change' event fires when setting Ace programmatically #503

Closed
matthew-dean opened this issue Nov 14, 2011 · 12 comments
Closed

'change' event fires when setting Ace programmatically #503

matthew-dean opened this issue Nov 14, 2011 · 12 comments

Comments

@matthew-dean
Copy link

For code like this:

editor.getSession().on('change', function() {
// Do something
});

I would expect this event to only fire when there's a user interaction. However, I've found that it fires when I set the value of the editor programmatically, which is not consistent with something like, say, the DOM. (If you add a 'change' event listener for an input field, it will not fire if we set the value with JavaScript.)

Can this be fixed, and is there a workaround or a different event that fires when the value of the editor has updated, but not programmatically?
@Gissues:{"order":33.540372670807756,"status":"backlog"}

@innotekservices
Copy link

Same reason why I dropped CodeMirror for ACE. But ACE's code is a lot easier to read. I made a patch to somewhat fix this. The change event is still fired, but I added a boolean property to the data object: userTyped. Will be true if the user typed or pasted text into the editor. False if it was programmatically entered. This works for my specific use case in Chrome.

This is for the ace-uncompressed.js file from 0.2.0: http://dl.dropbox.com/u/310281/ace-patch.txt

@nightwing
Copy link
Member

i don't think there is a good way of separating 'programmatical' and user changes (e.g. to which type belongs text entered by autocomplete widget?)
and adding userTyped adds lot's of bookkeeping all over the place

if you want only text input events you can add dom "input" event listener to editor.container
or if you want to filter out only you your own setValue call add a flag (similar to $blockScrolling in editor.js)
it will work since 'change' event is synchronous

@nightwing
Copy link
Member

either use method described in #1547 (comment) or

editor.on("change", function(e) {
  if (editor.curOp && editor.curOp.command.name) console.log("user change");
  else console.log("other change")
})

@groubis
Copy link

groubis commented Sep 25, 2015

The snippet posted by nightwing does not work as it should while copy-paste is applied. Another solution that may solve the issue is something like that e.g with jquery:

editor.getSession().on('change', function() {
    if ($(document.activeElement).closest("div").attr("id") == "editordiv") {
        console.log("editor was changed by user typing or copy paste");
    } else {
        console.log("editor was changed programmatically");
    }
})

@davidthornton
Copy link

Well

$(document.activeElement).closest("div").attr("id") == "editordiv"

doesn't work if you have two editor divs and they are both focus()ed, you're using applyDeltas and you're trying to sync them. If I'm using this wrongly, let me know. But I'm pretty sure I need a different solution.

@nightwing
Copy link
Member

The bug with paste not having a command have been solved some time ago.
Checking for activeElement won't work, and it's generally a bad idea to access dom from change event listener, since it will degrade editor performance.

Use either #1547 (comment)
or #503 (comment)

@davidthornton
Copy link

Can I use #1547 with applyDeltas though? and with #503, was groubis correct when talking about copy/paste (because I need that too?)

I could definitely be going about this syncing thing the wrong way, as well - so don't rule that out yet haha :)

@davidthornton
Copy link

I just tried

editor.on("change", function(e) {
  if (editor.curOp && editor.curOp.command.name) console.log("user change");
  else console.log("other change")
});

verbatim and when I pasted, it said "other change". I'm on Safari 9.0.1. Is that a supported platform?

@nightwing
Copy link
Member

Maybe you are using old version?
Approach described in #1547 works with applyDeltas.

@davidthornton
Copy link

Yep so I though you meant adding the {silent:true} object to applyDeltas(delta, {silent:true}). I am updated to the latest version now, and I guess my problem is that even though the change event is synchronous - I seem to be running into a concurrency issue where the "silent" lock will lock out remote deltas while the local user is typing.

@nightwing
Copy link
Member

Sorry there was an error in the example.
It should say

silent = true
editor.session.setValue("some text");
silent = false

and in change listener

if (silent) return

so silent is a variable in your module, not something you pass to ace.

Note that using applyDeltas is not enough for syncing, as you'll need to change deltas to take account that document to which you applying is most likely different from the document for which they were created.

@davidthornton
Copy link

Yes, the bug I was experiencing was unrelated to the actual API of ace. Ace is ace - I was experiencing a concurrency issue with multiple (collaborative) editors in the same document, coordinated by a central server.

That problem is non-trivial, requiring something called operational transformation; itself well beyond the scope of Ace. For those interested: https://en.wikipedia.org/wiki/Operational_transformation.

Sorry for any inconvenience I've caused on your great project :/ haha!

warpech added a commit to Juicy/juicy-ace-editor that referenced this issue Oct 30, 2018
changes externally (by setting juicyAceEditor.value = "..") should not result in change event being triggered
this avoids unnecessary notifications to two way data binding libraries such as Polymer
solution is inspired by ajaxorg/ace#503
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