search

rluders / oc-theme-angular

The skeleton theme for October's Angular applications
http://octobercms.com/theme/rluders-angular2
MIT License
11 stars 2 forks source link

Update to Angular 8 #7

Closed MilosStanic closed 5 years ago

MilosStanic commented 5 years ago

Surprise, surprise! :) I know I've been promising, since Angular 7 to help you move to latest Angular. And now we have Angular 8. So here's an update to Angular 8!

There was a LOT of changes. Plus there is a production mode added, which required a LOT more changes.

Now, it works, but I haven't been able to test the login, because it says it requires a login, but an email is sent in credentials. I suppose it is an issue with your JWTAuth plugin, but I'm too tired to try fixing it.

Plese give this a spin and tell me what you get.

If you want to just test before merging, go to my profile and clone the forked repo.

Oh, one thing, I wasn't able to replicate the folder name rluders-angular2 so, now the theme folder name is oc-theme-angular.

Looking forward for your feedback.

Cheers!

rluders commented 5 years ago

Wow! That is a lot of changes @MilosStanic.

Thank you. I'll test it this weekend, and give you feedback ASAP.

rluders commented 5 years ago

OK. Did some tests, and so far is everything great. I already fixed the integration with the JWT Auth, so... don't worry. I noticed some strange things, but nothing really important right now. It's just a few improvements that we can work on later.

I'll list them as new issues, so it will be better to track and work on it.

I also reorganized the documentation, to make sure that it was as clean as possible. I'll do a few more tests during this week and them accept the merge request.

rluders commented 5 years ago

@MilosStanic Can you enable me to contribute with this PR?

https://stackoverflow.com/questions/20928727/adding-commits-to-another-persons-pull-request-on-github

MilosStanic commented 5 years ago

Hi, I can not find a switch like that on my forked version of your repo. But looking at this pull request I see that the edits are allowed. See screenshot attached Screenshot_183

MilosStanic commented 5 years ago

one more screenshot with a better explanation Screenshot_184

MilosStanic commented 5 years ago

Anyway, back to the topic. Yes, there were 3 warnings when compiling for production, about some things becoming obsolete soon, but that can be fixed. You said you fixed JWT and that's great news! I was afraid because I wasn't able to test the API after logging in. I wasn't sure the new version of auth0/angular-jwt was properly attaching Bearer tokens to request with the new interceptor.

I admit the README was amended in a hurry when I was already very tired after a full day of migrating to Angular 8. So yes, it needs heavy editing. And I'm sure there will be lots of space for improvement.

Thank you again for considering to accept the pull request

rluders commented 5 years ago

No problem, @MilosStanic.

I did some changes already, I'll review it all and try to push it till the weekend. But I'll not promise anything, this week is just really busy for me.

As I told you, I already did:

I'll try to push it ASAP. But so far, thank you very much for all you work on this.

rluders commented 5 years ago

@MilosStanic I think that everything is OK. So, I'll accept the pull request and merge it into develop, so I can finish up the other tasks related to 2.0.0 milestone, then release this update.

Thank you for your help.