angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.79k stars 27.48k forks source link

Do not force a reload when navigating out of the app base #12719

Open jirihelmich opened 9 years ago

jirihelmich commented 9 years ago

This fix https://github.com/angular/angular.js/commit/9e492c358c19549696577c86c2c61b93f50ab356 in fact introduced a breaking change.

I have an application, where on top of angular modals are opened with usage of browser history API. Now, the page gets reloaded because of the bugfix, which does more than necessary.

There was a suitable workaround that I enclosed in the original issue, which did not break anything. https://github.com/angular/angular.js/issues/4776

It would be great if it was at least possible to disable the reload in configuration. Btw. magically reloading the page without logging any info anywhere for the developer is a bit unfair. He just finds his page being reloaded with no good reason.

Moreover, workarounds we already had does not work anymore, since the new code does not use $location.$$parse.

petebacondarwin commented 8 years ago

@jirihelmich - can you provide a runnable example of the problem? I believe that patching the $location services caused other use cases to fail. Perhaps you could show how your solution would work in a PR?

jirihelmich commented 8 years ago

Well, in fact, I don't have a running example after more than three months. In fact, I don't have the app either as our company decided to migrate it to React-ish stack.

The problem/use case was:

URL changed, angular detected that it's base URL is not in location anymore -> reload.

petebacondarwin commented 8 years ago

@jirihelmich - thanks for the extra information. It seems strange to push an entirely new URL to the history for a modal pop-up. What if the user did a manual refresh themselves? This would take you to completely different page, outside the original angular app, right?

I would expect that such a modal URL should be a sub-URL of the angular app at least. Otherwise I would consider putting the popup in an iframe or something to give it its own context.

What were the reasons for doing it the way you describe?

jirihelmich commented 8 years ago

So, the bigger picture. We are a PMS solution and in order to support user needs, we have a custom window manager. That means that we have stackable windows in our app. In the beginning, you get a "normal" page, when you click on something which goes more into detail, we open another layer on top of the previous one, you can go back.

We have a window manager that tracks URL changes, takes care of AJAX requests and if user presses refresh, it reconstructs the stack of windows from the window history.

image

On the left, you could see the stack of windows to make it more real for you.

Those are rendered mostly on server, downloaded via HTTP using XHR, links and forms are bound by jQuery to behave as expected. If user would press refresh now, the page would reload, the topmost window would be rendered on the server in the layout, then the manager requests the underlying windows and swaps the content to make it as it was.

But we had (and still have) some Angular apps that were injected into this. Meaning user clicked, the content of the window rendered on the server was a container for an angular app. We bootstrapped it manually, everything was running fine. The problem came, when we opened another layer above the angular app, which was some partial rendered fully on the server again. With all the previous angular releases, we were fine, but this broke it for us.

I know that our use case is out of the scope of pure Angular world and is rather complicated. But still, it`s a real world scenario broken by the commit. And I would not create an issue for it, if it wasn't because of the fact that someone decided to refresh a page if some condition fails. I don't find that fix very convenient, since refreshing a page is a brutal thing to do when some internal check fails.

The problem is, that we were trying to combine Angular with non-Angular world, which simply does not work very well. So far, we have overcome everything we had an issue with, but releasing refresh of a page as a part of bugfix revision was a very unpleasant experience.

watchwithmike commented 8 years ago

We ran into this issue recently and also agree with @jirihelmich that the introduction of the "feature" as part of a bugfix was unpleasant. I'm wondering why this is even needed as part of the core, it seems as if this would be more appropriately placed in angular-route.

We have created a somewhat contrived sample showing the problem, highlighting the differences between 1.4.3 and 1.4.4 (and onward). The basic premise is that an angular app exists inside a page with non-angular paradigms for updating content on the page. The user clicks thumbnails causing an area of the page to show new content and updating the URL with a slug appropriate for bookmarking. This sample is in no way comprehensive but I hope it illustrates the problem we're facing.

Without having this rolled back in angular.js, we'll likely have to decorate it out of the service which is undersireable from a maintenance perspective.

vladimirtsyupko commented 8 years ago

Is there a temporary workaround for this problem?