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

new response handlers for vislib #12661

Closed
wants to merge 9 commits into from

Conversation

ppisljar
Copy link
Member

@ppisljar ppisljar commented Jul 5, 2017

two response handlers:

  • series_data: works with all vislib types except pie (line, area, bar, heatmap. metric, gauge, goal)
  • hierarchical_data: works with pie chart

goals:

  • decouple vislib from ES response (aggConfigs)
  • decouple tooltips from ES response
  • decouple event handling (filter, brush, hover) from ES response
  • decouple <visualization> from ES response
  • provide easy to use data format

as side effect two issues are fixed:
closes #12746
closes #10543

things that are still missing:

  • tests for new response handlers

other charts should be converted to use one of the new request handlers as well, probably the third one will need to be added (table_data)

using visualizations is now fairly easy as the data object is no longer coupled to elastic search.

const myVis = new Vis(indexPattern, 'bar');
const myData = {
  charts: [{
    label: 'my chart label',
    series: [{
      label: 'Count',
      values: [{ x: 'a', y: 120}, { x: 'b', y: 80 }, { x: 'c', y: 140 }]
    }]
  }]
};
<visualization vis=myVis data=myData></visualization>

@ppisljar ppisljar force-pushed the feature/visualize branch from 20bf21c to 033806a Compare July 7, 2017 14:47
@ppisljar ppisljar force-pushed the refactor/dataFormat branch from 93e70dd to cbaa7a3 Compare July 20, 2017 08:01
@ppisljar ppisljar changed the base branch from feature/visualize to master July 20, 2017 08:01
@ppisljar ppisljar force-pushed the refactor/dataFormat branch from cbaa7a3 to f301fe3 Compare July 20, 2017 11:16
@ppisljar ppisljar added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) WIP Work in progress labels Jul 20, 2017
@ppisljar ppisljar requested a review from thomasneirynck July 20, 2017 11:19
@ppisljar ppisljar force-pushed the refactor/dataFormat branch 10 times, most recently from c2bd94a to 3c66c4e Compare July 27, 2017 17:28
@ppisljar ppisljar changed the title [WIP] changing data format and adding response handlers new response handlers for vislib Jul 27, 2017
@nreese
Copy link
Contributor

nreese commented Jul 27, 2017

Discover date histogram does not show extended bounds. Notice in the screen shot that the user is asking for the last 30 days yet the timeline only shows the last couple of days, 07-24 to 07-27. The same behavior occurs with a vertical bar chart split on date_histogram visualization.

screen shot 2017-07-27 at 12 59 23 pm

@nreese
Copy link
Contributor

nreese commented Jul 27, 2017

No tooltip and console error exist when you first create a pie chart visualization

screen shot 2017-07-27 at 1 12 54 pm

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good.

@@ -1,7 +1,8 @@
import moment from 'moment';

export default {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as you are updating all of the fixtures, how about exporting a named object instead of default?

const HierarchicalResponseHandlerProvider = function () {
/***
* Prepares series out of esResponse.
* each combination of metric, segment bucket and group bucket will add a series
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update comment for hierarchical response

@ppisljar ppisljar force-pushed the refactor/dataFormat branch 6 times, most recently from 43677da to c53dead Compare July 31, 2017 10:00
@@ -0,0 +1,23 @@
const tooltipFormatter = (event) => {
const getValue = (agg) => {
if (agg.value === 'y') {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably be {{y}} or similar

@ppisljar ppisljar force-pushed the refactor/dataFormat branch from b5c75b3 to 9087ac4 Compare September 7, 2017 08:50
@ppisljar
Copy link
Member Author

ppisljar commented Oct 6, 2017

closing, will reimplement but based on tabify resposne not es response

@ppisljar ppisljar closed this Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chart and legend should be ordered by filter agg order
2 participants