Feat/new UI components - #1190
Draft
clement-scality wants to merge 8 commits into
Draft
Conversation
Tracking the status of N entities over the same time buckets currently means stacking one GlobalHealthBar per row, which draws an x-axis per row and gives no way to read down a column. Two story-only recipes, so nothing is added to src until the shape is agreed: StatusHeatmap builds the grid from Box, Tooltip and the existing ChartLegend — clicking a legend entry filters, with unselected cells fading rather than vanishing so the shape of the data survives the filter — and HealthBarHeatmap keeps one GlobalHealthBar per row for comparison, hiding every x-axis but the last. A fifth story covers continuous values, where the discrete legend becomes a gradient scale.
…tives A hierarchical breakdown of one total, where each node is a box whose width is proportional to its parent and which contains its children. That containment is what a stacked Barchart loses: side by side, segments only say "these add up"; nested, they say "this one is part of that one". Story-only, like the Heatmap recipe. It also carries resolveChartColor, which is a candidate for charts/common: GlobalHealthBar hardcodes theme.statusX, Sparkline wants a hex and LegendShape does chartColors[c] || c — three conventions for one question, where a single theme-token then palette then raw-CSS resolution serves all three. Two behaviours worth reviewing. The value sits beside the children while the unattributed remainder leaves room for it and climbs onto the label row otherwise, which reproduces the reference mock without ever overflowing. And children summing past their parent scale down to fit the row rather than overflowing it, while every label keeps stating the value the payload sent — an inconsistent answer should read as inconsistent, not as a broken component. The tooltip is on each label, not on each box: Tooltip opens on pointer-enter, which reaches every ancestor, so a tooltip on a box stacks one overlay per level on a single hover. Labels never nest.
A metric idling at 2ms and spiking to 4s cannot be read on a linear axis: the axis is sized by the spike, so the baseline where the metric spends nearly all its time is pressed flat against zero. Same for a Barchart whose bars span four orders of magnitude — only the tallest one is legible. yAxisScale="log" on Barchart and LineTimeSerieChart, defaulting to linear. It is a display option and nothing more. The values are not logarithmic: none of them is rescaled, and the tooltips, the legend and the unit scaling all keep the numbers the caller passed in. All that changes is how the axis is drawn and where the marks land on it. Three things the axis has to do differently, all in charts/common so both charts share them: - The scale cannot start at zero. It is bounded by the decades enclosing the data — read off the smallest positive value and the largest, so it follows the data rather than being anchored to 1: values between 0.001 and 0.1 with spikes to 50 give a 0.001..100 axis. allowDataOverflow clips anything below rather than letting the axis chase it. - Its ticks are the decades. Evenly spaced linear ticks on a log axis crowd into the top of the plot and read as a broken chart; past six decades whole decades are skipped, never subdivided. - A measured zero needs somewhere to go, since it has no logarithm and no place on the scale. The axis reserves one slot below its first decade, labels it 0, and draws zeros there: a stub bar at the 0 tick, or a line running along it. Without that slot a zero has to be dropped, and a dropped zero leaves exactly the gap a missing sample leaves — making "the metric read zero" indistinguishable from "we have no data", which are not the same fact. The slot costs a decade's worth of height, so it is only reserved when the data actually holds a zero. Because a zero is plotted at that slot rather than at its own value, both tooltips map it back before formatting — including a caller's own Barchart `tooltip` renderer, which reads through getCurrentPoint. No real value can collide with the slot: it sits below the smallest positive value in the data. Negatives keep no slot and are dropped. A series that goes negative does not belong on a log axis at all, which is also why a symmetrical line chart refuses the scale at the type level. Stacked bars fall back to linear: stacking places each segment at a cumulative sum, so a segment's height would stop matching its value — the chart would still draw, and would be wrong. Stories put each scale next to its linear equivalent, since the point is the comparison — including what you give up: distances no longer read as differences.
The recipe existed to answer "could we get this from GlobalHealthBar instead of a new grid" — it could, but it needs one chart instance per row, hides every x-axis but the last, and claws back the space they still occupy with a negative margin. Now that the Box-grid recipe is the one being taken forward, keeping the comparison around is just a second thing to maintain. Takes SEVERITY_BY_STATUS, statusRowToAlerts and HealthBarCell with it, which had no other caller, and renumbers the remaining recipes.
The stories showed fixed compositions, so trying a different cell height or variant meant editing the file. Every prop that is a real design question is now an argType, which is also what makes the tradeoffs arguable rather than asserted: - Heatmap — rows, buckets, trailing no-data columns, cell height, gap, label frequency, label gutter, and the opacity floor for the numeric recipe. Push the gap to 0 and the grid becomes a continuous timeline. - NestedProportionBar — variant, value position, percentage base, the inline-value threshold and level height, driving every bar in a story at once. The three stories that hardcoded a setting now express it as their starting args instead, so the control governs. - Barchart and LineTimeSerieChart — a playground each for yAxisScale. The barchart one exposes `stacked` next to it, which demonstrates the fallback to linear rather than just documenting it; the line chart one exposes a `zeros` toggle, since where zeros land is the interesting part of a log axis. NumericHeatmap's opacity floor was hardcoded at 0.1 and becomes a real prop, which is what let it become a control.
… the layout Three things were wrong with the controls I added. The row control was a lie. It ranged to 24 while the labels came from MONITORING_SERVICES.slice(0, n), and that list holds five names — so anything above five rendered five rows. Labels are generated past the named services now, and the range means what it says. "Bucket" meant two things. The stories used it for the time axis, but a bucket here is a monitored entity, which is a row — so a control that added columns read as though it should add rows. The time axis is timeSlots/columns throughout, and the word is free again for the thing it names in this domain. And the statuses could not be edited at all, which is what you actually want to poke at: a Heatmap Playground now takes `rows` as an object control, so you type the labels and the OK/WARNING/CRITICAL/NONE cells and the grid follows — including the x-axis, derived from the longest row, so adding cells adds columns. NestedProportionBar gets the same treatment with `root`: change a value, add a child, set a colour. The generated stories keep count controls, since hand-editing 400 cells is not a thing anyone wants to do.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const logAxis = useMemo( | ||
| () => | ||
| isLogScale | ||
| ? getLogAxis(minPositiveValue, topDomain, { |
Contributor
There was a problem hiding this comment.
topDomain includes a 10% buffer (maxValue * 1.1 from normalizeChartDataWithUnits). Since getLogAxis already rounds up to the next decade, this buffer can push the value past a decade boundary and waste an entire decade of vertical space.
For example, data max = 950 → topDomain = 1045 → getLogAxis rounds to 10 000 instead of 1 000. The Barchart avoids this by passing the unbuffered normalizedMaxValue instead.
Consider exposing the raw normalized max from useChartData (the way Barchart.utils.ts does with normalizedMaxValue) and passing it here instead of topDomain.
| * passed in. | ||
| * | ||
| * Two things it cannot do: | ||
| * - A bar whose value is zero or negative is dropped, because a log axis |
Contributor
There was a problem hiding this comment.
This docstring says zeros are dropped ("A bar whose value is zero or negative is dropped"), but the implementation places zeros at a reserved band via placeNonPositiveValues + getLogAxis({ withZeroBand: true }). The tests verify they render as visible stubs. The same mismatch exists in LineTimeSerieChart.types.ts line 35-40.
The docs should describe the actual behavior: zeros are placed at a reserved 0 band, not dropped.
| entry.value, | ||
| // A zero is plotted at the axis's reserved band so it can be | ||
| // drawn; the tooltip has to report the 0 that was measured. | ||
| readLogPlottedValue(entry.value, logZeroValue), |
Contributor
There was a problem hiding this comment.
This correction only runs for the default tooltip path. When renderTooltip is provided (line 47), it receives the raw tooltipProps — zeros still carry the band position (e.g. 0.1) rather than 0. A custom tooltip on a log axis with zeros will display the internal band value.
The Barchart avoids this because its getCurrentPoint corrects values before passing to the custom tooltip callback.
The log stories had copied their series label from the fixtures next to them, which use a real-looking internal hostname. This repo is public and the stories end up in screenshots on a public page, so they use a generic storage-node-1 instead. Needs its own legend wrapper, since ChartWithProviders keys its colorSet by that hostname and the label has to match for the colour to resolve.
|
About the "NestedProportionBar — default story" part: Rectangle of the same level should have the same height, so should be equal to max height. Ex: In the given exemple, the height of "Available" should be equal to the height of "Used". |
Review feedback on #1190: rectangles at the same level should all be as tall as the tallest one. They were not — the children row centred each box at its own content height, so a leaf floated beside a deep branch and the row read as ragged. The row stretches its children now, which is all it takes: its cross size is already set by the child with the deepest subtree. Three things follow from that. NodeSlot becomes a flex column and NodeBox takes flex: 1, so the box fills the height the row hands the slot rather than hugging its content. And the inline value is centred explicitly, since stretch would otherwise pin it to the top of the row. Splits Row in two while it is here — it was doing two jobs with opposite alignment needs, and now says which is which: ChildrenRow stretches, HeaderRow centres.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Component:
charts— Barchart, LineTimeSerieChart, and two new Storybook recipesTL;DR — Adds an opt-in logarithmic Y axis to
BarchartandLineTimeSerieChart, plus two story-only recipes proposing aHeatmapand aNestedProportionBarbuilt from primitives that already exist.Context / Why
A metric that idles at 2 ms and spikes to 4 s cannot be read on a linear axis: the axis is sized by the spike, so the baseline where the metric spends nearly all its time is pressed flat against zero.
The two recipes are a design proposal, not a component. They live entirely in
stories/so the shape can be reviewed, and argued with, before anything is added tosrc/.🧩 Approach — the logarithmic axis
yAxisScale="log", defaulting tolinear. It is a display option and nothing more: no value is rescaled, and tooltips, legend and unit scaling all keep the numbers the caller passed in. Only the axis and where the marks land on it change.Three things a log axis has to do differently, all in
charts/commonso both charts share them:0, and draws zeros there.That last one is the part worth arguing about. Without the reserved slot a zero has to be dropped, and a dropped zero leaves exactly the gap a missing sample leaves — making "the metric read zero" indistinguishable from "we have no data", which are not the same fact. Clamping to the first decade is worse, not better: it draws the zero as the smallest measured value, a number the reader will believe. The slot costs a decade's worth of height, so it is only reserved when the data actually holds a zero.
Two pairings are refused rather than drawn wrong. Stacked bars fall back to linear, since stacking places each segment at a cumulative sum and a segment's height would stop matching its value. A symmetrical line chart refuses the scale at the type level — half its axis is negative by construction.
📷 Screenshots
Barchart —
LogarithmicScale. Linear on top: only the 40 000 bar is legible. Log below: every decade gets the same height, andcategory5(value0) is the stub at the reserved0.LineTimeSerieChart —
LogarithmicScaleExample. Same series twice. On the linear axis the baseline is a flat line on zero; on the log axis its variation and both spike tiers are readable.Heatmap — default story. One recipe of the proposal; the rest is worth clicking through rather than screenshotting.
NestedProportionBar — default story.
charts/common/chartUtils.ts › getLogAxis,placeNonPositiveValues— the domain, the decade ticks and the reserved zero slot. The zero handling is the design decision in this PR; if you disagree with reserving a slot, this is the place to say so.barchart/Barchart.utils.ts › getCurrentPointandLineTimeSerieChartTooltip— a zero is plotted at the reserved slot, so both tooltips map it back before formatting, including a caller's own Barcharttooltiprenderer. No real value can collide with the slot: it sits below the smallest positive value in the data.stories/Heatmap,stories/NestedProportionBar— story-only, nothing insrc/. Review the shape, not the code.🧪 How to test
The logarithmic axis — Charts → Barchartv2 and Charts → LineTimeSerieChart:
Logarithmic scaleon either chart and compare the two stacked charts. Same data, only the axis differs.Logarithmic scale playgroundand use the Controls panel —yAxisScaleflips the axis live.stacked. The axis snaps back to linear: that is the documented refusal, not a bug.zeros. The zeros land on the reserved0at the bottom; untick and the axis loses that slot.Logarithmic scale below oneshows the axis following data between 0.001 and 0.1 with spikes to 50 — nothing is anchored to 1.The two proposed recipes — Charts → Heatmap (composition) and Charts → NestedProportionBar (composition). Both open on a
Playgroundstory whose data is a control, so this is meant to be poked at rather than read:Playground→ Controls →rows. It is a JSON editor: add a row, rename one, change any cell toOK/WARNING/CRITICAL/NONE. The x-axis is derived from the longest row, so adding cells adds columns.Playground→ Controls →root. Change a value, add a child, set acolorto a theme token, achartColorskey or a raw CSS colour.root: make the children sum past their parent (say 80 and 60 of 100). They scale to fit the row while every label keeps stating what you typed — an inconsistent payload should read as inconsistent, not as a broken component.Edge casescovers the rest: an unattributed remainder, labels wider than their box, a total of zero.Theme switching is worth a pass on all of the above — the reserved
0, the status colours and the box outlines all come from tokens.npm test -- src/lib/components/charts🔗 References
Breaking Changes: none.
yAxisScaleis optional and defaults tolinear, so every existing caller renders exactly as before. Nothing is added tosrc/for the two recipes.No issue or ticket for this one.
What changed
charts/common/chartUtils.tscarries the whole log-axis vocabulary —getLogAxis,getMinPositiveValue,hasZeroValue,placeNonPositiveValues,formatLogTickValue,readLogPlottedValue— next togetRoundReferenceValue/getTicks, which do the same job for the linear axis. Both charts wire it the same way, which is why the two diffs read almost identically.stories/Heatmap/proposes two recipes:StatusHeatmapfor discrete statuses, built fromBox+Tooltip+ the existingChartLegend, andNumericHeatmapfor continuous values under an opacity ramp. An earlier third recipe stacking oneGlobalHealthBarper row was dropped — it worked, but it needs one chart instance per row and hides every x-axis but the last.stories/NestedProportionBar/also carriesresolveChartColor, which is a candidate forcharts/commonin its own right:GlobalHealthBarhardcodestheme.statusX,Sparklinewants a hex andLegendShapedoeschartColors[c] || c— three conventions for one question, where a single theme-token → palette → raw-CSS resolution serves all three.