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

Fixes data points on maps not showing up in reports #31949

Merged
merged 3 commits into from
Feb 26, 2019
Merged

Fixes data points on maps not showing up in reports #31949

merged 3 commits into from
Feb 26, 2019

Conversation

joelgriffith
Copy link
Contributor

Maps didn't like rendering into a parent with a 0 width/height, so I went with a more granular approach of explicitly hiding certain css properties vs the width/height hack. I've attached some before/after PDF's that highlight the fix and some subtle differences.
After.pdf
Uploading Before.pdf…

@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -121,8 +124,11 @@ visualize-app .visEditor__canvas {
* to handle this fine, but firefox moves the visualizations around.
*/
dashboard-app .react-grid-item {
height: 0 !important; /* 1. */
width: 0 !important; /* 1. */
Copy link
Member

Choose a reason for hiding this comment

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

Removing these makes the above block-level comment outdated.

It looks like it already was outdated though, as there is no server/lib/screenshot file anymore. I am not sure what "can only use the properties that will be manually set..." means

@joelgriffith
Copy link
Contributor Author

So, it turns out waiting a bit to inject the custom CSS allows all the items to render properly (and we don't have to touch hacky CSS stuff).
Final.pdf

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

LGTM - this solution is WAY better than adding additional hacks

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joelgriffith
Copy link
Contributor Author

Will work on backporting this

@joelgriffith joelgriffith merged commit db2e920 into elastic:master Feb 26, 2019
@joelgriffith joelgriffith added v7.0.0 backport pending (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v6.7.0 labels Feb 26, 2019
joelgriffith added a commit that referenced this pull request Feb 26, 2019
* Fixes data points on maps not showing up in reports
joelgriffith added a commit that referenced this pull request Feb 26, 2019
* Fixes data points on maps not showing up in reports
@tsullivan
Copy link
Member

@joelgriffith looks like this did not get backported to 7.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport pending (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants