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

Show values of bars inside bar charts #36511

Merged
merged 11 commits into from
Jun 24, 2019
Merged

Conversation

mmeshi
Copy link
Contributor

@mmeshi mmeshi commented May 11, 2019

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.

image

Screen Shot 2019-06-17 at 0 14 44

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

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?

@mmeshi
Copy link
Contributor Author

mmeshi commented May 11, 2019

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

@timroes
Copy link
Contributor

timroes commented May 22, 2019

Jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rayafratkina rayafratkina changed the title add labels to bat chart add labels to bar chart May 22, 2019
@timroes
Copy link
Contributor

timroes commented May 24, 2019

Hi, could you please merge current master into your branch again. There were some CI failures that should be fixed by this.

@mmeshi
Copy link
Contributor Author

mmeshi commented May 24, 2019

This is what i did when i force-push before couple of minutes ago. is it still required?

@timroes
Copy link
Contributor

timroes commented May 24, 2019

Jenkins, test this - sorry I have overlooked the force push, after the last CI failure.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticcla
Copy link

Hi @mmeshi, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@mmeshi
Copy link
Contributor Author

mmeshi commented May 29, 2019

Hi @mmeshi, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

Done.

@mmeshi mmeshi closed this May 29, 2019
@mmeshi mmeshi reopened this May 29, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mmeshi mmeshi force-pushed the barChartLabels branch 2 times, most recently from 2058be4 to 929bd1d Compare June 7, 2019 16:07
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@markov00 markov00 left a 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
Screenshot 2019-06-10 at 18 30 25

  • the Show Labels settings label doesn't imply a specific meaning. Maybe it's better to have something like: Show bar values on chart or Show 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

Copy link
Contributor

@cchaos cchaos left a 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

Copy link
Member

@markov00 markov00 left a 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. ❤️

@markov00 markov00 added Feature:XYAxis XY-Axis charts (bar, area, line) release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jun 24, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

Copy link
Contributor

@timroes timroes left a 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.

@timroes timroes merged commit 0e20785 into elastic:master Jun 24, 2019
@timroes timroes changed the title add labels to bar chart Show values of bars inside bar charts Jun 24, 2019
timroes pushed a commit to timroes/kibana that referenced this pull request Jun 24, 2019
* 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
@timroes
Copy link
Contributor

timroes commented Jun 24, 2019

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,
Tim

@mmeshi mmeshi deleted the barChartLabels branch June 24, 2019 15:36
@mmeshi
Copy link
Contributor Author

mmeshi commented Jun 24, 2019

Many thanks for @timroes and other involved. I’m gonna to celebrate my first PR 🎉🎂

@elasticmachine
Copy link
Contributor

💔 Build Failed

timroes pushed a commit that referenced this pull request Jun 24, 2019
* 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
@milandrop
Copy link

Hi Guys,

This future looks really interesting. Does anybody knows when it will been add to Kibana? I cannot see it yet

@timroes
Copy link
Contributor

timroes commented Jul 8, 2019

Hi, as you can see from the labels in this PR, the feature has been merged for the 7.3.0 release.

@markov00
Copy link
Member

markov00 commented Aug 1, 2019

Released yesterday: https://www.elastic.co/blog/kibana-7-3-0-released

@AlexM566
Copy link

Hi,

Is this feature available in Lens, I am not sure if you can do it..

Thx,
Alex

@nickofthyme
Copy link
Contributor

nickofthyme commented Oct 28, 2021

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 Filters or Top Values functions. This is true for 7.15.1, I'm not sure about support for this on older versions or when this feature was added.

Image 2021-10-28 at 9 47 09 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:XYAxis XY-Axis charts (bar, area, line) release highlight release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.3.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show bar value above each bar