Skip to content

Embed cutoff fix #4474

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.

Sign up for GitHub

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

Merged
ncarazon merged 1 commit into from
Mar 12, 2026
Merged

Embed cutoff fix #4474

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion front_end/src/components/charts/primitives/chart_value_box.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ const ChartValueBox: FC<{
const adjustedX =
isCursorActive || isDistributionChip
? x
: chartWidth - rightPadding + textWidth / 2 + CHIP_OFFSET;
: Math.min(
chartWidth - rightPadding + textWidth / 2 + CHIP_OFFSET,
chartWidth - textWidth / 2 - 2
);
Comment on lines +216 to +219
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clamp against the widest rendered label, not just displayText.

This cap only uses textWidth, which is measured from the lower chip text. When hasResolution is true, the separate RESOLVED label above can still overflow on the right if it is wider than the chip value, so the embed cutoff is only partially fixed here. Please base the clamp on the max width of both rendered texts.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@front_end/src/components/charts/primitives/chart_value_box.tsx` around lines
216 - 219, The right-side clamp currently uses textWidth (measured from the chip
label) which can allow the RESOLVED label to overflow when hasResolution is
true; update the clamp to base on the widest rendered label by computing
maxTextWidth = hasResolution ? Math.max(textWidth, resolvedLabelWidth) :
textWidth (use the actual variable name for the resolved label width in this
file), then replace textWidth in the Math.min expression with maxTextWidth so
the position calculation (using chartWidth, rightPadding, CHIP_OFFSET) prevents
overflow for either label.

const hasResolution = !!resolution && !isCursorActive;

const chipFill =
Expand Down
Loading