Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Add onOpen and onDismiss handlers to directive #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jmwohl
Copy link

@jmwohl jmwohl commented Aug 2, 2016

Adds a couple important missing options to the directive:

  • A hook called when the confirm dialog opens (useful for any corresponding UI updates)
  • A hook called when the confirm dialog is dismissed

@Schlogen - Please consider these changes, as they are required for our application! I didn't commit the minified file, happy to do that if you like.

@jameskleeh
Copy link
Owner

What is the use case for onOpen ?

The onDismiss option really only makes sense while it is being used as a directive. You can simply change to use $confirm directly and handle the .then how you want.

@jmwohl
Copy link
Author

jmwohl commented Aug 2, 2016

Right, both options are for the directive — neither are necessary using the service directly. We switched to the directive from the service in order to clean up the logic in our app a bit, keeping user-facing messaging in the markup. Is there any reason to prevent the handling of confirmation dismissal via the directive?

The onOpen hook is useful when there are UI updates that need to happen which correspond to the opening of the dialog. Specifically, in our case we're sometimes triggering the dialog from a popover, and we need the popover to close when the confirm dialog opens.

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

Successfully merging this pull request may close these issues.

None yet

2 participants