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

Update dependency: angular-ui-router to 0.2.10 #941

Closed
wants to merge 2 commits into from
Closed

Update dependency: angular-ui-router to 0.2.10 #941

wants to merge 2 commits into from

Conversation

oori
Copy link
Contributor

@oori oori commented Mar 28, 2014

ui-router 0.2.7 is outdated and contains basic bugs such as angular-ui/ui-router#692 preventing the use of "ui-sref-active" on distributed version. UI-view animations fix and much more..

I use ionic and enhance it a lot with ui-router directly (I dislike static href, and love ui-sref... in addition to using my own ui-views, etc..)

I've tested this on my setup with 0.2.10 and it is flawless.
Anything preventing us from moving forward to 0.2.10 ?

@keithdmoore
Copy link
Contributor

@oori I would be interested to know if you use nested states/views (like 3 or 4 levels deep). I am experiences some issues where calling $state.go('^') was returning to the previous state. Althought, the content was correct, the header title did not get updated. I am using a "side menu" style setup on ionic 0.9.27 currently. Do you happen to have any samples I could look at ?

Thanks!

@adamdbradley
Copy link
Contributor

When we previously upgraded ui-router we came across a few new bugs, basically with transcluded content. We'll have to setup more tests then we'll include it.

@oori
Copy link
Contributor Author

oori commented Mar 28, 2014

@adamdbradley Sure. I know ui-router got it's "fame" on 0.2.7, but lately some hard-core bugs are being actively fixed. I recommend..
Anyway - I currently use 0.2.10, I'll update this thread when I bump on useful info.

@keithdmoore At the moment I mainly use ui-router for two things:
a. nested tabbed interface which is "out" of the ionic navigation, so although it's stateful and the url changes, it doesn't affect the main ionic back button.
b. replace all href="" hard links with relative ui-router syntax.
I am not experiencing the issue you describe. the code I'm working on is closed-source (for a client), so I can't expose, but perhaps you can do a quick plunkr with your structure? on which I could alter and we'll see the differences.

@keithdmoore
Copy link
Contributor

@oori Thanks for the offer. I managed to figure out my problem. It was between my keyboard and chair. I ended up removing my nested views and opted to share the data thru services. I also needed to use the "@" to find the proper view to place the template in. Like "menuContent@menu", for example. Got the ionic back button and my own "back/cancel" buttons (that use stage to go back) all working.

I plan on upgrading the ui-router to 0.2.10 soon. I will post back here with my findings.

@keithdmoore
Copy link
Contributor

+1 for the upgrade: After doing some testing with ui-router 0.2.10, I see that the ui-sref-opts="{ reload: true }" is being honored now. Not seeing any issues.

@ajoslin
Copy link
Contributor

ajoslin commented Apr 3, 2014

Hi @oori, thanks. We're pushing out a minor fixes release in the next couple days, then we'll merge this and test it out over the next week.

@keithdmoore
Copy link
Contributor

When I tested angular-ui-router 0.2.10 previously, I referenced the ionic and angular libraries individually. I was wondering how to build the ionic-bundle.js with this new dependency. After modifying the bower.json file with the new version of angular-ui-router, I did a bower install and then a gulp build --release. The dist folder did not have the updated router version. Please help, I am new to building js projects.

@oori
Copy link
Contributor Author

oori commented Apr 16, 2014

Clean (purge) bower cache first. Otherwise, bower doesnt really read your changes..

@ajoslin
Copy link
Contributor

ajoslin commented Apr 16, 2014

The build system reads it from config/js/lib/angular-ui-router/

On Wed, Apr 16, 2014 at 2:25 PM, Oori [email protected] wrote:

Clean (purge) bower cache first. Otherwise, bower doesnt really read your
changes..


Reply to this email directly or view it on GitHubhttps://github.com//pull/941#issuecomment-40647662
.

@keithdmoore
Copy link
Contributor

Thanks guys. Moved the updated angular-ui-router files from bower_components to the config/js/lib/angular-ui-router and was able to get the new version in ionic-bundle.js.

@mlynch
Copy link
Contributor

mlynch commented Apr 29, 2014

I don't think this is going to happen. To be honest, we are looking into building a new nav system from scratch which will enable us to do a lot more stuff we want to do. Closing for now.

@mlynch mlynch closed this Apr 29, 2014
@oori
Copy link
Contributor Author

oori commented Apr 30, 2014

@ajoslin & @mlynch that's a shame.. this update resolves real-world issues, and we haven't run into any new ionic issues because of this. (have you run into any issues we should know of before going to production?)

sure, we can do this update manually (as we do), but until you move to a new routing module, your users will continue to hit 0.2.7 issues (for example: http://forum.ionicframework.com/t/errors-during-minifying/2330)

thanks.

@keithdmoore
Copy link
Contributor

That is unfortunate. I have been using 0.2.10 for about a month now too. Have not run into any issues. It fixed a problem with the ui-sref-opts being applied. So I need it. I guess I will just keep using a custom build.

@mlynch
Copy link
Contributor

mlynch commented Apr 30, 2014

Sorry I was under the impression the new version broke it.

I'll reopen this

Sent from my iPhone

On Apr 29, 2014, at 7:30 PM, "Keith D. Moore" [email protected] wrote:

That is unfortunate. I have been using 0.2.10 for about a month now too. Have not run into any issues. It fixed a problem with the ui-sref-opts being applied. So I need it. I guess I will just keep using a custom build.


Reply to this email directly or view it on GitHub.

@oori
Copy link
Contributor Author

oori commented May 3, 2014

@mlynch said: "I'll reopen this"

cheers.

@xaka
Copy link

xaka commented May 3, 2014

So what's the actual plan, friends?

@somethingnew2-0
Copy link

We will most likely update the ui-router if there aren't outstanding issues, but we are actively looking into implementing our own that is more friendly for hybrid mobile.

@fredgalvao
Copy link

@somethingnew2-0 Can you share more about what's making you consider the option to opt out of ui-router? It's an amazing project for what I know, and I'd really like to know which aspects of it aren't hybrid mobile friendly enough.

@somethingnew2-0
Copy link

@mlynch or @adamdbradley?

@ajoslin
Copy link
Contributor

ajoslin commented May 6, 2014

We'll upgrade right after we release beta.4 tomorrow.
On May 6, 2014 1:36 PM, "Frederico Galvão" [email protected] wrote:

@somethingnew2-0 https://github.com/somethingnew2-0 Can you share more
about what's making you consider the option to opt out of ui-router? It's
an amazing project for what I know, and I'd really like to know which
aspects of it aren't hybrid mobile friendly enough.


Reply to this email directly or view it on GitHubhttps://github.com//pull/941#issuecomment-42348536
.

@fredgalvao
Copy link

Hm, I take it ui-router still has a place in Ionic then. I'm glad so far.
Thanks @ajoslin , really looking forward to tomorrow's beta!

@ajoslin
Copy link
Contributor

ajoslin commented May 6, 2014

ui-router has a place in Ionic for now. We are evaluating options for the future.

@ajoslin ajoslin closed this in b9353e7 May 9, 2014
@ajoslin
Copy link
Contributor

ajoslin commented May 9, 2014

done. 0.2.10 is up on the nightly builds.

@somethingnew2-0 somethingnew2-0 mentioned this pull request May 15, 2014
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

Successfully merging this pull request may close these issues.

8 participants