Repository navigation
fix(app): route duration axis ticks through the compact formatter #3148
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
Changes from all commits
6e45c5c
f16ea7f
e037544
218d229
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| fix: keep duration axis-tick labels within the chart's width budget | ||
|
|
||
| Duration-formatted Y-axis labels (e.g. "13.33min") could render wider than the axis area allows and get clipped. They now use the same compact formatter already used by the heatmap chart's axis. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,12 @@ import { DisplayType } from '@hyperdx/common-utils/dist/types'; | |
| import { Popover } from '@mantine/core'; | ||
|
|
||
| import type { NumberFormat } from '@/types'; | ||
| import { COLORS, formatNumber, truncateMiddle } from '@/utils'; | ||
| import { | ||
| COLORS, | ||
| formatDurationMsCompact, | ||
| formatNumber, | ||
| truncateMiddle, | ||
| } from '@/utils'; | ||
|
|
||
| import { | ||
| AnnotationHitLayer, | ||
|
|
@@ -838,6 +843,14 @@ export function formatAxisTick( | |
| }).format(value); | ||
| } | ||
|
|
||
| // formatNumber returns early for 'duration', before the mantissa/width | ||
| // safety below ever runs, and formatDurationMs has no width budget of its | ||
| // own - use the compact formatter instead, as DBHeatmapChart's axis does. | ||
| if (axisNumberFormat.output === 'duration') { | ||
|
arj22 marked this conversation as resolved.
Resolved
arj22 marked this conversation as resolved.
Resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major — CLI's documented
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond |
||
| const factor = axisNumberFormat.factor ?? 1; | ||
|
arj22 marked this conversation as resolved.
Resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Third copy of the duration The same two lines already exist in Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| return formatDurationMsCompact(value * factor * 1000); | ||
|
arj22 marked this conversation as resolved.
Resolved
arj22 marked this conversation as resolved.
Resolved
|
||
| } | ||
|
|
||
| const displayed = axisNumberFormat.output === 'percent' ? value * 100 : value; | ||
|
|
||
| return formatNumber(value, { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.