angular-ui / ui-tinymce

AngularUI wrapper for TinyMCE
MIT License
488 stars 371 forks source link

do not trigger events when updating tinymce #230

Open analyst74 opened 8 years ago

analyst74 commented 8 years ago

...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.

deeg commented 8 years ago

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.

deeg commented 8 years ago

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.

analyst74 commented 8 years ago

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. This was previously avoided by calling editor.save which triggers an event that creates a new stack every time, this has a side effect of making undo to behave differently than vanilla tinymce.

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.

deeg commented 8 years ago

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.

analyst74 commented 8 years ago

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.

deeg commented 8 years ago

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?

analyst74 commented 8 years ago

Oops, never done that before, but here you go!

analyst74 commented 8 years ago

Interesting, the timeout was previously needed, but does not seem necessary anymore after merging with some recent changes. They are removed now.