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

[@kbn/ui-framework] move ui-framework to a package #17085

Merged
merged 4 commits into from
Mar 13, 2018

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Mar 10, 2018

Before opening x-pack we either need to remove our usage of react-router v3, which is used by the ui_framework doc site. Rather than make the massive amount of changes necessary to upgrade to react-router v4 this moves the ui_framework into a package so that it is isolated from core Kibana.

All yarn scripts have been updated to continue working as designed, the only real difference is that ui_framework components are now imported from @kbn/ui-framework/components rather than ui_framework/components.

@spalger spalger added WIP Work in progress Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Mar 10, 2018
@spalger spalger force-pushed the upgrade/ui-framework-react-router branch from e9e0521 to ae662bc Compare March 10, 2018 02:06
@elastic elastic deleted a comment from elasticmachine Mar 10, 2018
@spalger spalger force-pushed the upgrade/ui-framework-react-router branch from ae662bc to 21d57ad Compare March 10, 2018 02:23
@spalger spalger force-pushed the upgrade/ui-framework-react-router branch from 21d57ad to e839543 Compare March 10, 2018 02:45
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger requested review from kimjoar and cjcenizal March 13, 2018 00:08
@spalger spalger added review v7.0.0 v6.3.0 and removed WIP Work in progress labels Mar 13, 2018
@spalger
Copy link
Contributor Author

spalger commented Mar 13, 2018

@cjcenizal can you please help me verify that the build tasks and such still work as expected?

@spalger
Copy link
Contributor Author

spalger commented Mar 13, 2018

I'm also fairly confident that all the jest tests are still running, but if you could verify that would be appreciated.

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Great work Spencer! Thanks so much for doing this. The tests and tasks work fine but I found one thing that's broken. It might seem odd, but the doc_site/build directory needs to be checked in with an index.html file inside it for the docs to be served: https://github.com/elastic/kibana/blob/master/ui_framework/doc_site/build/index.html

@spalger
Copy link
Contributor Author

spalger commented Mar 13, 2018

Got it, fixed and updated the .gitignore file to not ignore the index.html file (apparently you have to use the * notation for the negation to work properly).

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

LGTM!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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 :shipit: I added some comments, but feel free to skip.

'import/no-extraneous-dependencies': [
'error',
{
devDependencies: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this setup for ui-framework but not for the rest of Kibana? Maybe it's been discussed somewhere (I've been sick the last week, so my brain is still a bit foggy).

Or is it just a "nice addon" to help make it clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only added it to the ui-framework to help me verify that I was pulling in the right dependencies and making the right things dev/normal deps. I think we could absolutely use this in other places, but I don't think it needs to go into this PR.

"xml2js": "0.4.19",
"xmlbuilder": "9.0.4",
"yeoman-generator": "1.1.1",
"yo": "2.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

I ❤️ THIS CLEANUP SO MUCH!

@@ -44,15 +40,14 @@ export default {
],
modulePathIgnorePatterns: [
'__fixtures__/',
'<rootDir>/packages/kbn-ui-framework/target/',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should just do /target/ here? I felt I had already included that in some PR, but maybe it's one of my active ones 🙈

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger merged commit ef3339b into elastic:master Mar 13, 2018
spalger added a commit to spalger/kibana that referenced this pull request Mar 13, 2018
* [@kbn/ui-framework] move ui-framework to a package

* [@kbn/ui-framework] restore doc_site index.html and fix build task names

* [jest] always ignore modules in target dirs
spalger added a commit that referenced this pull request Mar 13, 2018
* [@kbn/ui-framework] move ui-framework to a package

* [@kbn/ui-framework] restore doc_site index.html and fix build task names

* [jest] always ignore modules in target dirs
@spalger
Copy link
Contributor Author

spalger commented Mar 13, 2018

6.x/6.3: 2013bb9

@spalger spalger deleted the upgrade/ui-framework-react-router branch March 13, 2018 19:54
chrisronline added a commit to chrisronline/kibana that referenced this pull request Mar 27, 2018
chrisronline added a commit to chrisronline/kibana that referenced this pull request Mar 27, 2018
…lastic#17085)""

This reverts commit ce9ce14e1060c426090b55a5367de3ff4329e681.
chrisronline added a commit that referenced this pull request May 17, 2018
* Not working proto code

* More proto code

* Work in progress

* Just go back to non interactive searching, much easier

* This should be on the server

* Revert "[@kbn/ui-framework] move ui-framework to a package (#17085)"

This reverts commit ef3339b.

* Revert "Revert "[@kbn/ui-framework] move ui-framework to a package (#17085)""

This reverts commit ce9ce14e1060c426090b55a5367de3ff4329e681.

* Use BasicTable properly

* Table improvements

* Small tweaks to the table

* Improvements

* Flyout mostly working

* Remove in memory table

* Getting close

* Tweaks

* Revamping server code, still need to support editing

* Progress

* Fix export

* Updates and passing functional tests

* Better links in relationships flyout

* Add skip import option

* Fixes around importing and removing unnecessary code

* Remove tags for now

* Tests for lib/

* Some fixes

* Ensure we clear index pattern cache

* Parity with master

* Revert any changes in package.json

* Reset any changes in this file

* Move the new argumen to the end to prevent test failures

* Fix functional tests

* Add relationship tests

* Fix tests

* API integration tests for relationships

* Ensure we're properly waiting for things to happen

* Fix test issue

* Wait for the table to finish loading instead of the whole page

* Tests for objects_table

* Componentry tests

* Ensure this is grabbing the right field

* Update snapshot

* Fixes with importing index patterns

* PR feedback

* PR feedback

* PR feedback

* Update snapshot

* PR feedback

* Update snapshot

* Respect the savedObjects:perPage config

* Updates from PR feedback

* More updates from PR feedback

* Make this more efficient

* Add debugging for functional test failures

* Wait longer

* Wrap each button accessor with a retry.try

* Try wrapping this in a retry.try

* Debug

* Lets make sure it is visible

* Maybe the short timeout is affecting this - use the default timeout which should be higher and allow more time for the animation to finish

* Rewrite this per suggestions from stacey
chrisronline added a commit to chrisronline/kibana that referenced this pull request May 17, 2018
* Not working proto code

* More proto code

* Work in progress

* Just go back to non interactive searching, much easier

* This should be on the server

* Revert "[@kbn/ui-framework] move ui-framework to a package (elastic#17085)"

This reverts commit ef3339b.

* Revert "Revert "[@kbn/ui-framework] move ui-framework to a package (elastic#17085)""

This reverts commit ce9ce14e1060c426090b55a5367de3ff4329e681.

* Use BasicTable properly

* Table improvements

* Small tweaks to the table

* Improvements

* Flyout mostly working

* Remove in memory table

* Getting close

* Tweaks

* Revamping server code, still need to support editing

* Progress

* Fix export

* Updates and passing functional tests

* Better links in relationships flyout

* Add skip import option

* Fixes around importing and removing unnecessary code

* Remove tags for now

* Tests for lib/

* Some fixes

* Ensure we clear index pattern cache

* Parity with master

* Revert any changes in package.json

* Reset any changes in this file

* Move the new argumen to the end to prevent test failures

* Fix functional tests

* Add relationship tests

* Fix tests

* API integration tests for relationships

* Ensure we're properly waiting for things to happen

* Fix test issue

* Wait for the table to finish loading instead of the whole page

* Tests for objects_table

* Componentry tests

* Ensure this is grabbing the right field

* Update snapshot

* Fixes with importing index patterns

* PR feedback

* PR feedback

* PR feedback

* Update snapshot

* PR feedback

* Update snapshot

* Respect the savedObjects:perPage config

* Updates from PR feedback

* More updates from PR feedback

* Make this more efficient

* Add debugging for functional test failures

* Wait longer

* Wrap each button accessor with a retry.try

* Try wrapping this in a retry.try

* Debug

* Lets make sure it is visible

* Maybe the short timeout is affecting this - use the default timeout which should be higher and allow more time for the animation to finish

* Rewrite this per suggestions from stacey
chrisronline added a commit that referenced this pull request May 17, 2018
* [Management] Saved objects to React/EUI! (#17426)

* Not working proto code

* More proto code

* Work in progress

* Just go back to non interactive searching, much easier

* This should be on the server

* Revert "[@kbn/ui-framework] move ui-framework to a package (#17085)"

This reverts commit ef3339b.

* Revert "Revert "[@kbn/ui-framework] move ui-framework to a package (#17085)""

This reverts commit ce9ce14e1060c426090b55a5367de3ff4329e681.

* Use BasicTable properly

* Table improvements

* Small tweaks to the table

* Improvements

* Flyout mostly working

* Remove in memory table

* Getting close

* Tweaks

* Revamping server code, still need to support editing

* Progress

* Fix export

* Updates and passing functional tests

* Better links in relationships flyout

* Add skip import option

* Fixes around importing and removing unnecessary code

* Remove tags for now

* Tests for lib/

* Some fixes

* Ensure we clear index pattern cache

* Parity with master

* Revert any changes in package.json

* Reset any changes in this file

* Move the new argumen to the end to prevent test failures

* Fix functional tests

* Add relationship tests

* Fix tests

* API integration tests for relationships

* Ensure we're properly waiting for things to happen

* Fix test issue

* Wait for the table to finish loading instead of the whole page

* Tests for objects_table

* Componentry tests

* Ensure this is grabbing the right field

* Update snapshot

* Fixes with importing index patterns

* PR feedback

* PR feedback

* PR feedback

* Update snapshot

* PR feedback

* Update snapshot

* Respect the savedObjects:perPage config

* Updates from PR feedback

* More updates from PR feedback

* Make this more efficient

* Add debugging for functional test failures

* Wait longer

* Wrap each button accessor with a retry.try

* Try wrapping this in a retry.try

* Debug

* Lets make sure it is visible

* Maybe the short timeout is affecting this - use the default timeout which should be higher and allow more time for the animation to finish

* Rewrite this per suggestions from stacey

* Update snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.3.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants