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.

Sign up for GitHub

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

Jump to bottom

[Lens] fit line charts by default #196184

Merged
merged 13 commits into from
Oct 21, 2024
Merged

[Lens] fit line charts by default #196184

merged 13 commits into from
Oct 21, 2024

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Oct 14, 2024 •
edited by kibanamachine
Loading

Summary

Every line charts are now interpolated by default with a linear interpolation.

Solves the second task of #186076

fix #186076

before:
Screenshot 2024-10-17 at 16 25 47

after:
Screenshot 2024-10-17 at 16 25 56

Release note

Newly and default configured Lens line charts are now interpolated by default with a straight Linear interpolation.

@markov00 markov00 added release_note:enhancement enhancement New value added to drive a business result Feature:Lens backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 14, 2024
markov00
markov00 commented Oct 15, 2024
View reviewed changes
src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx
? 'always'
: 'never',
radius: pointsRadius !== undefined ? pointsRadius : xAccessor && !emphasizeFitting ? 5 : 0,
visible: showPoints || markSizeAccessor ? 'always' : 'never',
Copy link
Member Author

Choose a reason for hiding this comment

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

simplifying this, points should be visible if showPoints is true or markSizeAccessor exist. we don't need a check on the xAccessor

markov00
markov00 commented Oct 15, 2024
View reviewed changes
src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx
: 'never',
radius: pointsRadius !== undefined ? pointsRadius : xAccessor && !emphasizeFitting ? 5 : 0,
visible: showPoints || markSizeAccessor ? 'always' : 'never',
radius: pointsRadius,
Copy link
Member Author

Choose a reason for hiding this comment

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

same here on the point radius: if configured use the configured one, if not use the default from the theme. No need to check the fitting function or the emphasizeFitting property. This is already nicely handled by the library

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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
expressionXY 169 171 +2

Async chunks

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

id before after diff
dashboard 637.3KB 637.3KB +2.0B
expressionXY 127.7KB 127.6KB -115.0B
infra 1.7MB 1.7MB +2.0B
lens 1.5MB 1.5MB +213.0B
total +102.0B

Page load bundle

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

id before after diff
expressionXY 41.9KB 42.0KB +51.0B
Unknown metric groups

API count

id before after diff
expressionXY 180 182 +2

History

@markov00 markov00 marked this pull request as ready for review October 17, 2024 14:00
@markov00 markov00 requested review from a team as code owners October 17, 2024 14:00
@markov00 markov00 added backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 17, 2024
mbondyra
mbondyra approved these changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@mbondyra mbondyra left a comment •
edited
Loading

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! When you can could you please review before and after pics I added to the PR description? I am sure there can be better examples, but I added them for documentation purposes.

@markov00 markov00 added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v9.0.0 v8.17.0 and removed backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development labels Oct 17, 2024
@markov00 markov00 mentioned this pull request Oct 18, 2024
Closed
3 tasks
kpatticha
kpatticha approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@kpatticha kpatticha left a comment

Choose a reason for hiding this comment

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

changes in packages/kbn-lens-embeddable-utils LGTM

@markov00 markov00 added backport:version Backport to applied version labels and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 21, 2024
@markov00 markov00 merged commit 5fe8aad into elastic:main Oct 21, 2024
27 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11440752783

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 21, 2024
@markov00
[Lens] fit line charts by default (elastic#196184)
40922d1 
## Summary

Every line charts are now interpolated by default with a linear
interpolation.

Solves the second task of
[elastic#186076](elastic#186076)

fix [elastic#186076](elastic#186076)

before:
<img width="816" alt="Screenshot 2024-10-17 at 16 25 47"
src="https://github.com/user-attachments/assets/3b14c80b-deef-4d8d-9d5b-e118619e31cb">

after:
<img width="814" alt="Screenshot 2024-10-17 at 16 25 56"
src="https://github.com/user-attachments/assets/45788530-aeb6-4851-ac1e-c53efcd73068">

## Release note
Newly and default configured Lens line charts are now interpolated by
default with a straight Linear interpolation.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Marta Bondyra <[email protected]>
(cherry picked from commit 5fe8aad)
@kibanamachine kibanamachine mentioned this pull request Oct 21, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 21, 2024
@kibanamachine @markov00
[8.x] [Lens] fit line charts by default (#196184) (#197057)
b09f5ab 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens] fit line charts by default
(#196184)](#196184)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-21T13:05:02Z","message":"[Lens]
fit line charts by default (#196184)\n\n## Summary\r\n\r\nEvery line
charts are now interpolated by default with a
linear\r\ninterpolation.\r\n\r\nSolves the second task
of\r\n[#186076](https://github.com/elastic/kibana/issues/186076)\r\n\r\nfix
[#186076](https://github.com/elastic/kibana/issues/186076)\r\n\r\nbefore:\r\n<img
width=\"816\" alt=\"Screenshot 2024-10-17 at 16 25
47\"\r\nsrc=\"https://github.com/user-attachments/assets/3b14c80b-deef-4d8d-9d5b-e118619e31cb\">\r\n\r\n\r\nafter:\r\n<img
width=\"814\" alt=\"Screenshot 2024-10-17 at 16 25
56\"\r\nsrc=\"https://github.com/user-attachments/assets/45788530-aeb6-4851-ac1e-c53efcd73068\">\r\n\r\n##
Release note\r\nNewly and default configured Lens line charts are now
interpolated by\r\ndefault with a straight Linear
interpolation.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Marta Bondyra
<[email protected]>","sha":"5fe8aad89d59726060ba860d2f0692ddb3ca5ffa","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","enhancement","Feature:Lens","v9.0.0","apm:review","backport:version","v8.17.0"],"title":"[Lens]
fit line charts by
default","number":196184,"url":"https://github.com/elastic/kibana/pull/196184","mergeCommit":{"message":"[Lens]
fit line charts by default (#196184)\n\n## Summary\r\n\r\nEvery line
charts are now interpolated by default with a
linear\r\ninterpolation.\r\n\r\nSolves the second task
of\r\n[#186076](https://github.com/elastic/kibana/issues/186076)\r\n\r\nfix
[#186076](https://github.com/elastic/kibana/issues/186076)\r\n\r\nbefore:\r\n<img
width=\"816\" alt=\"Screenshot 2024-10-17 at 16 25
47\"\r\nsrc=\"https://github.com/user-attachments/assets/3b14c80b-deef-4d8d-9d5b-e118619e31cb\">\r\n\r\n\r\nafter:\r\n<img
width=\"814\" alt=\"Screenshot 2024-10-17 at 16 25
56\"\r\nsrc=\"https://github.com/user-attachments/assets/45788530-aeb6-4851-ac1e-c53efcd73068\">\r\n\r\n##
Release note\r\nNewly and default configured Lens line charts are now
interpolated by\r\ndefault with a straight Linear
interpolation.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Marta Bondyra
<[email protected]>","sha":"5fe8aad89d59726060ba860d2f0692ddb3ca5ffa"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196184","number":196184,"mergeCommit":{"message":"[Lens]
fit line charts by default (#196184)\n\n## Summary\r\n\r\nEvery line
charts are now interpolated by default with a
linear\r\ninterpolation.\r\n\r\nSolves the second task
of\r\n[#186076](https://github.com/elastic/kibana/issues/186076)\r\n\r\nfix
[#186076](https://github.com/elastic/kibana/issues/186076)\r\n\r\nbefore:\r\n<img
width=\"816\" alt=\"Screenshot 2024-10-17 at 16 25
47\"\r\nsrc=\"https://github.com/user-attachments/assets/3b14c80b-deef-4d8d-9d5b-e118619e31cb\">\r\n\r\n\r\nafter:\r\n<img
width=\"814\" alt=\"Screenshot 2024-10-17 at 16 25
56\"\r\nsrc=\"https://github.com/user-attachments/assets/45788530-aeb6-4851-ac1e-c53efcd73068\">\r\n\r\n##
Release note\r\nNewly and default configured Lens line charts are now
interpolated by\r\ndefault with a straight Linear
interpolation.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Marta Bondyra
<[email protected]>","sha":"5fe8aad89d59726060ba860d2f0692ddb3ca5ffa"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marco Vettorello <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbondyra mbondyra mbondyra approved these changes

@kpatticha kpatticha kpatticha approved these changes

Assignees
No one assigned
Labels
apm:review backport:version Backport to applied version labels enhancement New value added to drive a business result Feature:Lens release_note:enhancement v8.17.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Lens] Improve chart visual defaults when there are missing time intervals
6 participants
@markov00 @elasticmachine @kibanamachine @kpatticha @mbondyra @smith