umts / pvta-multiplatform

Transit realtime app for mobile devices
Other
6 stars 5 forks source link

Ionic 3 #406

Closed akaplo closed 7 years ago

akaplo commented 7 years ago

EDIT 8/3/17 Either I was misunderstanding from the beginning or they fixed this in some updates to 3.x, but most of what I believed was barring us from upgrading was simply untrue. See #413.

ORIGINAL Ionic 3 has been stable for almost 2 months. There's a single major roadblock for us to update to it: routing. We use explicit URLs (/favorites, /settings, etc) like a website, which Ionic has decided to make significantly harder.

In order to use this kind of routing, we must use their IonicPage and "Lazy Loading" features, which require a separate module for each page (instead of just an app.module.ts, we'd need to add a page_name.module.ts for each page in the app), which in my opinion is a ridiculous design choice that creates a lot of unnecessary code replication.

See part 1 and part 2 of a 2-part official post on the topic.

So, what I'm interested in determining is: should we just stick with Ionic 2.x? Bite the bullet, take the duplication hit and keep moving with Ionic? Or, change how we think about our routing?

mboneil10 commented 7 years ago

Whoa. I'm completely split on this. The change would create a lot of unnecessary duplicate code. Seeing as how frustrated I get with the four main pages all containing the same navmenu, I don't know how we could deal with fixes if everything is completely spread out. In terms of keeping our code clear, I vote no on upgrading. But in general, it always seems like a good idea to upgrade our apps.

mboneil10 commented 7 years ago

Since you said Ionic 3 would help us out with this bug, I'm currently torn. Will label pros and cons to updating. We'll figure something out.

akaplo commented 7 years ago

Yeah, same. Input from others like @Anbranin, @werebus, and @dfaulken might help!

mboneil10 commented 7 years ago

plz

mboneil10 commented 7 years ago

Let's just try it out.

akaplo commented 7 years ago

I think I did most of this work at one point - if somebody decides to hack at this, please wait till I get home tonight and see if I have a local branch sitting on my computer.

mboneil10 commented 7 years ago

Ok, I won't. I'll be working on the delete favorite routes and stops thing. I don't see anyone picking it up today. Thanks Aaron! 👍

mboneil10 commented 7 years ago

@Anbranin said yes, too. I'll do this later. If you still didn't find it.

akaplo commented 7 years ago

Yeah. Sadly, that branch must've gotten deleted out of either carelessness or frustration (both of which would be my fault).

mboneil10 commented 7 years ago

Ok. I try today.

akaplo commented 7 years ago

I'll drop assorted oddities from the upgrade in this comment

  1. image
mboneil10 commented 7 years ago

We're at a point where we get something similar to this issue, and the console.error reads: PlanTripComponent ionViewWillEnter error: google is not defined. Problem with loading.

akaplo commented 7 years ago

PR is good to go. Issue described by @mboneil10 was solved by using anonymous callbacks properly and is also present on master.