Skip to content
Snippets Groups Projects
Unverified Commit 95e4a346 authored by Mahatthana (Kelvin) Nomsawadi's avatar Mahatthana (Kelvin) Nomsawadi Committed by GitHub
Browse files

Static viz polish fixes improvements (#25361)

* Handle small number format

* Hide bar chart values on small bars

This matches the app viz behavior
parent 566a2688
No related branches found
No related tags found
No related merge requests found
...@@ -68,8 +68,8 @@ export default function Values({ ...@@ -68,8 +68,8 @@ export default function Values({
if (containBars && xScale.bandwidth) { if (containBars && xScale.bandwidth) {
const innerBarScaleDomain = multipleSeries const innerBarScaleDomain = multipleSeries
.filter(series => series.type === "bar") .filter(series => series.type === "bar")
.map((barSerie, index) => { .map((barSeries, index) => {
barSeriesIndexMap.set(barSerie, index); barSeriesIndexMap.set(barSeries, index);
return index; return index;
}); });
innerBarScale = scaleBand({ innerBarScale = scaleBand({
...@@ -79,8 +79,15 @@ export default function Values({ ...@@ -79,8 +79,15 @@ export default function Values({
} }
const multiSeriesValues: MultiSeriesValue[][] = multipleSeries.map(series => { const multiSeriesValues: MultiSeriesValue[][] = multipleSeries.map(series => {
const singleSerieValues = getValues(series, areStacked); const singleSeriesValues =
return singleSerieValues.map(value => { series.type === "bar"
? fixSmallBarChartValues(
getValues(series, areStacked),
innerBarScale?.domain().length ?? 0,
)
: getValues(series, areStacked);
return singleSeriesValues.map(value => {
return { return {
...value, ...value,
series, series,
...@@ -92,6 +99,21 @@ export default function Values({ ...@@ -92,6 +99,21 @@ export default function Values({
}); });
}); });
function fixSmallBarChartValues(
singleSeriesValues: Value[],
numberOfBarSeries: number,
) {
const barWidth = innerBarScale?.bandwidth() ?? Infinity;
const MIN_BAR_WIDTH = 20;
// Use the same logic as in https://github.com/metabase/metabase/blob/cb51e574de31c7d4485a9dfbef3261f67c0b7495/frontend/src/metabase/visualizations/lib/chart_values.js#L138
if (numberOfBarSeries > 1 && barWidth < MIN_BAR_WIDTH) {
singleSeriesValues.forEach(value => {
value.hidden = true;
});
}
return singleSeriesValues;
}
function getBarXOffset(series: HydratedSeries) { function getBarXOffset(series: HydratedSeries) {
if (containBars && innerBarScale) { if (containBars && innerBarScale) {
// Use the same logic when rendering <BarSeries />, as bar charts can display values in groups. // Use the same logic when rendering <BarSeries />, as bar charts can display values in groups.
...@@ -112,13 +134,15 @@ export default function Values({ ...@@ -112,13 +134,15 @@ export default function Values({
return ( return (
<> <>
{verticalOverlappingFreeValues.map((singleSerieValues, seriesIndex) => { {verticalOverlappingFreeValues.map((singleSeriesValues, seriesIndex) => {
const compact = getCompact(singleSerieValues.map(value => value.datum)); const compact = getCompact(
singleSeriesValues.map(value => value.datum),
);
return fixHorizontalOverlappingValues( return fixHorizontalOverlappingValues(
seriesIndex, seriesIndex,
compact, compact,
singleSerieValues, singleSeriesValues,
).map((value, index) => { ).map((value, index) => {
if (value.hidden) { if (value.hidden) {
return null; return null;
...@@ -180,7 +204,7 @@ export default function Values({ ...@@ -180,7 +204,7 @@ export default function Values({
function fixHorizontalOverlappingValues( function fixHorizontalOverlappingValues(
seriesIndex: number, seriesIndex: number,
compact: boolean, compact: boolean,
singleSerieValues: MultiSeriesValue[], singleSeriesValues: MultiSeriesValue[],
) { ) {
const valueStep = getValueStep( const valueStep = getValueStep(
multipleSeries, multipleSeries,
...@@ -190,7 +214,7 @@ export default function Values({ ...@@ -190,7 +214,7 @@ export default function Values({
innerWidth, innerWidth,
); );
return singleSerieValues.filter((_, index) => index % valueStep === 0); return singleSeriesValues.filter((_, index) => index % valueStep === 0);
} }
} }
......
import { merge } from "icepick";
export type NumberFormatOptions = { export type NumberFormatOptions = {
number_style?: "currency" | "decimal" | "scientific" | "percentage"; number_style?: "currency" | "decimal" | "scientific" | "percentage";
currency?: string; currency?: string;
...@@ -32,7 +34,7 @@ export const formatNumber = (number: number, options?: NumberFormatOptions) => { ...@@ -32,7 +34,7 @@ export const formatNumber = (number: number, options?: NumberFormatOptions) => {
prefix, prefix,
suffix, suffix,
compact, compact,
} = { ...DEFAULT_OPTIONS, ...options }; } = handleSmallNumberFormat(number, { ...DEFAULT_OPTIONS, ...options });
function createFormat(compact?: boolean) { function createFormat(compact?: boolean) {
if (compact) { if (compact) {
...@@ -43,6 +45,7 @@ export const formatNumber = (number: number, options?: NumberFormatOptions) => { ...@@ -43,6 +45,7 @@ export const formatNumber = (number: number, options?: NumberFormatOptions) => {
currency: currency, currency: currency,
currencyDisplay: currency_style, currencyDisplay: currency_style,
useGrouping: true, useGrouping: true,
maximumFractionDigits: decimals != null ? decimals : 2,
}); });
} }
...@@ -67,5 +70,26 @@ export const formatNumber = (number: number, options?: NumberFormatOptions) => { ...@@ -67,5 +70,26 @@ export const formatNumber = (number: number, options?: NumberFormatOptions) => {
return `${prefix}${formattedNumber}${suffix}`; return `${prefix}${formattedNumber}${suffix}`;
}; };
// Simple hack to handle small decimal numbers (0-1)
function handleSmallNumberFormat<T>(value: number, options: T): T {
const hasAtLeastThreeDecimalPoints = Math.abs(value) < 0.01;
if (hasAtLeastThreeDecimalPoints && Math.abs(value) > 0) {
options = maybeMerge(options, {
compact: true,
decimals: 4,
});
}
return options;
}
const maybeMerge = <T, S1>(collection: T, object: S1) => {
if (collection == null) {
return collection;
}
return merge(collection, object);
};
export const formatPercent = (percent: number) => export const formatPercent = (percent: number) =>
`${(100 * percent).toFixed(2)} %`; `${(100 * percent).toFixed(2)} %`;
...@@ -99,6 +99,13 @@ describe("formatNumber", () => { ...@@ -99,6 +99,13 @@ describe("formatNumber", () => {
expect(text).toEqual("10000.11"); expect(text).toEqual("10000.11");
}); });
it("should format small number", () => {
expect(formatNumber(0.00196)).toEqual("0.002");
expect(formatNumber(0.00201)).toEqual("0.002");
expect(formatNumber(-0.00119)).toEqual("-0.0012");
expect(formatNumber(-0.00191)).toEqual("-0.0019");
});
}); });
describe("formatPercent", () => { describe("formatPercent", () => {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment