forked from elastic/elastic-charts
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Using EUI strictly as Dev dependency #2
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* fix(specs.ts): add custom style inside each specific series style feat(specs, series, utils, state): decouple styles from BasicSeriesSpec refactor(rendering tests): add | BasicSpecSeries to tests test(tooltip chart_state interactions): resolve BasicSeriesSpec tests Revert "test(tooltip chart_state interactions): resolve BasicSeriesSpec tests" This reverts commit c70a5aae69ad45c450d1040fff2fda5b5053bca7. Revert "refactor(rendering tests): add | BasicSpecSeries to tests" This reverts commit 43979a273ca507f8cadeea03c6869fe8f5e10b2a. Revert "feat(specs, series, utils, state): decouple styles from BasicSeriesSpec" This reverts commit 04607d0377b9a1caf8744e21e6ae7fdfbae04ad1. feat(specs.ts utils.ts): add type guards for series spec line bar area
Add the definition for the pull request job so that it can be managed from this repo. The values in defaults.yml will be merged with the pull request definition. `check_paths_for_matches` is a utility script used to prevent unnecessary builds from happening on every push to master due to problems with the Jenkins `git` plugin.
Enhancements: - Tooltips now use the EUI mixin - Legend uses overflow shadows
- Remove unused SCSS files - Fix up readme - Reset files now come in 2 themes
Merged
5 tasks
snide
approved these changes
Jun 4, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions for docs.
Co-Authored-By: [email protected] <[email protected]>
Co-Authored-By: [email protected] <[email protected]>
Thanks @snide ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Looks like the original branch needs rebasing from master
After looking at all the duplication of functions, mixins, and variables, it didn't seem sustainable to try to keep them up to date with EUI. But thank you for all that effort!
What I've done is just added EUI strictly as a devDependency and importing the various "invisibles" needed for the custom charts styles.
Since you were already compiling the SASS to 2 theme .css files, I continued with that idea so that for consumer not using EUI, they can just import the compiled CSS files.
I also added two reset files for when projects are using Charts as a standalone app (these shouldn't be imported when using EUI).
I've tested this in EUI and Kibana and it all seems to work great. I've updated the Consuming.md file to express the changes needed for getting the CSS.