Skip to content

fixing brush in discover#26609

Merged
ppisljar merged 3 commits intoelastic:masterfrom
ppisljar:fix/discoverHistogram
Dec 4, 2018
Merged

fixing brush in discover#26609
ppisljar merged 3 commits intoelastic:masterfrom
ppisljar:fix/discoverHistogram

Conversation

@ppisljar
Copy link
Copy Markdown
Contributor

@ppisljar ppisljar commented Dec 4, 2018

Summary

Fixes #26571.

Fixes the Discover histogram so that clicking on bars/brushing works again, and adds functional tests for this behavior.

uses visualizeLoader in discover to load visualization:

  • additional flag autoFetch was added to visualizeLoader params, setting it to true won't auto fetch the data
  • render method of visualizeLoader was made public, so it can be called directly (without fetching data)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@ppisljar ppisljar added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Dec 4, 2018
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app

@timroes
Copy link
Copy Markdown
Contributor

timroes commented Dec 4, 2018

This looks like it's already fixed via #26600. So maybe sync up with @lukasolson which would be the better solution.

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@ppisljar ppisljar force-pushed the fix/discoverHistogram branch from 01a4f88 to b5e2e71 Compare December 4, 2018 08:40
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@ppisljar ppisljar force-pushed the fix/discoverHistogram branch from fbd82ba to e54de2f Compare December 4, 2018 09:53
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Copy link
Copy Markdown
Contributor

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Code LGTM. Tested on chrome and how brushing works correctly.

@ppisljar ppisljar merged commit 6f26171 into elastic:master Dec 4, 2018
ppisljar added a commit to ppisljar/kibana that referenced this pull request Dec 4, 2018
@markov00 markov00 mentioned this pull request Dec 4, 2018
ppisljar added a commit to ppisljar/kibana that referenced this pull request Dec 4, 2018
# Conflicts:
#	src/legacy/core_plugins/kibana/public/discover/controllers/discover.js
#	src/legacy/core_plugins/kibana/public/discover/index.html
#	src/ui/public/visualize/loader/embedded_visualize_handler.ts
ppisljar added a commit that referenced this pull request Dec 4, 2018
ppisljar added a commit that referenced this pull request Dec 4, 2018
ppisljar added a commit to ppisljar/kibana that referenced this pull request Dec 4, 2018
# Conflicts:
#	src/legacy/core_plugins/kibana/public/discover/controllers/discover.js
#	src/legacy/core_plugins/kibana/public/discover/index.html
#	src/ui/public/visualize/loader/embedded_visualize_handler.ts
ppisljar added a commit that referenced this pull request Dec 4, 2018
@stacey-gammon
Copy link
Copy Markdown

Nice tests! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants