Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

$dialog, modal and location change #335

Closed
pkozlowski-opensource opened this issue Apr 18, 2013 · 29 comments
Closed

$dialog, modal and location change #335

pkozlowski-opensource opened this issue Apr 18, 2013 · 29 comments

Comments

@pkozlowski-opensource
Copy link
Member

Created based on angular-ui/angular-ui-OLDREPO#538 from the angular-ui repo:

I have run into an issue with Modals and Dialogs when a browsers back button is pressed while a modal and glassis visible. (in a multi page app).

The application changes the view and controller while the modal stays activated and visible. As the controller has been swapped out from underneath the modal, strange things start happening.

How do we get around this issue? Is there a way to make sure the modal / dialog is dismissed when the back button is dismissed or the route is changed.

It is an interesting problem, we should consider closing modals on location change. The thing that the modal stays open while controllers can be sapped underneath sounds like a buggy situation.

@mxparajuli
Copy link
Contributor

I am facing this issue, here is a plunker for it http://plnkr.co/MhYssd1GiOx1d1YwOyf0. I am currently working around it by exposing the dialog in rootScope and I checking for it in the controllers/binding to route change. It is very hacky, I hope there is a better way.

@pkozlowski-opensource
Copy link
Member Author

@mxparajuli Wow, awesome plunk :-) I wish all the bug reports were like this!

We should definitively fix this in the $dialog service, probably by listening to $locationChangeSuccess.

@mxparajuli Would you be up to sending a pull request for this? It should be pretty straightforward.

@mxparajuli
Copy link
Contributor

@pkozlowski-opensource Sure thing! I'll send a pull request soon. My first pull request :)

@sudhakar
Copy link

Right now $dialog creates a new scope from $rootScope. Cant we have it to have a childscope from the current view. So then theoretically when the parent view is destroyed, $dialog will take care of itself.

I know it has been discussed before, but just thinking out loud 🔢

@sudhakar
Copy link

Or may be we can provide an additional option like destroyWith to the $dialog service and allow the user to pass the current scope. Then in the $dialog service, we can watch for $destroy event of that local scope and then destroy the $dialog scope.

But this will necessitate user to explicitly specify it..

@mxparajuli
Copy link
Contributor

@sudhakar I like the idea of $dialog having a child scope of the current view. How do you suggest $dialog be passed the view's childscope? I am new to angular but I am learning and I want to help :)

@sudhakar
Copy link

$dialog service is usually invoked inside a view controller. By design, $dialog/model itself requires another controller, which needs to be explicitly defined by the user. This controller gets a new scope using $rootScope.$new.

This is byDesign. Checkout more discussion on this @ #259

After giving it a thought, I think we might need to combine both destroyWith with $routeChangeSuccess. So that if user doesnot explicitly specify destroyWith, $routeChangeSuccess event can be used to destroy the dialog..

@pkozlowski-opensource is the right person to comment on this...

@mxparajuli
Copy link
Contributor

I've been occupied with other stuff but I have submitted a pull request that watches location change. I like @sudhakar 's suggestions but I will read up a bit more before trying anything.

@sudhakar thanks for the link to the discussion.

@Charuru
Copy link
Contributor

Charuru commented Jul 7, 2013

How do I get the dialog to not close on location change? I have an IM window in a dialog and would like it to stay open when going from page to page.

@blacroix
Copy link

In last build (4.0) this line:

this.$scope.$on('$locationChangeSuccess', this.handleLocationChange);

in function:

Dialog.prototype._bindEvents

is missing and @pkozlowski-opensource fix does not work anymore.

Why it desapears ?

Regards

@pkozlowski-opensource
Copy link
Member Author

@blacroix this fix was causing multitude of other issues so I had to revert it. Currently I'm working on the rewrite of the $dialog (via #441) and going to address this issue in the rewrite.

@blacroix
Copy link

OK, thank you @pkozlowski-opensource, is there a workaround to be able to close the modal when the user press back button or change the route ?

For now I add the missing line in my local file.

@pkozlowski-opensource
Copy link
Member Author

@blacroix I think that what you did (monkey-patching your local version) is the best approach right now.

@Rodeoclash
Copy link

👍 for the fix on the new version of $modal :)

@timfjord
Copy link

timfjord commented Oct 9, 2013

Here is a workaround that i use to close modal(i am using ui-router)

App.run([
  '$rootScope', '$modalStack', 
  function($rootScope, $modalStack) {
    $rootScope.$on('$stateChangeStart', function() {
      var top = $modalStack.getTop();
      if (top) {
        $modalStack.dismiss(top.key);
      }
    });
  }
]);

@bernhardh
Copy link

Is there already a working solution for this? Issue is still present.

@mvhecke
Copy link
Contributor

mvhecke commented Jan 14, 2014

@bernhardh I think #1552 should have solved this issue, although it seems the function isn't exposed yet to the $modal.

@scanferla
Copy link

Hi, any updates on this issue?

@joeylgutierrez
Copy link

i'm still seeing this issue but @Timsly workaround works great for me!

@jglinsek
Copy link

jglinsek commented May 9, 2014

@Timsly workaround ALMOST worked for me, except in my case, my dialog's are really a state of their own even though they don't have a state in ui-router (which we use as well). so, i wanted to prevent the back navigation in ui-router as well - but in doing so, ui-router will eat the actual toState that you wanted to go to within your browser history (it actually does go back in the browser history and then replaces the current history state) and then the history gets mangled.

long story short, you can simply replace $stateChangeStart with $locationChangeStart and it short circuits ui-router and prevents your history from being touched. so, this is working for me:

    .run([
        '$rootScope', '$modalStack',
        function ($rootScope, $modalStack) {
            $rootScope.$on('$locationChangeStart', function (event) {
                var top = $modalStack.getTop();
                if (top) {
                    $modalStack.dismiss(top.key);
                    event.preventDefault();
                }
            });
        }
    ])

@joelsaupe
Copy link

@jglinsek I found a problem with this solution. When trying to change $location within the modal, the modal closed but the $location does not change. The only way to change $location was to manually close the modal first and then update $location as follows:

$modalInstance.dismiss('cancel');
$location.path(url);

@lmiceli
Copy link

lmiceli commented May 26, 2014

@jglinsek thanks a lot for sharing your find was exactly my problem and your solution what I needed.

@awerlang
Copy link

awerlang commented Jul 1, 2015

If using ui-router and the modal itself triggers a state change, the code could be like this:

$state.go('state').then(() => $modalInstance.close())

If you use $state.go(), then the promise is fulfilled before a $locationChangeStart event is raised. By the time the event is handled, there would be no more tracked modals.

Rest of code by @jglinsek above.

@swapnilchincholkar
Copy link

@jglinsek, thanks a ton !! saved my day.

@filipesilva
Copy link

@Timsly's solution did it for me as well.

I agree with @pkozlowski-opensource in that something similar should be added to ui-bootstrap because of the implications of closing a modal in a state other than the one that opened it.

@joshualinog
Copy link

If the back button is clicked on initial page load, the default behavior happens too fast for the function to be considered so I did the following:

 $rootScope.$on('$locationChangeStart', function (event) {
        event.preventDefault();
        var top = $modalStack.getTop();
        if (top) {
             $modalStack.dismiss(top.key);

        }else{
            event.defaultPrevented = false;
        }
    });

@henck
Copy link

henck commented Feb 26, 2016

The issue remains. My workaround for a specific dialog was

$scope.$on('$routeChangeSuccess', function () {
  $uibModalInstance.dismiss('cancel');
});

I couldn't see where to access $modalStack.

@icfantv
Copy link
Contributor

icfantv commented Feb 26, 2016

This is still an issue, I'm going to reopen this.

@icfantv icfantv reopened this Feb 26, 2016
@wesleycho
Copy link
Contributor

One can access $uibModalStack through injection.

$scope.$on('$routeChangeSuccess', () => $uibModalStack.dismiss($uibModalInstance, 'cancel'));

or

$scope.$on('$routeChangeSuccess', () => $uibModalStack.dismissAll());

Closing as we decided we don't want this level of opinionation in the library itself, and there are enough mechanisms around this built into the library itself to allow the user to control to what degree this happens.

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

No branches or pull requests