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

Missing action hooks. #23

Open
esbanarango opened this issue Apr 12, 2016 · 5 comments
Open

Missing action hooks. #23

esbanarango opened this issue Apr 12, 2016 · 5 comments

Comments

@esbanarango
Copy link
Collaborator

There are certain situations where we have to perform some logic before opening the modal. An action hook like beforeOpen would be really handy to put this kind of logic; additionally, we could prevent the opening of the modal in this hook if for example we have a validation there.

@sethbrasile
Copy link
Owner

Wouldn't it be just as easy to perform whatever logic you would put there, inside the action you're using to open the modal? What would be the difference?

openModal() {
  this.get('remodal').open('modal1')
},
beforeOpenModal1() {
  // something that returns a promise
}

Would be the same as:

openModal() {
  somethingThatReturnsaPromise.then(() => {
    this.get('remodal').open('modal1');
  });
}

wouldn't it?

@esbanarango
Copy link
Collaborator Author

Yeah, that solution works when you’re opening the modal programmatically, but the case I’m facing is opening the modal using the openButton.

@sethbrasile
Copy link
Owner

That's a good point. I'm inclined to lean toward saying "that use-case calls for programmatically opening a modal", because I had envisioned the openButton's purpose as only being for simple "click and a modal opens" situations, but I can see where allowing that to be conditional (based on some validation or something) could be a good thing. In certain situations it could lead to cleaner implementation in the consuming app. I'm going to leave this one open and think about it a bit.

If anyone else has input, please chime in!

@sethbrasile
Copy link
Owner

Also, I'm assuming if we did this we would also add hooks for beforeCancel, beforeConfirm, and beforeClose.

@esbanarango
Copy link
Collaborator Author

Agree! We will also need to add those.

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

No branches or pull requests

2 participants