-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Add value labels to Heatmap #106406
Conversation
@elasticmachine merge upstream |
merge conflict between base and head |
Waiting on #111991 to be merged to unblock this PR. |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
merge conflict between base and head |
@elasticmachine merge upstream |
Pinging @elastic/kibana-vis-editors (Team:VisEditors) |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/categorization_field_examples·ts.apis Machine Learning jobs Categorization example endpoint - invalid, too many tokens.Standard Out
Stack Trace
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM! I like this addition and personally I am fine with the selected font sizes, I couldn't find any case that these font sizes are not ideal. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Tested on FF and works as expected!
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
To update your PR or re-run it, just comment with: |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
3 similar comments
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Summary
Fixes #105410
Creates the menu to control the value labels visibility option in the Heatmap.
ValueLabel
component as shared and reuse in both Heatmap and XYFunctionalcurrently blocked by [Heatmap] Add value label visibility/drawing state in debug data elastic-charts#1436Notes:
Value labels visibily dynamically changes based on the cell space.
I've set the min/max font size to 8/18 (different from XY) because, given the heatmap context I thought it may make sense for smaller text. But it can be easily changed.
When labels won't fit in the cell, even scaling down to the minFontSize, then it will be hidden:
Checklist
Delete any items that are not applicable to this PR.
Risk Matrix
Delete this section if it is not applicable to this PR.
Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.
When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:
For maintainers