Skip to content

Commit

Permalink
👌 Build on feedback comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dej611 committed Aug 24, 2020
1 parent b687378 commit a11c646
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 35 deletions.
101 changes: 73 additions & 28 deletions x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
EuiTitle,
} from '@elastic/eui';
import { EuiFieldNumber } from '@elastic/eui';
import { EuiFieldNumberProps } from '@elastic/eui';
import {
VisualizationLayerWidgetProps,
VisualizationDimensionEditorProps,
Expand Down Expand Up @@ -267,10 +268,33 @@ export function XyToolbar(props: VisualizationToolbarProps<State>) {
});
};

const shouldEnableValueLabels = state?.layers.some((layer) => layer.seriesType.includes('bar'));
const isBarChartType = state?.layers.some((layer) => layer.seriesType.includes('bar'));
const isHistogram = state?.layers.every((layer) => {
if (!layer.xAccessor) {
return false;
}
const xAxisOperation = frame.datasourceLayers[layer.layerId]?.getOperationForColumnId(
layer.xAccessor
);
return Boolean(
xAxisOperation &&
xAxisOperation.isBucketed &&
xAxisOperation.scale &&
xAxisOperation.scale !== 'ordinal'
);
});
const shouldEnableValueLabels = isBarChartType && !isHistogram;
const showValueLabels = Boolean(shouldEnableValueLabels && state?.displayValues?.showLabels);
const valueLabelsPositioning = state?.displayValues?.position || 'inside';

const valueLabelsDisabledMessage = isHistogram
? i18n.translate('xpack.lens.xyChart.valueLabelsDisabledHelpText.histogram', {
defaultMessage: 'This setting only applies to non-time related bar charts.',
})
: i18n.translate('xpack.lens.xyChart.valueLabelsDisabledHelpText.nonBarTypes', {
defaultMessage: 'This setting only applies to bar type of charts.',
});

const legendMode =
state?.legend.isVisible && !state?.legend.showSingleSeries
? 'auto'
Expand Down Expand Up @@ -408,16 +432,21 @@ export function XyToolbar(props: VisualizationToolbarProps<State>) {
defaultMessage: 'Value Labels Display',
})}
>
<EuiSwitch
data-test-subj="lnsshowShowValueLabelsSwitch"
showLabel={false}
label={i18n.translate('xpack.lens.xyChart.valueLabels.showlabels', {
defaultMessage: 'Value Labels Display',
})}
onChange={({ target }) => onValueDisplayVisibilitySettingsChange(target.checked)}
checked={showValueLabels}
disabled={!shouldEnableValueLabels}
/>
<EuiToolTip
anchorClassName="eui-displayBlock"
content={!shouldEnableValueLabels && valueLabelsDisabledMessage}
>
<EuiSwitch
data-test-subj="lnsshowShowValueLabelsSwitch"
showLabel={false}
label={i18n.translate('xpack.lens.xyChart.valueLabels.showlabels', {
defaultMessage: 'Value Labels Display',
})}
onChange={({ target }) => onValueDisplayVisibilitySettingsChange(target.checked)}
checked={showValueLabels}
disabled={!shouldEnableValueLabels}
/>
</EuiToolTip>
</EuiFormRow>
<EuiFormRow
display="columnCompressed"
Expand All @@ -426,20 +455,7 @@ export function XyToolbar(props: VisualizationToolbarProps<State>) {
defaultMessage: 'Value Labels size',
})}
>
<EuiFieldNumber
value={state?.displayValues?.fontSize || 10}
onChange={({ target }) =>
setState({
...state,
displayValues: {
...state.displayValues,
showLabels: true,
fontSize: Number(target.value),
},
})
}
disabled={!shouldEnableValueLabels}
/>
<ValueLabelFontSizeInput {...props} disabled={!shouldEnableValueLabels} />
</EuiFormRow>
<EuiFormRow
display="columnCompressed"
Expand Down Expand Up @@ -598,9 +614,6 @@ const idPrefix = htmlIdGenerator()();

export function DimensionEditor(props: VisualizationDimensionEditorProps<State>) {
const { state, setState, layerId, accessor } = props;
// const horizontalOnly = isHorizontalChart(state.layers);
// const chartHasMoreThanOneSeries =
// state.layers.length > 1 || state.layers.some(({ splitAccessor }) => splitAccessor);
const shouldEnableValueLabels = state?.layers.some((layer) => layer.seriesType.includes('bar'));

const index = state.layers.findIndex((l) => l.layerId === layerId);
Expand Down Expand Up @@ -719,6 +732,38 @@ const tooltipContent = {
}),
};

const ValueLabelFontSizeInput = ({
state,
setState,
disabled,
}: VisualizationToolbarProps<State> & { disabled: boolean }) => {
const [fontSize, setFontSize] = useState(state?.displayValues?.fontSize || 10);

const updateFontSizeInState = React.useMemo(
() =>
debounce((value: string) => {
setState({
...state,
displayValues: {
...state.displayValues,
showLabels: true,
fontSize: Number(value),
},
});
}, 256),
[state, setState]
);

const handleFontSize: EuiFieldNumberProps['onChange'] = ({ target }) => {
setFontSize(Number(target.value));
if (!Number.isNaN(target.value) || target.value !== '') {
updateFontSizeInState(target.value);
}
};

return <EuiFieldNumber value={fontSize} onChange={handleFontSize} disabled={disabled} />;
};

const ColorPicker = ({
state,
setState,
Expand Down
32 changes: 25 additions & 7 deletions x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,9 @@ export function XYChart({
);
};

const VALUE_LABELS_ARE_INSIDE = displayValues?.position === 'inside';
const VALUE_LABELS_FONTSIZE = displayValues.fontSize || 10;

return (
<Chart>
<Settings
Expand All @@ -339,19 +342,34 @@ export function XYChart({
theme={{
...chartTheme,
...{
chartMargins: {
...chartTheme.chartMargins,
...(!VALUE_LABELS_ARE_INSIDE && {
top: shouldRotate ? chartTheme.chartMargins?.top : VALUE_LABELS_FONTSIZE,
right: shouldRotate ? 1.5 * VALUE_LABELS_FONTSIZE : chartTheme.chartMargins?.right,
}),
},
barSeriesStyle: {
...chartTheme.barSeriesStyle,
displayValue: {
...chartTheme.axes?.tickLabelStyle,
fontSize: displayValues.fontSize,
fill: displayValues.position === 'inside' ? 'white' : 'black',
offsetX: shouldRotate ? (displayValues.position === 'inside' ? +5 : -15) : 0,
offsetY: shouldRotate ? 0 : displayValues.position === 'inside' ? -5 : +10,
fontSize: VALUE_LABELS_FONTSIZE,
fill: VALUE_LABELS_ARE_INSIDE ? 'white' : 'black',
offsetX: shouldRotate
? VALUE_LABELS_ARE_INSIDE
? VALUE_LABELS_FONTSIZE
: -(2 * VALUE_LABELS_FONTSIZE)
: 0,
offsetY: shouldRotate
? 0
: VALUE_LABELS_ARE_INSIDE
? -VALUE_LABELS_FONTSIZE
: VALUE_LABELS_FONTSIZE,
},
},
},
}}
baseTheme={chartBaseTheme}
// baseTheme={chartBaseTheme}
tooltip={{
headerFormatter: (d) => xAxisFormatter.convert(d.value),
}}
Expand Down Expand Up @@ -562,8 +580,8 @@ export function XYChart({
valueFormatter: (d: any): string => associatedAxes?.formatter.convert(d) || '',
showValueLabel: shouldShowValueLabel,
isAlternatingValueLabel: false,
isValueContainedInElement: true,
hideClippedValue: true,
isValueContainedInElement: VALUE_LABELS_ARE_INSIDE,
hideClippedValue: VALUE_LABELS_ARE_INSIDE,
},
};
return <BarSeries key={index} {...seriesProps} {...valueLabelsSettings} />;
Expand Down

0 comments on commit a11c646

Please sign in to comment.