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

if the tabs directive is used in index.html with a custom template, the route may be lost on page reload. (was route is lost on reload when using custom tabs template) #15

Closed
johngwilkinson opened this issue Feb 24, 2015 · 9 comments

Comments

@johngwilkinson
Copy link

When I use the following custom template on a page with two tabs, adding an id field to each tab in the tab data, reloading the page always returns the user to the default tab. This behaviour doesn't happen if I make the same change to the default template right in ui-router-tabs.js, so it appears to be a custom template issue (the template is just the default with an id added):

@rpocklin
Copy link
Owner

I'll take a look. A plunkr or jsfiddle would help to demonstrate if you could make one.

@johngwilkinson
Copy link
Author

Thanks!

http://plnkr.co/edit/m9xhXWD6R73G9x4mghr5?p=preview

Not that you can repro it directly in the plunk, since it's reloading the page or copying the url for the second tab that seems to have the problem.

@johngwilkinson
Copy link
Author

Actually, I get the same behaviour even when the "custom" template is identical to the default one--I've updated the plunkr accordingly.

@rpocklin
Copy link
Owner

Strange, I fork it and it works for me. Could be an XHR security issue with Plunkr.
http://plnkr.co/edit/w33YK7RL88BcedeZKJjO?p=preview

I changed my code in the /example/ folder to your example and it works locally for me too.

Which browser are you using?

Have a look at the network tab in the chrome debugger - is twos.html trying to get loaded and failing?

@johngwilkinson
Copy link
Author

I'll try to look into further when I can, but meanwhile I'll try to clarify what I'm seeing, since I don't think I described it well above. :)

I've tried chrome, ff, and ie 11, and the behaviour is the same on all three.

The only thing that seems required to reproduce is that template-url be specified on the tabs. I'm viewing my page live, served from a local nginx.

The problem is that visiting #/twos (e.g. by typing it in the browser, copy-pasting the url, or refreshing the page while on the twos tab) displays the ones tab instead. (I don't know how to repro in plunkr, since there's no hash fragment to edit there.)

Oddly, editing the url to change it from #/ones to #/twos works fine. It's only reloaded and new tabs that have the issue.

Everything else, including clicking from tab to tab and seeing the partials' content, works as expected.

@johngwilkinson
Copy link
Author

So, I believe this is a race condition. It seems that the initial call to $scope.update_tabs always happens (for me) before $state has any value, so no default tab is set. When the default template is used, a $stateChangeSuccess event fires just as the default tab is being set, which causes the url to take precedence. When a custom template is used, the state gets updated to the default tab first.

Suggested fix: remove the call to update_tabs from the directive body (leaving just the event handler), and also remove the default-tab-selecting code. Users of the code then have to use $urlRouterProvider.otherwise(); when configuring the $stateProvider to get the desired default tab.

It works for me with or without a custom template with these changes. Gist here: https://gist.github.com/johngwilkinson/dd8b1d08e20c7374a89b

@rpocklin
Copy link
Owner

You say $state has no value, do you mean it is undefined or that there's no current route in the application? In your example, you are using this directive in the default route in index.html which isn't typical - you would normally navigate to a default route which means there is always a current route set by the time you load this directive.

I'm concious no one has raised this problem before, i've created a branch called tab-in-index-page and put code code from your plunkr into the example. Since the plunkr messes with the routes it's not ideal for debugging.

Can you checkout that branch and double-check you still see this race condition?

@johngwilkinson
Copy link
Author

Yes, that repros for me.

I mean that there's no current route. And yes, it would make sense that having the tabs in index.html would be the only way to see the issue. So the issue, more clearly, would be that if the tabs directive is used in index.html with a custom template, the route may be lost on page reload.

Your example (master branch) uses index.html just as a loading point for a single ui-view, which has the tabs in it. But the examples in the angular-ui README have "tabs" (anchors with ui-sref) and a view in index.html. I can see the advantage of doing it your way, but I'm genuinely not clear on whether it's best not to support tabs-in-index. If the tabs are to be a universal part of an app's ui, would you still not recommend having them in index.html?

The above pull request suggests a solution, btw. It does require the app to declare a default route, though (.otherwise), or loading the index page may fail to load the default tab.

@johngwilkinson johngwilkinson changed the title route is lost on reload when using custom tabs template if the tabs directive is used in index.html with a custom template, the route may be lost on page reload. (was route is lost on reload when using custom tabs template) Feb 27, 2015
@rpocklin
Copy link
Owner

rpocklin commented Mar 4, 2015

For what it's worth, it's generally better to avoid embedding angular views in your bootstrap index.html, you need to use ng-bind to get around some quirks of the HTML parsing.

I'll review #16 and merge it if we can work out a way to fix it for this case without regressing the tests.

I'm closing this issue since the heading is misleading, it's nothing to do with custom templates.

@rpocklin rpocklin closed this as completed Mar 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants