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

Modal does not close on confirm? #13

Open
jochendaum opened this issue Jan 9, 2015 · 5 comments
Open

Modal does not close on confirm? #13

jochendaum opened this issue Jan 9, 2015 · 5 comments

Comments

@jochendaum
Copy link

Hi,

I've downloaded confirm_with_reveal.js just now and implemented like this:

  • Foundation from here: https://packagist.org/packages/bmatzner/foundation-bundle
  • script src="/bundles/bmatznerfoundation/js/foundation/foundation.js"></script>
    script src="/bundles/bmatznerfoundation/js/foundation/foundation.reveal.js"></script>
    script src="/bundles/agrihealthahp/js/offlineHandler.js"></script> <-- thats mine
    script src="/bundles/agrihealthahp/js/confirm_with_reveal.js"></script>
  •               button data-confirm="{"ok":"Yup","cancel":"Nope"}">Delete /button>
    

I catch the confirm event with:

jQuery('#' + model + '_edit .form_actions.delete').show().on('confirm.reveal', null, { name: model, rowId: rowId }, call.deleteRecord );

which works fine. However the modal does not close, I'm using

jQuery('.reveal-modal').hide();
jQuery('.reveal-modal-bg').hide();

to close it manually.

It is closing on Cancel.

Any idea why this would be happening?

@jletourneau
Copy link
Contributor

Currently, the plugin does not close the dialog when the confirmation button is pressed — this was by design, for what I felt was the more common case of these dialogs triggering full page refreshes as opposed to Ajax or front-end MVC updates (the idea being that leaving the dialog up while the new page loads prevents any other actions from being taken in the mean time if the response is not immediate).

I will see about adding a configuration option for this behavior, probably something along the lines of close_after_ok or the like, for cases where people want to use this to confirm actions that don't entail a page refresh.

@jochendaum
Copy link
Author

Cheers, I can live with that. However it it was to close by default, the people using it to link to other pages would not have any downside either?

@jletourneau
Copy link
Contributor

The only downside to closing the dialog by default when the OK button is hit is that if the dialog is confirming (for example) deletion of a resource, or some other important action, and the server response takes more than a few tenths of a second to refresh the page in the user's browser, the user may mistakenly think that the action was carried out via Ajax, and navigate off the page before the response arrives.

I recognize the value of having a close_after_ok option and will look into adding that, but I think the safer or more conservative setting is to have that behavior as a configuration option that must be explicitly chosen by the developer using the plugin.

@MandicaAtSupsi
Copy link

Fixed with this in code on 49 line, before:
if ($el.is('form, :input')) {
return $el.closest('form').removeAttr('data-confirm').submit();
}

After:
if ($el.is('form, :input')) {
modal.foundation('reveal', 'close');
return $el.closest('form').removeAttr('data-confirm').submit();
}

@jekuno
Copy link

jekuno commented Mar 9, 2017

I agree closing the modal when clicking confirm should become the deafult. (Nowadays most apps are SPA i.e. there's no full page reload after clicking confirm.)
However IMHO modal.foundation('reveal', 'close'); should be one line above (outside of the if clause).

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

No branches or pull requests

4 participants