[ML] Reactive time-range selection in Single Metric Viewer#51008
Merged
darnautov merged 27 commits intoelastic:masterfrom Nov 25, 2019
Merged
[ML] Reactive time-range selection in Single Metric Viewer#51008darnautov merged 27 commits intoelastic:masterfrom
darnautov merged 27 commits intoelastic:masterfrom
Conversation
Contributor
|
Pinging @elastic/ml-ui (:ml) |
Contributor
💔 Build Failed
|
Contributor
💔 Build Failed
|
Contributor
💔 Build Failed
|
Contributor
|
I couldn't find a definitive reference in any of the style guides, but Kibana uses a |
292f9e0 to
98e8fd7
Compare
Contributor
💔 Build Failed
|
Contributor
peteharverson
left a comment
There was a problem hiding this comment.
Overall this is looking good. On Firefox I am occasionally seeing a blank focus chart on switching jobs, with these errors in the console:
Contributor
💚 Build Succeeded
|
10 tasks
walterra
reviewed
Nov 21, 2019
Contributor
walterra
left a comment
There was a problem hiding this comment.
Great work, I added some minor suggestions. Will report back once I did some local testing 😅
x-pack/legacy/plugins/ml/public/services/ml_api_service/index.d.ts
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/ml/public/services/ml_api_service/index.js
Outdated
Show resolved
Hide resolved
walterra
approved these changes
Nov 21, 2019
Contributor
walterra
left a comment
There was a problem hiding this comment.
Latest changes LGTM, also tested locally in Chrome/OSX.
alvarezmelissa87
approved these changes
Nov 21, 2019
Contributor
alvarezmelissa87
left a comment
There was a problem hiding this comment.
Tested on FF/Chrome/Safari and LGTM ⚡️
Contributor
💚 Build Succeeded
|
…ating the default range
ac068dc to
828fc04
Compare
Contributor
💚 Build Succeeded
|
Contributor
💚 Build Succeeded |
Contributor
💚 Build Succeeded |
Contributor
💚 Build Succeeded |
darnautov
added a commit
to darnautov/kibana
that referenced
this pull request
Nov 25, 2019
* [ML] http service to TS, add httpCall using fromFetch * [ML] types, add esSearchRx * [ML] timeresiesexplorer_contans to ts * [ML] timeseries_search_service to TS, add getMetricDataRx * [ML] result service with observables * [ML] update resolvers, forecast data support * [ML] wip timeseriesexplorer * [ML] fix state update for zoom * [ML] skip loading update * [ML] cleanup contextChartSelected * [ML] add to subscriptions * [ML] update imports * [ML] timeseriesexplorer_utils * [ML] refactor result service * [ML] getAnnotations * [ML] rename subject * [ML] fix explorer and unit tests * [ML] fix forecast * [ML] replace skipWhilte with filter * [ML] rename http$ * [ML] rename esSearch$ * [ML] remove filter operator, check for contextChartData before calculating the default range * [ML] remove casting for FocusData * [ML] replace with an arrow function * [ML] fix Job import path * [ML] fix annotations
darnautov
added a commit
that referenced
this pull request
Nov 25, 2019
* [ML] http service to TS, add httpCall using fromFetch * [ML] types, add esSearchRx * [ML] timeresiesexplorer_contans to ts * [ML] timeseries_search_service to TS, add getMetricDataRx * [ML] result service with observables * [ML] update resolvers, forecast data support * [ML] wip timeseriesexplorer * [ML] fix state update for zoom * [ML] skip loading update * [ML] cleanup contextChartSelected * [ML] add to subscriptions * [ML] update imports * [ML] timeseriesexplorer_utils * [ML] refactor result service * [ML] getAnnotations * [ML] rename subject * [ML] fix explorer and unit tests * [ML] fix forecast * [ML] replace skipWhilte with filter * [ML] rename http$ * [ML] rename esSearch$ * [ML] remove filter operator, check for contextChartData before calculating the default range * [ML] remove casting for FocusData * [ML] replace with an arrow function * [ML] fix Job import path * [ML] fix annotations
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Summary
The main purpose of this PR is to demonstrate how RxJS can improve UX and developer experience by helping to deal with async flow. As an example, I've used time-range selection with a brush on Single Metric Viewer. There is a wide range of functions and operators, for instance, I used
debounceTimefor postponing the actual API calls andfromFetchto take advantage of built-in abort signal for HTTP requests cancellation on transforming the Observable withswitchMap.Some of the files have been migrated to TS, but it's not the main focus so suggestions regarding missing interfaces and types appreciated! Besides, there are new files that partially duplicate the code from JS services, which will be updated and merged accordingly.
Checklist
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibility