-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[TSVB] Allows the user to change the tooltip mode #67775
Conversation
1a54910
to
d26a32b
Compare
@elasticmachine merge upstream |
Pinging @elastic/kibana-app (Team:KibanaApp) |
We need help on the options of the dropdown menu from a writer. @KOTungseth can you help me with this? |
1c236d5
to
23a6c86
Compare
What do you think about changing And for the dropdown, how about:
|
@KOTungseth I like it, it is much simpler |
23a6c86
to
0fc0902
Compare
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.
Screenshots combined with text updates LGTM. 👍 Thanks!
src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/index.js
Outdated
Show resolved
Hide resolved
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.
We have a validation in place on the serverside, that validates all config sent to it. You need to add the tooltip_mode
to the validation schema there, because currently this PR will log to the server console: Request validation error: [panels.0.tooltip_mode]: definition for this key is missing (saved object id: unsaved). This most likely means your TSVB visualization contains outdated configuration. You can report this problem under https://github.com/elastic/kibana/issues/new?template=Bug_report.md
@@ -247,6 +247,7 @@ export const visPayloadSchema = schema.object({ | |||
series: schema.arrayOf(seriesItems), | |||
show_grid: numberIntegerRequired, | |||
show_legend: numberIntegerRequired, | |||
tooltip_mode: stringOptionalNullable, |
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.
Instead of allowing any string here, could we please make this just specific for the options? schema.maybe(schema.oneOf(['show_all', 'show_focused']))
341998a
to
073405d
Compare
{ | ||
label: intl.formatMessage({ | ||
id: 'visTypeTimeseries.timeseries.tooltipOptions.showFocused', | ||
defaultMessage: 'Show single value', |
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.
Let's please rename this to Show focused values
. Show single value does not really make sense, since we not really show only a single value, but if multiple values are close together we still show them all. I have the feeling it's rather confusing naming it "Show single value" and then actually showing multiple values in the tooltip.
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.
Despite the label change, everything looks good to me. Tested on Firefox & Chrome Linux. Code LGTM. Once label changed and CI is green, feel free to merge
1c1eac0
to
05a7bc4
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Allows the user to select the tooltip mode * Fix problem on new TSVB and add new field to schema * Change default value on tooltip dropdown for the focused series option
* master: (37 commits) [DOCS] Adds documentation for drilldowns (elastic#68122) [Telemetry] telemetry.sendUsageFrom: "server" by default (elastic#68071) [ML] Transforms: Support sub-aggregations (elastic#68306) Fixed pre-configured docs link points to the wrong page and functional tests configs (elastic#68606) [Ingest Manager] Update queries from `stream.*` to `dataset.*` (elastic#68322) Enable Watcher by default to fix bug in which Watcher doesn't render in the side nav (elastic#68602) Convert Index Templates API routes to snakecase. (elastic#68463) [SECURITY SOLUTION] [Detections] Add / Update e2e tests to ensure initial rule runs are successful (elastic#68441) [Ingest] OpenAPI spec file (elastic#68323) chore(NA): skip apis Endpoint plugin Endpoint policy api (elastic#68638) bumping makelogs version to v6.0.0 (elastic#66163) [SIEM] Add create template button (elastic#66613) Bump websocket-extensions from 0.1.3 to 0.1.4 (elastic#68414) [ML] Sample data modules - use event.dataset instead of index name (elastic#68538) [ML] Functional tests - fix job validation API test with maxModelMemoryLimit (elastic#68501) [ML] Functional tests - stabilize DFA job creation (elastic#68495) [TSVB] Allows the user to change the tooltip mode (elastic#67775) chore(NA): skip apis Endpoint plugin Endpoint alert API when data is in elasticsearch (elastic#68613) chore(NA): skip endpoint Endpoint Alert Page: when es has data and user has navigated to the page (elastic#68596) [SIEM][Detection Engine] Converts from joi to use io-ts and moves the types to common (elastic#68127) ...
Could it be added to the TSVB docs? The current one points to |
Summary
Fixes #60013. We added a dropdown to the panel options of TSVB in order the user to be able to toggle the tooltip mode. So now the user can also see in the tooltip only the values of the selected datapoint.
On the following video we can see the new functionality
Checklist
Delete any items that are not applicable to this PR.