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.

Sign up for GitHub

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

Jump to bottom

[NP] Import saved objects styles directly #58208

Merged
merged 8 commits into from
Feb 27, 2020
Merged

[NP] Import saved objects styles directly #58208

merged 8 commits into from
Feb 27, 2020

Conversation

maryia-lapata
Copy link
Contributor

@maryia-lapata maryia-lapata commented Feb 21, 2020 •
edited
Loading

Part of #58211.

Imports src/plugins/saved_objects's styles in that module instead of proxy import in ui/saved_objects. The folder ui/public/saved_objects was completely removed.

@maryia-lapata maryia-lapata added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration v7.7.0 labels Feb 21, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293 flash1293 mentioned this pull request Feb 21, 2020
Closed
2 tasks
flash1293
flash1293 approved these changes Feb 21, 2020
View reviewed changes
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.

Changes LGTM, didn't test locally

@maryia-lapata maryia-lapata marked this pull request as ready for review February 21, 2020 12:06
@maryia-lapata maryia-lapata requested a review from a team as a code owner February 21, 2020 12:06
@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

cchaos
cchaos reviewed Feb 26, 2020
View reviewed changes
src/plugins/saved_objects/public/plugin.ts Outdated
@@ -19,6 +19,8 @@

import { Plugin } from 'src/core/public';

import './_index.scss';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the pattern now is that if the SASS file is imported only and directly in a JS file, it should no longer use the _ pattern. So the file name and this import should change to index.scss

cchaos
cchaos approved these changes Feb 26, 2020
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

I didn't test this locally, but the imports LGTM 👍

@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@maryia-lapata maryia-lapata merged commit 92840fe into elastic:master Feb 27, 2020
@maryia-lapata maryia-lapata deleted the saved_objects_scc branch February 27, 2020 07:36
@maryia-lapata maryia-lapata mentioned this pull request Feb 27, 2020
Merged
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Feb 28, 2020
maryia-lapata added a commit that referenced this pull request Feb 28, 2020
@maryia-lapata @elasticmachine
[NP] Import saved objects styles directly (#58208) (#58697)
8077d34 
* Import styles for saved objects

* Rename index.scss

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cchaos cchaos cchaos approved these changes

@flash1293 flash1293 flash1293 approved these changes

Assignees
No one assigned
Labels
backported Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maryia-lapata @elasticmachine @kibanamachine @cchaos @flash1293