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
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,19 @@
</div>
</div>

<div class="visEditorSidebar__formRow" ng-show="vis.type.type === 'histogram'">
<label
class="visEditorSidebar__formLabel"
for="showLabels"
i18n-id="kbnVislibVisTypes.editors.pointSeries.showLabels"
i18n-default-message="Show values on chart"
></label>
<div class="visEditorSidebar__formControl">
<input class="kuiCheckBox" id="showLabels" type="checkbox" ng-model="editorState.params.labels.show">
</div>
</div>

<vislib-grid></vislib-grid>
</div>

</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ export default function PointSeriesVisType(Private) {
legendPosition: 'right',
times: [],
addTimeMarker: false,
labels: {
show: false,
}
},
},
editorConfig: {
Expand Down
2 changes: 2 additions & 0 deletions src/legacy/ui/public/vislib/_index.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import './variables';

@import './lib/index';

@import './visualizations/point_series/index';
10 changes: 9 additions & 1 deletion src/legacy/ui/public/vislib/lib/axis/axis_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,13 @@ const defaults = {
title: {
text: '',
elSelector: '.visAxis__column--{pos} .axis-div',
}
},
padForLabels: 0,
};

const padForLabelsX = 40;
const padForLabelsY = 15;

const categoryDefaults = {
type: 'category',
position: 'bottom',
Expand Down Expand Up @@ -159,6 +163,10 @@ export class AxisConfig {
this._values.scale.inverted = _.get(axisConfigArgs, 'scale.inverted', true);
}

if (chartConfig.get('labels.show', false) && !isCategoryAxis) {
this._values.padForLabels = isHorizontal ? padForLabelsX : padForLabelsY;
}

let offset;
let stacked = true;
switch (this.get('scale.mode')) {
Expand Down
17 changes: 16 additions & 1 deletion src/legacy/ui/public/vislib/lib/axis/axis_scale.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,21 @@ export class AxisScale {
return [Math.min(0, min), Math.max(0, max)];
}

getDomain(length) {
const domain = this.getExtents();
const pad = this.axisConfig.get('padForLabels');
if (pad > 0 && this.canApplyNice()) {
const domainLength = domain[1] - domain[0];
const valuePerPixel = domainLength / length;
const padValue = valuePerPixel * pad;
if (domain[0] < 0) {
domain[0] -= padValue;
}
domain[1] += padValue;
}
return domain;
}

getRange(length) {
if (this.axisConfig.isHorizontal()) {
return !this.axisConfig.get('scale.inverted') ? [0, length] : [length, 0];
Expand Down Expand Up @@ -212,7 +227,7 @@ export class AxisScale {
getScale(length) {
const config = this.axisConfig;
const scale = this.getD3Scale(config.getScaleType());
const domain = this.getExtents();
const domain = this.getDomain(length);
const range = this.getRange(length);
const padding = config.get('style.rangePadding');
const outerPadding = config.get('style.rangeOuterPadding');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import './labels';
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
$visColumnChartBarLabelDarkColor: #000; // EUI doesn't yet have a variable for fully black in all themes;
$visColumnChartBarLabelLightColor: $euiColorGhost;

.visColumnChart__barLabel {
font-size: 8pt;
pointer-events: none;
}

.visColumnChart__barLabel--stack {
dominant-baseline: central;
text-anchor: middle;
}

.visColumnChart__bar-label--dark {
fill: $visColumnChartBarLabelDarkColor;
}

.visColumnChart__bar-label--light {
fill: $visColumnChartBarLabelLightColor;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@
*/

import _ from 'lodash';
import d3 from 'd3';
import { isColorDark } from '@elastic/eui/lib/services';
import { PointSeries } from './_point_series';


const defaults = {
mode: 'normal',
showTooltip: true,
color: undefined,
fillColor: undefined,
showLabel: true,
};

/**
Expand Down Expand Up @@ -58,6 +60,7 @@ export class ColumnChart extends PointSeries {
constructor(handler, chartEl, chartData, seriesConfigArgs) {
super(handler, chartEl, chartData, seriesConfigArgs);
this.seriesConfig = _.defaults(seriesConfigArgs || {}, defaults);
this.labelOptions = _.defaults(handler.visConfig.get('labels', {}), defaults.showLabel);
}

addBars(svg, data) {
Expand Down Expand Up @@ -124,8 +127,10 @@ export class ColumnChart extends PointSeries {
const yScale = this.getValueAxis().getScale();
const isHorizontal = this.getCategoryAxis().axisConfig.isHorizontal();
const isTimeScale = this.getCategoryAxis().axisConfig.isTimeDomain();
const isLabels = this.labelOptions.show;
const yMin = yScale.domain()[0];
const gutterSpacingPercentage = 0.15;
const chartData = this.chartData;
const groupCount = this.getGroupedCount();
const groupNum = this.getGroupedNum(this.chartData);
let barWidth;
Expand Down Expand Up @@ -154,6 +159,22 @@ export class ColumnChart extends PointSeries {
return yScale(d.y0 + d.y);
}

function labelX(d, i) {
return x(d, i) + widthFunc(d, i) / 2;
}

function labelY(d) {
return y(d) + heightFunc(d) / 2;
}

function labelDisplay(d, i) {
if (isHorizontal && this.getBBox().width > widthFunc(d, i)) return 'none';
if (!isHorizontal && this.getBBox().width > heightFunc(d)) return 'none';
if (isHorizontal && this.getBBox().height > heightFunc(d)) return 'none';
if (!isHorizontal && this.getBBox().height > widthFunc(d, i)) return 'none';
return 'block';
}

function widthFunc(d, i) {
if (isTimeScale) {
return datumWidth(barWidth, d, bars.data()[i + 1], xScale, gutterWidth, groupCount);
Expand All @@ -167,17 +188,48 @@ export class ColumnChart extends PointSeries {
if (d.y0 === 0 && yMin > 0) {
return yScale(yMin) - yScale(d.y);
}

return Math.abs(yScale(d.y0) - yScale(d.y0 + d.y));
}

function formatValue(d) {
return chartData.yAxisFormatter(d.y);
}

// update
bars
.attr('x', isHorizontal ? x : y)
.attr('width', isHorizontal ? widthFunc : heightFunc)
.attr('y', isHorizontal ? y : x)
.attr('height', isHorizontal ? heightFunc : widthFunc);

const layer = d3.select(bars[0].parentNode);
const barLabels = layer.selectAll('text').data(chartData.values.filter(function (d) {
return !_.isNull(d.y);
}));

if (isLabels) {
const colorFunc = this.handler.data.getColorFunc();
const d3Color = d3.rgb(colorFunc(chartData.label));
let labelClass;
if (isColorDark(d3Color.r, d3Color.g, d3Color.b)) {
labelClass = 'visColumnChart__bar-label--light';
} else {
labelClass = 'visColumnChart__bar-label--dark';
}

barLabels
.enter()
.append('text')
.text(formatValue)
.attr('class', `visColumnChart__barLabel visColumnChart__barLabel--stack ${labelClass}`)
.attr('x', isHorizontal ? labelX : labelY)
.attr('y', isHorizontal ? labelY : labelX)

// display must apply last, because labelDisplay decision it based
// on text bounding box which depends on actual applied style.
.attr('display', labelDisplay);
}

return bars;
}

Expand All @@ -191,12 +243,14 @@ export class ColumnChart extends PointSeries {
addGroupedBars(bars) {
const xScale = this.getCategoryAxis().getScale();
const yScale = this.getValueAxis().getScale();
const chartData = this.chartData;
const groupCount = this.getGroupedCount();
const groupNum = this.getGroupedNum(this.chartData);
const gutterSpacingPercentage = 0.15;
const isTimeScale = this.getCategoryAxis().axisConfig.isTimeDomain();
const isHorizontal = this.getCategoryAxis().axisConfig.isHorizontal();
const isLogScale = this.getValueAxis().axisConfig.isLogScale();
const isLabels = this.labelOptions.show;
let barWidth;
let gutterWidth;

Expand All @@ -220,10 +274,29 @@ export class ColumnChart extends PointSeries {
if ((isHorizontal && d.y < 0) || (!isHorizontal && d.y > 0)) {
return yScale(0);
}

return yScale(d.y);
}

function labelX(d, i) {
return x(d, i) + widthFunc(d, i) / 2;
}

function labelY(d) {
if (isHorizontal) {
return d.y >= 0 ? y(d) - 4 : y(d) + heightFunc(d) + this.getBBox().height;
}
return d.y >= 0 ? y(d) + heightFunc(d) + 4 : y(d) - this.getBBox().width - 4;
}

function labelDisplay(d, i) {
if (isHorizontal && this.getBBox().width > widthFunc(d, i)) {
return 'none';
}
if (!isHorizontal && this.getBBox().height > widthFunc(d)) {
return 'none';
}
return 'block';
}
Copy link
Member

Choose a reason for hiding this comment

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

Specially with small bars or with bigger values, there is the issue that only lower bars or lower values are displayed. like in this screenshot
Screenshot 2019-06-18 at 12 14 36
What do you think if we can add a configurable parameter that force the display of all the labels, also if they are bigger than the bar width?
As for the stacked case, just be aware of the color of the text for non stacked bars because the text can overlap with a bar with the same color.
Maybe we can just use the black/white color also for non stacked bars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid that if we allow to label to spread out, it may become a mess. Even, with a configuration (which is a user responsiblity), still at configuration time, it may remained nice, but later with specific set of data, the graph become ugly.

function widthFunc(d, i) {
if (isTimeScale) {
return datumWidth(barWidth, d, bars.data()[i + 1], xScale, gutterWidth, groupCount);
Expand All @@ -236,13 +309,44 @@ export class ColumnChart extends PointSeries {
return Math.abs(yScale(baseValue) - yScale(d.y));
}

function formatValue(d) {
return chartData.yAxisFormatter(d.y);
}

// update
bars
.attr('x', isHorizontal ? x : y)
.attr('width', isHorizontal ? widthFunc : heightFunc)
.attr('y', isHorizontal ? y : x)
.attr('height', isHorizontal ? heightFunc : widthFunc);

const layer = d3.select(bars[0].parentNode);
const barLabels = layer.selectAll('text').data(chartData.values.filter(function (d) {
return !_.isNull(d.y);
}));

barLabels
.exit()
.remove();

if (isLabels) {
const labelColor = this.handler.data.getColorFunc()(chartData.label);

barLabels
.enter()
.append('text')
.text(formatValue)
.attr('class', 'visColumnChart__barLabel')
.attr('x', isHorizontal ? labelX : labelY)
.attr('y', isHorizontal ? labelY : labelX)
.attr('dominant-baseline', isHorizontal ? 'auto' : 'central')
.attr('text-anchor', isHorizontal ? 'middle' : 'start')
.attr('fill', labelColor)

// display must apply last, because labelDisplay decision it based
// on text bounding box which depends on actual applied style.
.attr('display', labelDisplay);
mmeshi marked this conversation as resolved.
Show resolved Hide resolved
}
return bars;
}

Expand Down