-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Migrate timelion vis #62819
Migrate timelion vis #62819
Conversation
@elasticmachine merge upstream |
merge conflict between base and head |
Pinging @elastic/kibana-app (Team:KibanaApp) |
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.
Changes to features plugin LGTM
@@ -84,7 +84,7 @@ export class Plugin { | |||
// Register OSS features. | |||
for (const feature of buildOSSFeatures({ | |||
savedObjectTypes: this.legacyAPI.savedObjectTypes, | |||
includeTimelion: timelion !== undefined && timelion.uiEnabled, | |||
includeTimelion: visTypeTimelion !== undefined && visTypeTimelion.uiEnabled, |
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.
nit:
includeTimelion: visTypeTimelion !== undefined && visTypeTimelion.uiEnabled, | |
includeTimelion: !!visTypeTimelion?.uiEnabled, |
@import '@elastic/eui/src/themes/eui/eui_colors_light'; | ||
|
||
@import '@elastic/eui/src/global_styling/functions/index'; | ||
@import '@elastic/eui/src/global_styling/variables/index'; | ||
@import '@elastic/eui/src/global_styling/mixins/index'; |
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.
All of these imports can be removed when importing the manifest SASS file via the NP plugin.ts
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.
Done, thanks 👍
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.
Code LGTM, nice to have this clear structure for our 🦁. Just minor remarks
Tested locally in Chrome, creating a timelion vis, adding it to a dashboard, disabling the plugin, adding the timelion icon to the navbar, works.
What currently doesn't work so well is the Timelion app, but this is unrelated to this PR, it's also broken in master. It could be fixed here, else we should open an issue. There seems to be a problem with i18n (Just a "don't show again"
displayed in the following gif) and the brushing of the time range isn't displayed in the time picker
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.
Great, thanks for removing those. The SASS changes LGTM, so long as it's still building properly.
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / X-Pack Detection Engine API Integration Tests.x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses·ts.detection engine api security and spaces enabled find_statuses should return a single rule status when a single rule is loaded from a find status with defaults addedStandard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
* master: (40 commits) [APM]Upgrade apm-rum agent to latest version to fix full page reload (elastic#63723) add deprecation warning for legacy 3rd party plugins (elastic#62401) Migrate timelion vis (elastic#62819) Replacebad scope link with actual values (elastic#63444) Index pattern management UI -> TypeScript and New Platform Ready (create_index_pattern_wizard) (elastic#63111) [SIEM] Threat hunting enhancements: Filter for/out value, Show top field, Copy to Clipboard, Draggable chart legends (elastic#61207) [Maps] fix term join agg key collision (elastic#63324) [Ingest] Fix agent config key sorting (elastic#63488) [Monitoring] Fixed server response errors (elastic#63181) update elastic charts to 18.3.0 (elastic#63732) Start services (elastic#63720) [APM] Encode spaces when creating ML job (elastic#63683) Uptime 7.7 docs (elastic#62228) [DOCS] Updates remote cluster and ccr docs (elastic#63517) [Maps] Add 3rd party vector tile support (elastic#62084) [Endpoint][EPM] Retrieve Index Pattern from Ingest Manager (elastic#63016) [Endpoint] Host Details Policy Response Panel (elastic#63518) [Uptime] Certificate expiration threshold settings (elastic#63682) Refactor saved object types to use `namespaceType` (elastic#63217) [SIEM][CASE] Create comments sequentially (elastic#63692) ...
This PR moves the timelion visualization to the new platform.
It consolidates plugins for timelion api and visualization (
vis_type_timelion
) and renames all config keys.Prior to this PR there was
timelion
(NP plugin),timelion
(legacy plugin) andvis_timelion
(legacy plugin). Now it'stimelion
(legacy plugin containing just the app) andvis_type_timelion
(NP plugin containing everything else).All
timelion.*
configs keys are deprecated in favor ofvis_type_timelion.*
(old entries in kibana.yml will be forwarded to the new keys for the remaining 7.x releases - when they are present, Kibana will log a warning on startup)