New WebUI: Update angular and angular-route and add workaround to be compatible#1813
Conversation
…test upstream releases)
|
Oh, forgot: This PR also updates jQuery to the latest upstream release. I did test the new Web UI after the changes:
All functions seem to work as before |
|
Oops, sorry |
FloEdelmann
left a comment
There was a problem hiding this comment.
The code changes look fine. I haven't checked out the code locally though.
|
Looks like the Travis CI job doesn't even start. Should we merge the PR manually without that check? |
daveol
left a comment
There was a problem hiding this comment.
Code changes seem fine, Haven't tested it in the real world though.
|
Travis CI is giving the following error on the site (when i logged in at least):
|
peternewman
left a comment
There was a problem hiding this comment.
As with others LGTM but I've not tried it.
Just the one possible comment improvement.
| // AngularJS 1.6 now uses an '!' as the default hashPrefix, breaking | ||
| // our routes. Following lines revert the default to the previous empty | ||
| // string. |
There was a problem hiding this comment.
I think we could also add a note saying given this webpage is only used internally, there is no benefit in the web crawler changes.
We could possibly add a TODO to update our URLs to remove this change, but maybe that's not necessary if we're rewriting to a new framework in future?
There was a problem hiding this comment.
I agree that this might be useful but since AngularJS (= 1.x) is a dead end, we won't bother for now
You could try going here @kripton and logging into Travis: Maybe this is just a guerrilla attempt to force more people to GitHub Actions? Maybe we should work on this for Linux too:
Quite possibly, the tests won't really check it aside from the JS linting stuff which I guess you've run? Or do a dummy commit and see if it does anything the second time around. |
|
@peternewman I'm happy to work on a Linux build for GitHub Actions to compliment #1809 and #1812 to fully replace Travis. |
I did. It gave me some "plan exceeded" limit, mainly regarding the "number of users on the plan". However, I am unsure if it meant my personal GitHub account or the OpenLightingProject organization. With all the positive review feedbacks here and since Travis won't catch errors in JavaScript anyways, I'll just merge this one. |
This PR updates angular and angular-route to the latest upstream releases (v1.8.3). However, without changes to our new Web UI, this causes it to break as described in #1782. To fix this, a workaround has been added to restore angular's previous behaviour. See https://docs.angularjs.org/guide/migration#commit-aa077e8
As a bonus, the list of dependencies in
javascript/new-src/package.jsonis not sorted alphabetically as is done automatically when adding or updating packages listed therein.This PR also updates the "built" app.min.js and related files.