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

ModalPane's callback doesn't work #36

Closed
siovene opened this issue Sep 29, 2012 · 5 comments · Fixed by #37
Closed

ModalPane's callback doesn't work #36

siovene opened this issue Sep 29, 2012 · 5 comments · Fixed by #37

Comments

@siovene
Copy link

siovene commented Sep 29, 2012

Hi,
I was having trouble with the ModalPane's callback, and I created a minimal app in a jsfiddle:

http://jsfiddle.net/siovene/n29y7/

The callback function is never called on click, but only on keypress if I hit ESC.

Thanks in advance for looking into this.

@bradleypriest
Copy link
Member

Yeah, the modal pane has been broken for quite a while with ember 1.0.pre whilst setting a rootElement on the application because it is appending the modal pane outside of the applicationView.
I meant to have posted an issue a while ago.

This is how I've currently hacked it to work in my app. I'll get a test and PR (a different fix though) for it up in the next couple of days, thanks for the minimal app.

Bootstrap.ModalPane.reopen({
  _triggerCallbackAndDestroy: function(options, event){
    this._super(options, event);
    App.router.applicationController.set('modal', null);
  }
});

Bootstrap.ModalPane.reopenClass({
  popup: function(options) {
    var modalPane;
    if (!options) options = {};
    modalPane = this.create(options);
    App.router.applicationController.set('modal', modalPane);
    return modalPane;
  }
});

@bradleypriest
Copy link
Member

Here's a better short-term fix:

Bootstrap.ModalPane.reopenClass({
  popup: function(options) {
    var modalPane;
    if (!options) options = {};
    modalPane = this.create(options);
    modalPane.appendTo(App.rootElement);
    return modalPane;
  }
});

@siovene
Copy link
Author

siovene commented Sep 30, 2012

I haven't tested the following, but won't appending the backdrop to the root element rather than body break the z-indexing in IE?

@bradleypriest
Copy link
Member

@siovene Does PR #37 fix your issue?

@dmathieu
Copy link
Contributor

dmathieu commented Oct 4, 2012

While there's no news from @siovene, I'm merging #37 anyway.
The modal should anyway be scoped inside the .ember-application.

About the z-index problem in IE, it would be easily fixable in CSS by adding a higher lower z-index to .ember-application. So it doesn't seem to be a real problem.

@dmathieu dmathieu closed this as completed Oct 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants