-
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
Show values of bars inside bar charts #36511
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
yes |
Jenkins, test this |
💔 Build Failed |
Hi, could you please merge current master into your branch again. There were some CI failures that should be fixed by this. |
This is what i did when i force-push before couple of minutes ago. is it still required? |
Jenkins, test this - sorry I have overlooked the force push, after the last CI failure. |
💔 Build Failed |
💚 Build Succeeded |
2058be4
to
929bd1d
Compare
💔 Build Failed |
💚 Build Succeeded |
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.
Hey @mmeshi sorry for the delay on reviewing this.
I've took a first check on the functionality and this is what I've found:
- I've tested with few color combination but seems that in some situation, adding the inverted color doesn't work right. I also think it's better to avoid using inverted colors on text and it's maybe better to add black or white color depending on the darkness of the bar color. For that you can use the utility function in EUI https://elastic.github.io/eui/#/utilities/is-color-dark:
import { isColorDark } from '@elastic/eui/lib/services';
const fontColor = isColorDark(...color) ? 'white' : 'black' ;
On this case for example the green and the blue/violet text on bars are quite indistinguishable from the bar
-
the
Show Labels
settings label doesn't imply a specific meaning. Maybe it's better to have something like:Show bar values on chart
orShow values on chart
@emmacunningham can you provide us a better "label" for this setting? -
as this function is only available for bars, it will be better to hide the
Show labels
setting if we are visualising a line or area.
Thanks and sorry again for the long delay
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.
From a SASS perspective, LGTM
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.
Changes LGTM! Thanks for implementing that feature, a lot of users request that. ❤️
Pinging @elastic/kibana-app |
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.
Tested on Chrome Linux, works fine, code LGTM.
* add labels to bar chart * code simplification * clean trailing spaces * for stack mode label color will be w/b * change label message on option panel, and show the option only for histogram type * better way to manipulate axis to make room for labels * fixes for user defined axis, and negative value * fix label position in grouped vertical mode * refactor label color style to css classes * cosmetic changes in label classes
Hi Meir, I just merged the PR and we're now "backporting" it for the 7.3 release. Thanks a lot for your contribution of such a valuable and highly requested feature. We really appreciate that you took the time to work with Marco and Caroline to get everything in a mergable state. Cheer, |
Many thanks for @timroes and other involved. I’m gonna to celebrate my first PR 🎉🎂 |
💔 Build Failed |
* add labels to bar chart * code simplification * clean trailing spaces * for stack mode label color will be w/b * change label message on option panel, and show the option only for histogram type * better way to manipulate axis to make room for labels * fixes for user defined axis, and negative value * fix label position in grouped vertical mode * refactor label color style to css classes * cosmetic changes in label classes
Hi Guys, This future looks really interesting. Does anybody knows when it will been add to Kibana? I cannot see it yet |
Hi, as you can see from the labels in this PR, the feature has been merged for the 7.3.0 release. |
Released yesterday: https://www.elastic.co/blog/kibana-7-3-0-released |
Hi, Is this feature available in Lens, I am not sure if you can do it.. Thx, |
Hey Alex, unfortunately this is not yet widely supported in Lens. We are working through some issues with displaying values which you can track in #82954. Currently, the option to show these values is under the Visual options (i.e. the brush icon ). This option is only enabled for charts with horizontal axis using |
Closes #7116
Complete #3686, #10360
Users can now add labels to bar chart also. (from all types: horizontal and vertical, stacked and normal)
Labels are auto hide if there is not enough space to display.
In stack mode (labels display inside the bar) -
color are varsia the bar's color (brighter or darker)color are black or white depends on the darkness of the bar color.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers