GetmeUK / ContentTools

A JS library for building WYSIWYG editors for HTML content.
http://getcontenttools.com
MIT License
3.96k stars 398 forks source link

DialogUI Cancel event not responding? #359

Closed d0brii closed 7 years ago

d0brii commented 7 years ago

Hi There, I was trying to add cancel button to the dialog though apparently is not working. Documentation said that is implemented, I'm not sure am I doing something wrong (works well for image and video dialog);

modal           = new ContentTools.ModalUI(true, false);
dialog          = new ContentTools.DialogUI("Dialog Window"),

dialog.addEventListener('cancel', (function(_this) {
  return function() {

    modal.hide();
    dialog.hide();
    if (element.restoreState) {
      element.restoreState();
    }
    return callback(false);
  };
})(this));

Thank you :)

anthonyjb commented 7 years ago

By default dialogs don't have a cancel button, you can close them using the close button and the escape key but if you want there to be a close button then you'll need to add it into the dialogs DOM element. That means you'll need to inherit from the DialogUI class and override the mount method. A good example is the Video dialog which adds an Insert button: https://github.com/GetmeUK/ContentTools/blob/master/src/scripts/ui/dialogs/video.coffee#L14

Once you add the button then you will then need to add an event handler for it to trigger the cancel event against the dialog, for example you can see how this is done for the standard close button here: https://github.com/GetmeUK/ContentTools/blob/master/src/scripts/ui/dialogs/dialogs.coffee#L208

d0brii commented 7 years ago

Thank you for your answer. Let's scratch cancel button for a moment, what's confusing me in this point is that close button (little x in right corner) isn't working, that after dialog is created and visible on the screen. I don't have any extra code just plain this (am I missing something?);

modal           = new ContentTools.ModalUI(true, false);
dialog          = new ContentTools.DialogUI('Dialog Window'),

editor.attach(modal);
editor.attach(dialog);
dialog.show();
modal.show();
anthonyjb commented 7 years ago

So the close button and escape key only trigger a cancel event the don't hide/remove the dialog or modal. Here's the a bare-bones example of how to support closing the dialog when the cancel event is triggered (similar to your initial code):

var editor = ...;
var modal = new ContentTools.ModalUI();
var dialog = new ContentTools.DialogUI('Dialog Window'),

editor.attach(modal);
editor.attach(dialog);
dialog.show();
modal.show();
dialog.modal = modal;

dialog.addEventListenter('cancel', function () {
    this.modal.hide();
    this.hide();
});
anthonyjb commented 7 years ago

Actually having tested my code this doesn't work as you stated :( The dialog's mount method is missing a call to the_addDOMEventListeners - I've just never noticed because mount is always overridden (except in this case of course). So here's a work around for now and I'll flag this an issue to resolve (sorry for the confusion):

var editor = ...;
var modal = new ContentTools.ModalUI();
var dialog = new ContentTools.DialogUI('Dialog Window'),

editor.attach(modal);
editor.attach(dialog);
dialog.show();
modal.show();
dialog.modal = modal;
dialog._addDOMEventListeners();

dialog.addEventListener('cancel', function () {
    this.modal.hide();
    this.hide();
});
anthonyjb commented 7 years ago

@d0brii sorry quick update on this - this is actually required behaviour. So the reason is that classes that inherit from the editor will typically call the the parent DialogUI's mount method in their own mount method (because they want the default dialog UI components). However if DialogUI calls _addDOMEventListeners in the base mount method then the DOM elements set up by the overriding mount method wont exist which can (and does in some cases) raise an issue because we try to add event listeners to DOM elements that haven't yet been created.

There's a note explaining this in the code (which I missed on first read through) explaining this, see here: https://github.com/GetmeUK/ContentTools/blob/master/src/scripts/ui/dialogs/dialogs.coffee#L18

d0brii commented 7 years ago

Thank you :) needless to say, it's working now.