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

Add support for custom async confirmation handler #18

Merged
merged 2 commits into from
Nov 6, 2019

Conversation

krasnoukhov
Copy link
Contributor

@krasnoukhov krasnoukhov commented Sep 7, 2018

This could be useful to people who use custom-styled dialog modals. Obviously this will only work if ember transition is triggered (not a hard refresh).

  • Better test for _transitionConfirmed state
  • Documentation?

Example using sweetalert:

confirmation

@krasnoukhov
Copy link
Contributor Author

krasnoukhov commented Sep 7, 2018

@jasonmit @blimmer Please let me know if you think this will be useful, I'm wiling to polish it up 😆

@krasnoukhov
Copy link
Contributor Author

Hey, any feedback here? I'm wiling to rebase/polish this in case it can be merged

@blimmer
Copy link
Collaborator

blimmer commented Dec 16, 2018

Hi @krasnoukhov - I think it'd be OK to expose logic like this. The only part I'm not sure about is the _transitionConfirmed state. What do you think about simplifying this logic down to be only the customConfirm method that takes a promise? That way, state wouldn't need to be managed with the _transitionConfirmed property.

@krasnoukhov
Copy link
Contributor Author

@blimmer Thanks for the feedback! Maybe I'm missing something here, but _transitionConfirmed exists just so we can first abort the transition and then either retry or do nothing. I'm not sure how to achieve this kind of flow with just the promise. Any ideas?

@tylerturdenpants
Copy link
Collaborator

@jasonmit @blimmer is there any way that we could get some movement on this? I'd appreciate this being available upstream.

Also, do you need a new maintainer? I'd be willing to do some of the chore work to make sure this is available for Octane.

@jasonmit
Copy link
Owner

jasonmit commented Nov 1, 2019

@tylerturdenpants I would love another maintainer :) I am currently not active in the Ember space and would love all the help I can get so that the community does not suffer.

I'll work on adding you here and on npm now. Feel free to ping on twitter @_jasonmit if you have any questions/issues that need my attention.

@tylerturdenpants
Copy link
Collaborator

@jasonmit Thank you very much. I handle a few other ember related things so I know my way around. I will make you proud.

@jasonmit
Copy link
Owner

jasonmit commented Nov 1, 2019

Should be all set now - invites sent :) Much appreciated!

@tylerturdenpants tylerturdenpants merged commit 4164d10 into jasonmit:master Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants