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

ckeditor helper not working? #111

Open
fartymonk opened this issue Aug 2, 2016 · 3 comments
Open

ckeditor helper not working? #111

fartymonk opened this issue Aug 2, 2016 · 3 comments
Labels

Comments

@fartymonk
Copy link

Hi, I'm new with this plugin and currently trying to use it alongside CKEditor.

Following the guide, I've included the javascript for ckeditor, dirtyform, and dirtyform's ckeditor helper in the page (in that order). This is the sample pen I've made to show how I did it : http://codepen.io/fartymonk/pen/qNrajO

And since I can't found any additional instruction about how to use the helper, I assume it's automatically initiliazed along with dirtyform. But when I changed the contents of ckeditor textarea, the form doesn't become dirty (although a separate check for ckeditor said it's dirty).

Is there anything extra I should've done to make the helper 'work'?

Thanks.

@NightOwl888
Copy link
Collaborator

Not sure what your question is, the sample you provided is working as expected. However, you have not put any links on the page to navigate off of it, so you don't see the dialog.

If you edit the CKEditor and then try to type another address in the address bar of the browser you can see that it indeed is working. Although, the code editor you provided is also doing the same thing, so their dialog pops up first, and the Dirty Forms dialog shows after you click "leave" on the first dialog. It is better if you make sample code in standalone HTML files for Dirty Forms since code sharing editors tend to already do dirty checking (among other things) which blocks Dirty Forms from working right.

As for the documentation for the helper, it is here

@fartymonk
Copy link
Author

fartymonk commented Aug 3, 2016

Thank you for the response and the helper documentation. I've read that and it doesn't say anything about extra coding needed, only I need to include the js stuff and initialize dirtyform as usual.

I actually need the dirty check before the user decided to leave, which is the reason why I didn't redirect the form to any page. At least, I need to know if the form is dirty when user is changing the data, or when they focused out the field.

To make things clear, this is what I did to reproduce my problem (the pen still have the same problem, actually) :

  1. Change the ckeditor content (until there's a 'dirty' red label on the ckeditor label)
  2. Click outside the field, or move to any other field. But don't change other fields value.
  3. Check the header. It should say "Dirty!" since the ckeditor is dirty, but strangely, it still say "Clean!"
  4. Opt: changing other fields value does make the header say "Dirty!", but if their value is reverted back to the original except for ckeditor, the header say "Clean!" even though ckeditor is still dirty.

Changing other fields are okay, they're detected correctly. The header changed to "Dirty" as intended. Only ckeditor that doesn't trigger the form dirty-ness.

@NightOwl888
Copy link
Collaborator

Now I see what you mean. This is a bug.

The issue is that this line:

var changed = (isDirty !== ($form.hasClass(dirtyClass) && $form.find(':dirty').length === 0));

does not consider helpers in the dirty/clean check. I think that was originally because of performance concerns, but since then isDirty method was optimized a lot better by returning true on the first dirty field instead of checking all of them. I think this can be fixed by changing that line to:

var changed = (isDirty !== $form.dirtyForms('isDirty'));

But it will need to be checked to see how well it performs, and double-checked that this fix doesn't cause infinite recursion.

I suggest you give it a try, and if the fix works without issues to create a pull request with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants