Skip to content

Conversation

@AntoineThebaud
Copy link
Contributor

@AntoineThebaud AntoineThebaud commented Jan 21, 2026

Description

Relates to perses/perses#2123.

Needed for perses/plugins#531.

This PR mainly updates TimeSeriesTooltip to be compatible with a multiple Y axis setup (e.g nearby series computation has to be made using pixel proximity in that case instead of series values, as the orders of magnitude between series can be very different). It also adds a new getFormattedMultipleYAxes utility that plugins relying on ECharts can call in the same way than the existing getFormattedAxis

NB: TimeSeriesTooltip still has to be moved to TimeSeriesChart code as it's not really generic (ref), I really need to work on this 😅

Screenshots

See screenshots of perses/plugins#531.

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.
  • E2E tests are stable and unlikely to be flaky.
    See e2e docs for more details. Common issues include:
    • Is the data inconsistent? You need to mock API requests.
    • Does the time change? You need to use consistent time values or mock time utilities.
    • Does it have loading states? You need to wait for loading to complete.

@AntoineThebaud AntoineThebaud marked this pull request as ready for review January 21, 2026 16:33
@AntoineThebaud AntoineThebaud requested a review from a team as a code owner January 21, 2026 16:33
Comment on lines 90 to 155
// Calculate cumulative offsets based on actual formatted label widths
let cumulativeOffset = 0;

// Additional Y axes (right side) for each unique format
additionalFormats.forEach((format, index) => {
// For subsequent axes, add the width of the previous axis's labels
if (index > 0 && maxValues) {
const prevMaxValue = maxValues[index - 1] ?? 1000;
cumulativeOffset += estimateLabelWidth(additionalFormats[index - 1], prevMaxValue);
}

const rightAxisConfig: YAXisComponentOption = {
type: 'value',
position: 'right',
// Dynamic offset based on cumulative width of preceding axis labels
offset: cumulativeOffset,
boundaryGap: [0, '10%'],
axisLabel: {
formatter: (value: number): string => {
return formatValue(value, format);
},
},
splitLine: {
show: false, // Hide grid lines for right-side axes to reduce visual noise
},
show: baseAxis?.show,
};
axes.push(rightAxisConfig);
});
Copy link
Contributor

@jgbernalp jgbernalp Jan 23, 2026

Choose a reason for hiding this comment

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

I think a "simpler" approach will be to calculate the width. This way the text can be truncated to the next line to avoid cutting it.

Suggested change
// Calculate cumulative offsets based on actual formatted label widths
let cumulativeOffset = 0;
// Additional Y axes (right side) for each unique format
additionalFormats.forEach((format, index) => {
// For subsequent axes, add the width of the previous axis's labels
if (index > 0 && maxValues) {
const prevMaxValue = maxValues[index - 1] ?? 1000;
cumulativeOffset += estimateLabelWidth(additionalFormats[index - 1], prevMaxValue);
}
const rightAxisConfig: YAXisComponentOption = {
type: 'value',
position: 'right',
// Dynamic offset based on cumulative width of preceding axis labels
offset: cumulativeOffset,
boundaryGap: [0, '10%'],
axisLabel: {
formatter: (value: number): string => {
return formatValue(value, format);
},
},
splitLine: {
show: false, // Hide grid lines for right-side axes to reduce visual noise
},
show: baseAxis?.show,
};
axes.push(rightAxisConfig);
});
// Calculate cumulative offsets based on actual formatted label widths
let width = 0;
let cumulativeWidth = 0;
// Additional Y axes (right side) for each unique format
additionalFormats.forEach((format, index) => {
// For subsequent axes, add the width of the previous axis's labels
if (maxValues) {
width = estimateLabelWidth(format, maxValues[index] ?? 1000);
}
const rightAxisConfig: YAXisComponentOption = {
type: 'value',
position: 'right',
// Dynamic offset based on cumulative width of preceding axis labels
offset: cumulativeWidth,
boundaryGap: [0, '10%'],
axisLabel: {
formatter: (value: number): string => {
return formatValue(value, format);
},
width: width,
overflow: 'breakAll',
},
splitLine: {
show: false, // Hide grid lines for right-side axes to reduce visual noise
},
show: baseAxis?.show,
};
axes.push(rightAxisConfig);
cumulativeWidth += width + AXIS_LABEL_PADDING;
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the fixed witdth + line break behavior is not satisfactory imho, considering that

  1. the break can happen anywhere in the string, which can make it hard to understand
image
  1. it can lead to overlapping texts with small panels:
image

Comment on lines 35 to 77
function estimateLabelWidth(format: FormatOptions | undefined, maxValue: number): number {
const formattedLabel = formatValue(maxValue, format);
return formattedLabel.length * AVG_CHAR_WIDTH + AXIS_LABEL_PADDING;
}
Copy link
Contributor

@jgbernalp jgbernalp Jan 23, 2026

Choose a reason for hiding this comment

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

I tried a new approach that keeps character width variance in mind. It seems to be working much better.

// Character width multipliers (approximate for typical UI fonts)
const CHAR_WIDTH_BASE = 12;
const CHAR_WIDTH_MULTIPLIERS = {
  dot: 0.5, // Dots and periods are very narrow
  uppercase: 1.0,
  lowercase: 0.55, // Lowercase letters slightly narrower
  digit: 0.65,
  symbol: 0.7, // Symbols like %, $, etc.
  space: 0.5, // Spaces
};
const AXIS_LABEL_PADDING = 14;

/**
 * Calculate the width of a single character based on its type
 */
function getCharWidth(char?: string): number {
  if (!char || char.length === 0) {
    return 0;
  }

  if (char === '.' || char === ',' || char === ':') {
    return CHAR_WIDTH_BASE * CHAR_WIDTH_MULTIPLIERS.dot;
  }
  if (char === ' ') {
    return CHAR_WIDTH_BASE * CHAR_WIDTH_MULTIPLIERS.space;
  }
  if (char >= 'A' && char <= 'Z') {
    return CHAR_WIDTH_BASE * CHAR_WIDTH_MULTIPLIERS.uppercase;
  }
  if (char >= 'a' && char <= 'z') {
    return CHAR_WIDTH_BASE * CHAR_WIDTH_MULTIPLIERS.lowercase;
  }
  if (char >= '0' && char <= '9') {
    return CHAR_WIDTH_BASE * CHAR_WIDTH_MULTIPLIERS.digit;
  }
  // Symbols like %, $, -, +, etc.
  return CHAR_WIDTH_BASE * CHAR_WIDTH_MULTIPLIERS.symbol;
}

/**
 * Estimate the pixel width needed for an axis label based on the formatted max value.
 * This provides dynamic spacing that adapts to the actual data scale.
 */
function estimateLabelWidth(format: FormatOptions | undefined, maxValue: number): number {
  const formattedLabel = formatValue(maxValue, format);

  // Calculate width based on individual character types
  let totalWidth = 0;
  for (let i = 0; i < formattedLabel.length; i++) {
    totalWidth += getCharWidth(formattedLabel[i]);
  }

  return totalWidth;
}
Image

* This provides dynamic spacing that adapts to the actual data scale.
*/
function estimateLabelWidth(format: FormatOptions | undefined, maxValue: number): number {
const formattedLabel = formatValue(maxValue, format);
Copy link
Contributor

Choose a reason for hiding this comment

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

With some values there still a problem. IIUC this max value might not be the value with more text. e.g. 0.00001 vs 1

Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
…n for regular single-Y-axis situations

Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
…nent (UnitSelector) in the call hierarchy

Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
@AntoineThebaud
Copy link
Contributor Author

AntoineThebaud commented Jan 26, 2026

@jgbernalp with your 2 suggestions it's still not perfect, even worse than before with my sample I'd say:

image

However I like your approach that is more fine-grained so I'll try to code further from it, thanks for sharing 🙏

Signed-off-by: AntoineThebaud <antoine.thebaud@yahoo.fr>
@AntoineThebaud AntoineThebaud force-pushed the antoinethebaud/multiple-y-axis-2 branch from 75f530d to 9e05e58 Compare January 26, 2026 15:13
@AntoineThebaud
Copy link
Contributor Author

@jgbernalp I pushed a new commit that takes most of your suggestions with slight changes (mostly constant values), please check again
image

@jgbernalp
Copy link
Contributor

@AntoineThebaud with the current value of:

const CHAR_WIDTH_BASE = 8;

I see:

Screenshot 2026-01-26 at 18 16 27

with

const CHAR_WIDTH_BASE = 9;
Screenshot 2026-01-26 at 18 17 49

The font size might also impact in different browsers / OS. But I think it works much better. I left it to you If you still want to change the base to 9. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants