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

Upgrade to react 16.0.0 #15469

Merged
merged 8 commits into from
Dec 11, 2017
Merged

Upgrade to react 16.0.0 #15469

merged 8 commits into from
Dec 11, 2017

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Dec 7, 2017

@cjcenizal and I paired up to upgraded to react 16.0.0 so kibana is running the same version as EUI

@kimjoar
Copy link
Contributor

kimjoar commented Dec 7, 2017

Should we go straight to 16.2.0?

@cjcenizal
Copy link
Contributor

@kjbekkelund I think it makes sense to complete our upgrade to 16.0.0 and then once that's done immediately bump Kibana, X-Pack, and EUI to 16.2.0. Does that sound good to you?

@kimjoar
Copy link
Contributor

kimjoar commented Dec 7, 2017

Works for me 👍

* Update to Enzyme 3.x. (#12)

* Add findTestSubject test helper. Fix KuiCodeEditor tests.

* Fix KuiContextMenu tests.

* Fix KuiConfirmModal tests.

* Fix tests for KuiPager.

* Update KuiConfirmModal tests to use findTestSubject helper.

* Publish test helpers from ui_framework. Fix DashboardCloneModal tests.
* Fix DashboardPanelContainer tests.

* Update KuiColorPicker tests.

* Fix ControlsTab tests.

* Rename vis to input_control_vis. Update and fix InputControlVis tests.

* Fix KuiListingTable tests.

* Fix KuiTextInput tests.

* Update DashboardGrid test snapshots.
@nreese
Copy link
Contributor Author

nreese commented Dec 7, 2017

jenkins, test this

* Remove old enzyme setup file.

* Add .test to YesNo and AddDeleteButtons test files.
Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!!

@@ -1,10 +1,11 @@
import React from 'react';
import sinon from 'sinon';
import { mount, shallow } from 'enzyme';
import { findTestSubject } from 'ui_framework/test';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

Copy link
Contributor

@kimjoar kimjoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Added my thoughts on findTestSubject, but if you don't agree I'm fine with merging as-is.

@@ -0,0 +1,10 @@
// Extract the DOM node which matches a specific test subject selector.
export const findTestSubject = (mountedComponent, testSubjectSelector, isDOMNode = true) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big +1 for extracting this function.

Hm, maybe add some docs or examples that explains isDOMNode? Not sure what if it from reading this function.

Copy link
Contributor

@kimjoar kimjoar Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking a bit more on this. Personally I also find boolean args difficult to read. I think I'd personally just move the getDomNode outside this function. I feel it's easier to read findTestSubject(myComponent, 'foo').getDOMNode(). We also just needed getDOMNode 3 times before, so it doesn't feel too bad to do that (but maybe we need it more now because of changes in enzyme?)

Copy link
Contributor

@cjcenizal cjcenizal Dec 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like your point about boolean args -- I have the same difficulty with them. I also like your suggestion. We'll try it out.

@nreese nreese merged commit cd9af07 into elastic:master Dec 11, 2017
nreese added a commit to nreese/kibana that referenced this pull request Dec 11, 2017
* get kibana to start with react 16.0.0

* ensure width and height for dashboard grid

* Update to Enzyme 3.x. (#12)

* Fix tests broken by Enzyme upgrade (#13)

* Update to Enzyme 3.x. (#12)

* Add findTestSubject test helper. Fix KuiCodeEditor tests.

* Fix KuiContextMenu tests.

* Fix KuiConfirmModal tests.

* Fix tests for KuiPager.

* Update KuiConfirmModal tests to use findTestSubject helper.

* Publish test helpers from ui_framework. Fix DashboardCloneModal tests.

* Fix more tests (#14)

* Fix DashboardPanelContainer tests.

* Update KuiColorPicker tests.

* Fix ControlsTab tests.

* Rename vis to input_control_vis. Update and fix InputControlVis tests.

* Fix KuiListingTable tests.

* Fix KuiTextInput tests.

* Update DashboardGrid test snapshots.

* Fix test file names (#15)

* Remove old enzyme setup file.

* Add .test to YesNo and AddDeleteButtons test files.

* needed window.cancelAnimationFrame to accompany requestAnimationFrame

* remove getDOMNode function call from findTestSubject
nreese added a commit that referenced this pull request Dec 11, 2017
* get kibana to start with react 16.0.0

* ensure width and height for dashboard grid

* Update to Enzyme 3.x. (#12)

* Fix tests broken by Enzyme upgrade (#13)

* Update to Enzyme 3.x. (#12)

* Add findTestSubject test helper. Fix KuiCodeEditor tests.

* Fix KuiContextMenu tests.

* Fix KuiConfirmModal tests.

* Fix tests for KuiPager.

* Update KuiConfirmModal tests to use findTestSubject helper.

* Publish test helpers from ui_framework. Fix DashboardCloneModal tests.

* Fix more tests (#14)

* Fix DashboardPanelContainer tests.

* Update KuiColorPicker tests.

* Fix ControlsTab tests.

* Rename vis to input_control_vis. Update and fix InputControlVis tests.

* Fix KuiListingTable tests.

* Fix KuiTextInput tests.

* Update DashboardGrid test snapshots.

* Fix test file names (#15)

* Remove old enzyme setup file.

* Add .test to YesNo and AddDeleteButtons test files.

* needed window.cancelAnimationFrame to accompany requestAnimationFrame

* remove getDOMNode function call from findTestSubject
@nreese nreese deleted the react16 branch February 13, 2018 19:31
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