Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

do not trigger events when updating tinymce #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

do not trigger events when updating tinymce #230

wants to merge 1 commit into from

Conversation

analyst74
Copy link

...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
Copy link
Contributor

deeg commented Feb 15, 2016

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
Copy link
Contributor

deeg commented Feb 16, 2016

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:

  • Model is not always updated when it should be.
  • An extra tag can be left with no content inside.
    • With no content in the editor, hit bold, and then hit bold again, and then begin typing.

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
Copy link
Author

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
Copy link
Contributor

deeg commented Feb 17, 2016

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
Copy link
Author

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
Copy link
Contributor

deeg commented Feb 17, 2016

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
Copy link
Author

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


it('should not trigger event on KeyUp', function(done) {
compile();
setTimeout(function() {
Copy link
Contributor

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
@analyst74
Copy link
Author

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

KurtWagner added a commit to Accelo/ui-tinymce that referenced this pull request Feb 22, 2018
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants