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

Restructure files in kbn_vislib_vis_types folder #43866

Merged

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Aug 23, 2019

Summary

This changes intend more comfortable usage of common options params and consistency with other places.

file_structure

After further EUIfication we'll get rid of controls and editors folders

Instead of importing components from different files, now they are available just from one place:

--before

import { SwitchOption } from '../../controls/switch';
import { TextInputOption } from '../../controls/text_input';

--now

import { SwitchOption, TextInputOption } from '../../common';

--from external plugins

import {
  NumberInputOption,
  SelectOption,
  SwitchOption,
} from '../../../kbn_vislib_vis_types/public/components';

This also removes unused point-series-options directive with its template.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@sulemanof sulemanof added the release_note:skip Skip the PR/issue when compiling release notes label Aug 23, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Is there a reason why we are not keeping the folders in the hierarchy to separate general purpose components and whole editors but re-export them on the top level? Then you can import easily from the same path but you don't lose a clean folder structure below.

@sulemanof
Copy link
Contributor Author

Is there a reason why we are not keeping the folders in the hierarchy to separate general purpose components and whole editors but re-export them on the top level? Then you can import easily from the same path but you don't lose a clean folder structure below.

Hey @flash1293 , I didn't really get your point..
Could you please attach an example of your vision?

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

@sulemanof Looked at the folder structure again and it's already separated as I would have done it - I misread the diff, sorry for that. LGTM

@maryia-lapata
Copy link
Contributor

retest

@sulemanof sulemanof force-pushed the EUIfication/options/file_restructure branch from fc9399c to 056320b Compare August 26, 2019 10:10
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@maryia-lapata maryia-lapata left a comment

Choose a reason for hiding this comment

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

LGTM!

@maryia-lapata maryia-lapata added Feature:Vis Editor Visualization editor issues Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.0 v8.0.0 labels Aug 26, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@dmlemeshko dmlemeshko requested a review from kobelb August 27, 2019 12:06
@dmlemeshko
Copy link
Member

@kobelb Maybe you can help with any ideas why security tests under x-pack are failing for this PR?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sulemanof
Copy link
Contributor Author

Hey @kobelb !
I fixed CI by the last commit, but it still seems weird for us. Could you please take a look ?

@sulemanof sulemanof merged commit 15377f3 into elastic:master Aug 28, 2019
@sulemanof sulemanof deleted the EUIfication/options/file_restructure branch August 28, 2019 07:36
sulemanof added a commit to sulemanof/kibana that referenced this pull request Aug 28, 2019
* Files restructure

* Get rid of export from index
@sulemanof sulemanof removed the v7.5.0 label Aug 28, 2019
sulemanof added a commit that referenced this pull request Aug 28, 2019
* Files restructure

* Get rid of export from index
friol pushed a commit to friol/kibana that referenced this pull request Aug 29, 2019
* Files restructure

* Get rid of export from index
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Feature:Vis Editor Visualization editor issues release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants