[TSVB] Show an indicator when using Last Value mode by DianaDerevyankina · Pull Request #91977 · elastic/kibana

DianaDerevyankina · 2021-02-19T07:16:05Z

Closes #90248

Summary

Put "Last value" badge to all TSVB panel types except timeseries

Added a tooltip with last bucket date and interval information (was moved here from the editor)

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

alexwizp · 2021-02-19T07:21:15Z

src/plugins/vis_type_timeseries/public/application/components/vis_editor_visualization.js

-    const interval = convertIntervalIntoUnit(this.state.panelInterval, false);
-
-    return interval ? `${interval.unitValue}${interval.unitString}` : null;
+    return [


nit: I see you renamed that method to isAnyPanelTypeExceptTimeseries maybe in that case instead of

[ PANEL_TYPES.METRIC, PANEL_TYPES.TOP_N, PANEL_TYPES.GAUGE, PANEL_TYPES.MARKDOWN, PANEL_TYPES.TABLE, ].includes(type) &&

we can writee just type !== PANEL_TYPES.TIMESERIES

alexwizp · 2021-02-19T07:42:28Z

Broken case:

Open TSVB and open any sample data
Set 1 second as a value for global timeeframe
Refresh visualization and see next screen

alexwizp · 2021-02-19T07:49:45Z

src/plugins/vis_type_timeseries/public/application/components/data_time_range_mode_label.tsx

+import { TIME_RANGE_DATA_MODES } from '../../../common/timerange_data_modes';
+
+const lastValueFormattedMessage = (
+  <FormattedMessage


use i18n.translate for non-JSX translations

alexwizp · 2021-02-19T07:50:56Z

src/plugins/vis_type_timeseries/public/application/components/data_time_range_mode_label.tsx

+);
+
+interface DataTimeRangeModeLabelProps {
+  seriesData: Array<Array<number | null>>;


is it PanelData['data'] ?

alexwizp · 2021-02-19T07:55:42Z

src/plugins/vis_type_timeseries/public/application/components/lib/get_interval.ts

+export const convertIntervalIntoUnit = (interval: number, hasTranslateUnitString = true) => {
  // Iterate units from biggest to smallest
-  const units = Object.keys(unitLookup).reverse();
+  const units = Object.keys(unitLookup).reverse() as TimeUnits;


what if we reefactor it to:

type TimeUnit = keyof typeof unitLookup; const units = Object.keys(unitLookup).reverse() as TimeUnit[];

alexwizp · 2021-02-19T07:57:08Z

src/plugins/vis_type_timeseries/public/application/components/lib/get_interval.ts

+export const getInterval = (visData: TimeseriesVisData, model: TimeseriesVisParams) => {
+  const series = get(
+    visData,
+    model?.type === 'table' ? `series[0].series` : `${model.id}.series`,


please replace 'table' to PANEL_TYPES.TABLE

alexwizp · 2021-02-19T07:59:50Z

src/plugins/vis_type_timeseries/public/application/components/lib/get_interval.ts

+export const isAutoInterval = (interval: string) => !interval || interval === AUTO_INTERVAL;
+
+interface ValidationResult {
+  errorMessage: string;


looks like errorMessage should be optional

…ctoring

alexwizp · 2021-02-23T07:12:30Z

@elasticmachine merge upstream

alexwizp · 2021-02-23T07:16:05Z

src/plugins/vis_type_timeseries/public/application/components/data_time_range_mode_label.tsx

+  modelInterval,
+  modelTimeRangeMode,
+}: DataTimeRangeModeLabelProps) => {
+  const hasShowPanelIntervalValue = () =>


I see you usee that method only for getting value for getFormattedPanelInterval. Maybe we can move isAutoInterval(modelInterval) || isGteInterval(modelInterval) directly to getFormattedPanelInterval

alexwizp · 2021-02-23T07:17:13Z

src/plugins/vis_type_timeseries/public/application/components/data_time_range_mode_label.tsx

+  };
+
+  const getLastValueLabelWithTooltip = () => {
+    const dateFormat = getUISettings().get('dateFormat');


please move it up to don't calculate dateFormat and scaledDataFormat on each time

alexwizp · 2021-02-23T07:19:25Z

x-pack/plugins/infra/server/lib/adapters/framework/kibana_framework_adapter.ts

      filters,
    };
-    return getVisData(requestContext, rawRequest, options);
+    return getVisData(requestContext, rawRequest, options) as Promise<InfraTSVBResponse>;


could you please explain why it wass changed?

@dziyanadzeraviankina could you please try to export right types from TSVB plugin and replace InfraTSVBResponse to it? I see in infra plugin some types which can be probably safely removed

alexwizp · 2021-02-23T07:30:52Z

@VladLasitsa @sulemanof please have a look

DianaDerevyankina · 2021-03-18T10:57:50Z

src/plugins/vis_type_timeseries/public/actions/last_value_mode_action.tsx

+    return false;
+  },
+  execute: async () => {},
+});


@streamich could you please take a look?
during the discussion, it's been decided to add an indicator on top of the embeddable to indicate if visualization is using last value mode
so what do you think about this solution? would appreciate any comments or other ideas how to implement that

i am missing some context, what is this supposed to do ? action without execute ? or is this work in progress ?

so after being explained what we need i think we should try to put the message inside TSVB (instead of panel header).
currently panel headers are showing things that are common for all the embeddables and i think it should stay that way.

if we really wanted to show some indication like this inside the panel header we need to expand the embeddable API to support this, which will take some time as we'll want to take it thru planning, rfc and arch review.

VladLasitsa · 2021-03-19T13:50:22Z

src/plugins/vis_type_timeseries/public/actions/last_value_mode_action.tsx

+import { PANEL_TYPES } from '../../common/panel_types';
+import { Vis } from '../../../visualizations/public';
+
+const LAST_VALUE_MODE = 'lastValueMode';


Why do not you use constants from here src\plugins\vis_type_timeseries\common\timerange_data_modes.ts?

This reverts commit 15f16d6.

…ion to hide it

alexwizp · 2021-03-25T15:10:57Z

src/plugins/vis_type_timeseries/public/application/components/timeseries_visualization.tsx

@@ -87,18 +98,43 @@ function TimeseriesVisualization({

  const VisComponent = TimeseriesVisTypes[model.type];


Could you please check, I18nProvider should be already provided here

…derer

stratoula · 2021-03-26T09:18:53Z

@dziyanadzeraviankina this works great. Maybe it would make sense to display the gear only when the last value is selected. wdyt?

@wylieconlon do you want to have another look?

…led, fix gauge scroll issue

DianaDerevyankina · 2021-03-26T13:39:02Z

Maybe it would make sense to display the gear only when the last value is selected

nice idea, added a condition for that 👍

stratoula · 2021-03-26T14:51:01Z

Thank you Diana, it works fine 🙂

@gchaps do you have any suggestions for the popover text above?
If the toggle is on, the last value grey badge will appear on the visualization and dashboard, otherwise will not. I think I prefer something shorter. Something like:
Title: Last value options
Text: Display last value indicator. (I don't like the word indicator tbh, maybe badge? label?)

wylieconlon · 2021-03-26T15:09:35Z

I think this is working very well! I like all the new tweaks. I have comments that shouldn't block merging:

This new indicator is on by default for Last Value visualizations, even ones that I created before this change. This seems good to me, but raising it to everyone's attention.
I found this indicator confusing when using the Markdown visualization, because Markdown gives full access to the data, even when using Last Value mode. I don't think you need to change anything since there is a way to disable this. For example:

gchaps · 2021-03-26T18:39:19Z

@stratoula, I discussed the text with @wylieconlon. Here is a suggestion:

Title: Last value options
Text: Show label when using Last value mode

stratoula · 2021-03-26T18:40:22Z

I love it! Thank you both 🙂

ppisljar

LGTM

… is empty, create migration script to hide last value label for previously created visualizations and a test for that

kibanamachine · 2021-03-29T14:30:06Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypeTimeseries`	485	505	+20

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`visTypeTimeseries`	1.6MB	1.6MB	+9.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeTimeseries`	126.8KB	127.7KB	+883.0B

History

💚 Build #115472 succeeded 297b61f
💚 Build #115223 succeeded 8b8ee72
💔 Build #115131 failed 483d486
💔 Build #113945 failed c8d9ac2
💔 Build #113936 failed 15f16d6

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

cc @dziyanadzeraviankina

stratoula

I haven't tested it again but it seems fine to me. To wrap up we decided to hide the label for old TSVB visualizations and we applied a new text to the Last value options

stratoula · 2021-03-30T06:25:29Z

@elasticmachine run elasticsearch-ci/docs

simianhacker

LGTM

* [TSVB] Show an indicator when using Last Value mode * Extended some TSVB types, remove unused translations and do some refactoring * Fix some functional tests and label displaying for Last value * Fix some functional tests and label displaying for Last value * Refactor data_time_range_mode_label and change some types * fix CI * Refactor timeseries_visualization seriesData * Remove unused re export * Replace "href" prop with "onClick" in EuiLink and refactor tooltip content * Change link to text and add pointer style to it * FIx import in kibana_framework_adapter * Remove label for entire time range mode and add an icon for last value mode label * Add action to show last value label for TSVB embeddables * Fix TimeseriesVisParams import * Revert "Add action to show last value label for TSVB embeddables" This reverts commit 15f16d6. * Put the "Last value" badge on the top of visualization and add an option to hide it * Fix failing _tsvb_markdown test and refactor timeseries_visualization * Move I18nProvider frim timeseries_visualization to timeseries_vis_renderer * Add condition to hide gear button when entire time range mode is enabled, fix gauge scroll issue * Change text in the popover, add condition to indicator if series data is empty, create migration script to hide last value label for previously created visualizations and a test for that Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>

* [TSVB] Show an indicator when using Last Value mode * Extended some TSVB types, remove unused translations and do some refactoring * Fix some functional tests and label displaying for Last value * Fix some functional tests and label displaying for Last value * Refactor data_time_range_mode_label and change some types * fix CI * Refactor timeseries_visualization seriesData * Remove unused re export * Replace "href" prop with "onClick" in EuiLink and refactor tooltip content * Change link to text and add pointer style to it * FIx import in kibana_framework_adapter * Remove label for entire time range mode and add an icon for last value mode label * Add action to show last value label for TSVB embeddables * Fix TimeseriesVisParams import * Revert "Add action to show last value label for TSVB embeddables" This reverts commit 15f16d6. * Put the "Last value" badge on the top of visualization and add an option to hide it * Fix failing _tsvb_markdown test and refactor timeseries_visualization * Move I18nProvider frim timeseries_visualization to timeseries_vis_renderer * Add condition to hide gear button when entire time range mode is enabled, fix gauge scroll issue * Change text in the popover, add condition to indicator if series data is empty, create migration script to hide last value label for previously created visualizations and a test for that Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com> Co-authored-by: Diana Derevyankina <54894989+DziyanaDzeraviankina@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>

[TSVB] Show an indicator when using Last Value mode

5c21f3c

DianaDerevyankina added Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.0.0 v7.13.0 labels Feb 19, 2021

DianaDerevyankina requested review from VladLasitsa, alexwizp and sulemanof February 19, 2021 07:16

DianaDerevyankina self-assigned this Feb 19, 2021

alexwizp reviewed Feb 19, 2021

View reviewed changes

DianaDerevyankina added 4 commits February 19, 2021 11:57

Extended some TSVB types, remove unused translations and do some refa…

2b7d05d

…ctoring

Merge branch 'master' into issues/90248

597235f

Fix some functional tests and label displaying for Last value

50d6548

Fix some functional tests and label displaying for Last value

d64166f

Merge branch 'master' into issues/90248

fc6afcc

alexwizp reviewed Feb 23, 2021

View reviewed changes

alexwizp requested a review from stratoula February 23, 2021 08:12

DianaDerevyankina and others added 3 commits February 24, 2021 12:19

Refactor data_time_range_mode_label and change some types

1a1fb28

fix CI

7805485

Refactor timeseries_visualization seriesData

9b99278

DianaDerevyankina commented Mar 18, 2021

View reviewed changes

Fix TimeseriesVisParams import

c8d9ac2

VladLasitsa reviewed Mar 19, 2021

View reviewed changes

DianaDerevyankina added 4 commits March 24, 2021 12:57

Revert "Add action to show last value label for TSVB embeddables"

ff0d774

This reverts commit 15f16d6.

Merge branch 'master' into issues/90248

201ab65

Put the "Last value" badge on the top of visualization and add an opt…

b4986ac

…ion to hide it

Merge branch 'master' into issues/90248

483d486

alexwizp reviewed Mar 25, 2021

View reviewed changes

DianaDerevyankina added 2 commits March 25, 2021 18:35

Fix failing _tsvb_markdown test and refactor timeseries_visualization

4d5bd1a

Move I18nProvider frim timeseries_visualization to timeseries_vis_ren…

8b8ee72

…derer

DianaDerevyankina added 2 commits March 26, 2021 14:24

Merge branch 'master' into issues/90248

879661f

Add condition to hide gear button when entire time range mode is enab…

297b61f

…led, fix gauge scroll issue

Merge branch 'master' into issues/90248

745e70e

ppisljar approved these changes Mar 29, 2021

View reviewed changes

Change text in the popover, add condition to indicator if series data…

7be6a0d

… is empty, create migration script to hide last value label for previously created visualizations and a test for that

stratoula approved these changes Mar 30, 2021

View reviewed changes

simianhacker self-requested a review March 30, 2021 15:22

simianhacker approved these changes Mar 30, 2021

View reviewed changes

stratoula merged commit 78b16fd into elastic:master Mar 30, 2021

stratoula mentioned this pull request Mar 30, 2021

[7.x] [TSVB] Show an indicator when using Last Value mode (#91977) #95817

Merged

		@@ -87,18 +98,43 @@ function TimeseriesVisualization({

		const VisComponent = TimeseriesVisTypes[model.type];

Conversation

DianaDerevyankina commented Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexwizp commented Feb 19, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexwizp commented Feb 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexwizp commented Feb 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula commented Mar 26, 2021

Uh oh!

DianaDerevyankina commented Mar 26, 2021

Uh oh!

stratoula commented Mar 26, 2021

Uh oh!

wylieconlon commented Mar 26, 2021

Uh oh!

gchaps commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stratoula commented Mar 26, 2021

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 29, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula commented Mar 30, 2021

Uh oh!

simianhacker left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

DianaDerevyankina commented Feb 19, 2021 •

edited

Loading

gchaps commented Mar 26, 2021 •

edited

Loading