-
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
Hide gauge labels when value is hidden #34171
Conversation
Pinging @elastic/kibana-app |
}); | ||
} | ||
|
||
gauges |
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.
This (value label) has just been moved above the other two labels, so we know whether or not that's shown.
💔 Build Failed |
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 actually modified the behavior slightly. Since the label above the value is usually shown for the labels of "Split Group" it makes sense to have that even if the value label doesn't fit anymore. That was also what the test failed on. So I decided to only hide the below value label in case the value cannot be shown anymore. |
💚 Build Succeeded |
@timroes I've also seen something else that can prevent us from hiding labels too early
For each text, the textTooLong computes as the above mean that we will display the text only of the text width is less than the radius and not less than the diameter, that I think it's what we want. and this is specifying the following: (btw I'm not sure the totalWidth is the right value to use, I mean we need that check against the diameter - 2 times the width of the gauge path.
|
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. Tested locally on chrome, safari and firefox and works fine.
💔 Build Failed |
💔 Build Failed |
Jenkins, test this - CI failure |
💚 Build Succeeded |
* Hide gauge labels when value is hidden * Only hide subtext when value is hidden * Use better free space calculation * Fix tests
* Hide gauge labels when value is hidden * Only hide subtext when value is hidden * Use better free space calculation * Fix tests
Summary
Fixes #33110.
The behavior described in this issue was actually present since forever, it was just triggered now most likely for Bhavya, because we changed the font in 7.0, thus leading to different widths of elements and triggering that. I've actually seen the same behavior on other machines with the same sample data already in 6.7.
Until now the Gauge chart checked for each of the possible three labels (above value, value, below value) independently if they fit within the gauge and hide them if not. I think that behavior is very weird, since it leaves you with charts, like Bhavya reported, where you only have the "below value" label which kind of looks like the chart is just broken and the value is missing.
I changed the behavior now in a way, that each of the above/below labels can be hidden individually if they don't fit, but as soon as the value label is hidden, those the below label will always be hidden too, so it never stay there as long as the value label are hidden. The above value actually will still show independently, since it's used to show the label e.g. for a chart split that makes sense to also show without the value.
You can see in the below screencast, that the "average spend" vanishes as soon as it doesn't have space anymore, and the "per order" even though it would have space vanishes together with the value itself.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] Unit or functional tests were updated or added to match the most common scenarios[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
[ ] This was checked for breaking API changes and was labeled appropriately