-
Notifications
You must be signed in to change notification settings - Fork 370
do not trigger events when updating tinymce #230
base: master
Are you sure you want to change the base?
Conversation
I still have not been able to reproduce the problem. Mind posting plunkers with and without the problem so I can see it broken and fixed? No problem getting this in, just want to confirm this is fixing an issue. |
I do see the issue now, sorry about that. Unfortunately, while this does seem to fix the issue you describe, it breaks other things. Take a look at this plunker with your fix. Some issues I see:
I am happy to merge this in if we can come up with a solution which does not break other stuff. I will continue to think about this. |
I took a deeper look into tinymce, the first problem seems to be that 'change' event is only fired on the first character typed of an Undo stack, and not fired in subsequent character changes. The second problem seems to be a tinymce 'bug', you can see the same behaviour on their homepage demo by clicking on bold button a few times on an empty line, then look at the source, a few empty tags will be created there. I'll have to think about how to properly fix this behaviour. |
I'm happy to not worry about that then, and file a bug with TinyMCE instead. The model still needs to update correctly in normal use though. |
cool, I added more changes and merged with latest. Main change is listening to KeyUp event instead of change event, which will be fire on all keystrokes instead of the first sequence of keystrokes of an Undo level. I also removed the call to editor.save, which I believed was used to make 'change' event work, hence no longer necessary. But if it serves other purposes, let me know and I can add it back with no_events argument. |
It looks like you update the pull request incorrectly. I am seeing changes which should already be in master showing up. Can you squash this down to one commit and make sure only your changes show up in changes tab? |
Oops, never done that before, but here you go! |
|
||
it('should not trigger event on KeyUp', function(done) { | ||
compile(); | ||
setTimeout(function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the setTimeouts needed here?
…highlight those buttons properly. also make undo button delete whole word/sentence as opposed to one character at a time
Interesting, the timeout was previously needed, but does not seem necessary anymore after merging with some recent changes. They are removed now. |
Fix (based on angular-ui#230) - Only save editor when editor is marked as stale (like done in debounce, which should really be using the same code) - Listen to keyup for new keystrokes that are sometimes missed by changes event - Do not file events when updating model
...which prevents bold/itlic button to not gain focus on empty lines due to markCaretContainersBogus() function in tinymce being called. The is to fix issue #229.