Skip to content

DM-5276: Upgrade JS third party packages #163

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

Merged
merged 4 commits into from
Sep 6, 2016
Merged

Conversation

loitly
Copy link
Contributor

@loitly loitly commented Sep 2, 2016

Main purpose is to upgrade babel to v6, React to v15, and latest version of webpack.
Things you should be aware of in React 15: https://facebook.github.io/react/blog/2016/04/07/react-v15.html
As far as I can tell, they are all fixed in this pull request.

Don't forget to gradle purge first.
The pull request should be built with a newly created node_module.

Also:

  • upgrade most packages to latest versions as well
    • see package.json diff for more details.
  • fixed warnings related to React upgrade
  • remove some of the unused packages
    • history
    • icepick
    • babel-eslint
    • react-tools
    • query
    • less
    • less-loader
    • jscs
    • eslint-config-airbnb

Original ticket:
https://jira.lsstcorp.org/browse/DM-5276

 - goal: babel 6, React 15, and latest version of webpack
 - upgrade most packages to latest versions as well
 - remove some unused packages
 - fixed warnings related to React upgrade
@ejoliet
Copy link
Contributor

ejoliet commented Sep 4, 2016

I've built the branch locally and did some random tests. It seems ok in general but so far i can see one problem:
Some image tool dialog windows miss the 'x' close button when there is no a 'close' button explicitly, for example drawing layers, load regions ds9,etc . Will try to continue doing some tests.

@robyww
Copy link
Contributor

robyww commented Sep 5, 2016

the 'x' now shows up, pushed the fix.

@robyww
Copy link
Contributor

robyww commented Sep 5, 2016

The color picker api changed with upgrade. Fixed our code to work with it.

@ejoliet
Copy link
Contributor

ejoliet commented Sep 5, 2016

Thanks for fixing those. I have found another problem: The compass rose doesn't show up, error log in the console says:

Uncaught TypeError: (0 , _DirectionArrowDrawObj.makeDirectionArrowDrawObj) is not a function

@ejoliet
Copy link
Contributor

ejoliet commented Sep 5, 2016

And stretch/color changes seems to be applied but shows a fail message on the image viewer, and the console throw the following log:
TypeError: (0 , _DirectionArrowDrawObj.makeDirectionArrowDrawObj) is not a function
at makeCompass (http://localhost:8080/firefly/firefly-dev.js:112578:71)
at getDrawData (http://localhost:8080/firefly/firefly-dev.js:112543:24)
at DrawLayerFactory.getDrawData (http://localhost:8080/firefly/firefly-dev.js:88514:25)
at getDrawData (http://localhost:8080/firefly/firefly-dev.js:97428:39)
at http://localhost:8080/firefly/firefly-dev.js:97290:42
at Array.forEach (native)
at updateFromLayer (http://localhost:8080/firefly/firefly-dev.js:97289:20)
at http://localhost:8080/firefly/firefly-dev.js:97247:25
at http://localhost:8080/firefly/firefly-dev.js:55625:26
at Array.map (native)

@robyww
Copy link
Contributor

robyww commented Sep 5, 2016

fixed the direction arrow failure

@robyww
Copy link
Contributor

robyww commented Sep 5, 2016

OK, it looks good as far as I know. I sure we will find a couple of more issues.

@tgoldina needs to check the highcharts update.

@tgoldina
Copy link
Contributor

tgoldina commented Sep 6, 2016

It looks like Loi got the latest version of react-highcharts, and it appears to work as expected, no errors - I've checked both regular and binned plots.

@robyww
Copy link
Contributor

robyww commented Sep 6, 2016

Ok, I think it is good to merge. We fix other issues as we find them.

Review Complete.

@loitly loitly merged commit 1c95589 into dev Sep 6, 2016
@loitly loitly deleted the DM-5276_update_packages branch September 6, 2016 17:54
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.

4 participants