Skip to content
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

Update vega vizs in sample web logs dashboard so applying filter won't cause them to look like they are erroring out #75485

Closed
bhavyarm opened this issue Aug 19, 2020 · 7 comments · Fixed by #117518
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Vega Vega visualizations impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure :VisEditors:fix-it-week

Comments

@bhavyarm
Copy link
Contributor

Kibana version: 7.9.0

**Elasticsearch version:**7.9.0

Server OS version: darwin_x86_64

Browser version: chrome latest

Browser OS version: OS X

Original install method (e.g. download page, yum, from source, etc.): from staging

Describe the bug: In 7.9.0 - on weblogs sample data dashboard - if user adds particular source country and byte value in input controls - sample vega vizs display - Infinite extent for field "time": [Infinity, -Infinity] and other messages of that variety

@flash1293 told me (thank you) - its because vega viz got updated 5(#68639 (review)) and they are valid messages but unfortunately they are really unfriendly to the user who might be using Kibana and sample dashboards for the first time.

Can we either fix the messages or change the vizs so this doesn't happen?

Screen Shot 2020-08-19 at 3 17 38 PM

Screen Shot 2020-08-19 at 3 17 42 PM

@bhavyarm bhavyarm added bug Fixes for quality problems that affect the customer experience Feature:Vega Vega visualizations Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Aug 19, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this issue Nov 6, 2020
…t cause them to look like they are erroring out

Closes elastic#75485
@timroes timroes added the impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. label Jul 22, 2021
@flash1293
Copy link
Contributor

@dziyanadzeraviankina I thought about this and let's go the disabling warnings route via config in the vega spec with an explanatory comment for this.

@DianaDerevyankina
Copy link
Contributor

Disabling warnings by default with hideWarnings option also hides other vega warnings, which is probably barely better than showing this one caused by no results. So the question is what is more important: not showing all warnings at all or keep it as it is? @ghudgins what is your opinion on that?
I guess at least we might add hideWarnings to config with an explanatory comment to the default spec and other vega sample data visualizations, to highlight it as a way to get rid of this warning.

@ghudgins
Copy link
Contributor

ghudgins commented Oct 21, 2021

Warnings aren't errors right? I think hiding is okay if so. I really wish we could hide warnings only in the view mode (dashboard) but show in the editor* but I understand this might not be doable right now.

@flash1293
Copy link
Contributor

flash1293 commented Oct 21, 2021

@ghudgins Actually I just checked the code and I was wrong earlier today - sorry about that. We can control the way warnings work and we can just not show them at all in "view mode" (on the dashboard).

Technically it would work like this:
In the renderer, pass handlers.getRenderMode() to the component:

Pass the flag to createVegaVisualization:

const VegaVis = createVegaVisualization(deps);

pass it to the VegaView class in the constructor:

this.vegaView = new VegaViewClass(vegaViewParams);

Store it in a member:

this._vegaStateRestorer = opts.vegaStateRestorer;

Read it in onWarn (and don't warn if it's set to "view"):

this._addMessage('warn', Utils.formatWarningToStr(...arguments));

But besides that - yes, errors are different and shown even if warnings are disabled.

@flash1293
Copy link
Contributor

@dziyanadzeraviankina once you get to it, could you resolve it as described above by showing warnings only in edit mode?

@flash1293
Copy link
Contributor

@dziyanadzeraviankina Here are the steps to set the render mode correctly for visualizations:

Add optional renderMode to the visualize embeddable input:

In

pass the mode to the expression loader (it will be handled from there)

      renderMode: this.input.renderMode || 'view',

In visualize, pass in edit render mode when creating the embeddable handler:

searchSessionId: data.search.session.getSessionId(),

  renderMode: 'edit',

From there is should work as described above as the expression loader will forward it to the expression renderer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Vega Vega visualizations impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure :VisEditors:fix-it-week
Projects
None yet
6 participants