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

[Lens] Xy gap settings #127749

Merged
merged 12 commits into from
Mar 22, 2022
Merged

[Lens] Xy gap settings #127749

merged 12 commits into from
Mar 22, 2022

Conversation

flash1293
Copy link
Contributor

Fixes #117280
Fixes #72691

This PR adds controls for the style of interpolated regions (either use dotted lines or regular style) as well as controls for end values.

Screenshot 2022-03-15 at 13 51 23

Screenshot 2022-03-15 at 13 51 28

This depends on #127453 to work right (otherwise there are no empty buckets to do fitting on)

@flash1293 flash1293 added release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:skip This commit does not require backporting v8.2.0 labels Mar 15, 2022
@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

expected head sha didn’t match current head ref.

@flash1293 flash1293 marked this pull request as ready for review March 17, 2022 18:01
@flash1293 flash1293 requested a review from a team as a code owner March 17, 2022 18:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@flash1293 I am sorry if my question is silly but I am confused a bit.

Here we mention that by default we are hiding the gaps
image

but by default the Include empty rows is on.
image

So this confuses me a bit. Include empty rows feels like filling the gaps to me.

@flash1293
Copy link
Contributor Author

@stratoula its showing at the table level (having rows without a metric value in the table), but hiding at the chart level. Should we change the wording? Or maybe the default? We could do linear interpolation by default maybe.

@stratoula
Copy link
Contributor

stratoula commented Mar 22, 2022

@flash1293 we changed it to linear by default to our agg-based visualizations. Maybe it makes sense to also do it for Lens?
I get the difference now btw, thanx :)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

After Joe's explanation, I tested it locally with many combinations. Code LGTM. It works as expected 👍

@flash1293
Copy link
Contributor Author

@stratoula To improve a bit, I changed the copy to By default, area and line charts hide the gaps in the data. To fill the gap, make a selection. to emphasize that this is a chart-type thing, not a general behavior.

@flash1293 flash1293 enabled auto-merge (squash) March 22, 2022 18:12
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 757 758 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 353 355 +2

Async chunks

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

id before after diff
lens 1.1MB 1.1MB +1.6KB

Page load bundle

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

id before after diff
lens 43.2KB 44.2KB +958.0B
Unknown metric groups

API count

id before after diff
lens 426 428 +2

History

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

@flash1293 flash1293 merged commit 9449f1d into elastic:main Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.2.0
Projects
None yet
5 participants