Skip to content

fix(viz): visual-review defects across 41 charts - #102

Merged
mortonanalytics merged 41 commits into
mainfrom
fix/visual-review-defects
Jul 29, 2026
Merged

fix(viz): visual-review defects across 41 charts#102
mortonanalytics merged 41 commits into
mainfrom
fix/visual-review-defects

Conversation

@mortonanalytics

Copy link
Copy Markdown
Owner

Fixes the defects found in a full visual + interaction review of the demo app (41 charts), plus ~24 additional issues surfaced during the work.

Highest impact

  • Linked brushing now propagates. Source chart dimmed correctly but the linked target never responded. Verified: both charts now report 22/32 dimmed.
  • whiskerType = "minmax" no longer errors on every boxplot — exported-API correctness bug.
  • Waterfall total rows render a real value instead of NA.
  • Significance brackets now sit over the pairs they name.

Chart fixes

  • Waffle: renders its legend (previously absent from both the inline and panel surfaces)
  • Bump: per-series colours (Okabe-Ito) matching the legend; marks aligned to x ticks
  • Small multiples: facet title rendered once; every panel gets its own axes
  • Ridgeline: y axis labelled with group names rather than stacking offsets; deterministic order
  • Radar: grid rings and radial value labels
  • Funnel / Sankey: values and percentages rendered; labels kept inside the plot
  • Bar family: value axes start at zero
  • Violin: median marker inside the IQR box
  • Regression: split hues for data vs model; y axis no longer runs negative

Cross-cutting

  • Grouped layers labelled with the group value alone, with a legend title naming the variable
  • Rotated y-axis titles no longer clipped; left margin fits its tick labels
  • Floating action button kept clear of marks, x-axis labels and the legend
  • scripts/screenshot-all.js enumerates tabs from the live DOM (it previously assumed a "Financial" dropdown and crashed partway)

Verification

  • vitest: 569 passed / 61 files
  • testthat: 1373 passed, 0 failed (3 skipped — duckdb fixture unavailable)
  • Live sweep of all 41 charts: 0 console errors or warnings
  • Each visual fix confirmed in-browser against the rebuilt bundle

No changes to DESCRIPTION, cran-comments.md, or .Rbuildignore. NEWS entries land under a new # myIO (development version) section.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

mortonanalytics and others added 30 commits July 28, 2026 21:50
The script hard-coded a tab list that assumed a "Financial" dropdown. Financial,
Relational, Theme Demo and Export Demo are plain top-level tabs, and
Candlestick/Waterfall/Heatmap/Sankey are nested sub-tabs, so the run crashed
partway and never reached most charts.

Enumerate navbar entries and nested tabsets from the DOM instead, covering all
41 charts, and record per-chart console errors/warnings in report.json.

scripts/ is .Rbuildignore'd, so tracking this one file keeps the CRAN build
unchanged while letting the regression test import it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bindLinked() registered its inbound Crosstalk subscriptions as
sel.on("change.myIO", ...) / fil.on("change.myIO", ...). Crosstalk's
Events class keys listeners by the exact event-type string and has no
jQuery-style namespace support, and it only ever triggers "change", so
this._types["change"] was undefined and the dispatch loop was a silent
no-op. applySelection()/applyFilter() therefore never ran on target
charts: the outbound side published the selection correctly, the group
var updated, and nothing visible happened -- with no console error.

Subscribe to the un-namespaced "change" instead, matching the working
sibling path in crosstalk-adapter/index.js. The namespace was never
needed for teardown: cleanupLinked() calls handle.close(), which does
removeAllListeners().

Regenerates the tracked esbuild bundle so the fix reaches the runtime.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WaffleRenderer declares legendType "ordinal", so legend construction routes
through buildOrdinalLegendData. That function derived its keys from an
if/else-if chain covering treemap, donut, funnel, radar and parallel only --
there was no waffle arm, so keys stayed empty and it returned
{ type: "ordinal", items: [] }. With zero labels resolveLegendPlacement fell to
"too-few-items", which skips the inline strip and defers to the panel, and the
panel then rendered an empty legend section. Net result: no legend on either
surface.

Add a waffle arm that dedupes layer.mapping.category in first-occurrence order.
The arm is inserted after funnel and before radar; no existing arm's condition
or ordering changes.

Second gap: swatches would all have rendered grey. buildOrdinalLegendData colours
them via chart.colorDiscrete, and WaffleRenderer kept its ordinal scale as a
local, never publishing it (chart.derived.colorDiscrete is undefined for waffle
because applyDerivedScales returns early for non-axes charts). Publish the scale
the way FunnelRenderer and ParallelRenderer do, setting the domain explicitly in
first-occurrence order first -- which reproduces d3.scaleOrdinal's implicit
assignment exactly, so cell fills are byte-identical to before.

No R API surface is touched and no animation is added or changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BumpRenderer.render() never consulted layer.color. It fell back to
`chart.derived.colorDiscrete || d3.scaleOrdinal(d3.schemeCategory10)`,
and colorDiscrete only exists when setColorScheme() is active. R splits a
bump chart on `group` into one layer per series, so render() ran once per
series and each call built a brand-new ordinal scale whose first lookup
always returned schemeCategory10[0] (#1f77b4). Every series painted the
same blue while the layer legend drew the correct per-series swatches.

Resolve the mark colour the way LineRenderer does: resolveColor(chart,
name, layer.color), falling back to the ordinal scale only when the layer
carries no colour. An array colour (JS/MCP surface) is indexed by group
position so it cannot be stringified into the fill attribute.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On a categorical x axis, resolveScaleSemantics() forces a d3.scaleBand for
bump charts, but BumpRenderer positioned lines and dots at the bare
xScale(value), i.e. the band's left edge, while d3-axis places band ticks
at scale(d) + bandwidth/2. Every mark therefore sat exactly bandwidth/2
px left of the tick it labelled.

Compute the same bandOffset that LollipopRenderer and DumbbellRenderer
already use and add it to the line generator's x accessor and to both
circle cx assignments. bandOffset is 0 for scales without .bandwidth, so
non-categorical bump charts are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Small multiples repeated the chart title in every panel and dropped the
y axis on all panels but the first.

Root cause: FacetPanel.buildPanelChart built each panel's config by
copying the parent config wholesale, so every panel inherited
config.title and initializeScaffold drew a myIO-chart-title node per
panel SVG; nothing rendered a title for the composite (the parent SVG is
display:none and Chart.renderCurrentLayers returns before its own
renderChartTitle call). Separately, updateGridPosition implemented
ggplot-style shared-edge-axis suppression -- y axis only in column 0, x
axis only in the last row -- which was invalid here for two reasons.
The .myIO-facet-grid container had no CSS anywhere in the package, so
the inline grid-template-columns was inert and the panels stacked
vertically at full width, meaning the "left column" axis was really an
axis on the top panel only. And buildMargin shrank the suppressed side's
margin from 50/60 to 12, so suppressed panels had a different plot rect
than the panel carrying the shared axis -- the same domain mapped to
different pixels per panel, so the axis did not describe them.

Fix: panels are built with title: null so no panel draws a title, and
FacetController.createGrid renders the composite title once as a
.myIO-facet-title div immediately above the grid (removed again in
destroy(), so toggling facet off leaves no orphan). The suppressX and
suppressY fields, updateGridPosition, getColumnCount and
applyAxisSuppression are removed; every panel now gets uniform margins
and both axes, and the panel's suppressAxis option is taken from the
user's own suppressAxis() setting instead of being overridden. style.css
gains the facet rules that never existed: display:grid on the grid (so
setFacet(ncol=, minWidth=) finally take effect), plus title, panel and
label styling using existing theme vars so light/dark follow the theme.

Domain sharing under scales = "fixed" is unchanged; it still comes from
globalScaleSnapshot. No R API change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RadarRenderer emitted only two sub-layers: `.radar-axis-layer` (one radial
spoke plus a category label per axis) and `.radar-polygon-layer`. There was no
code anywhere in the renderer that emitted a concentric ring, a circle, or a
radial value tick, so the chart had no magnitude reference at all -- the
"grid" visible in the plot was just the spokes. This was not a config or
default-flip problem; the rendering simply did not exist.

Add a `.radar-grid-layer` group, appended before the axis and polygon layers so
rings paint underneath the data. It draws `gridLevels` (default 4) equally
spaced polygonal rings at fractions of the radius domain max -- the outermost
ring coincides exactly with the spoke ends -- plus a `.radar-grid-label` value
readout per ring running up the index-0 spoke. Rings and labels are
`pointer-events: none`, so hover and tooltip behaviour on `.radar-polygon` is
unchanged, and they reuse the existing `var(--chart-grid, ...)` /
`var(--chart-fg, ...)` tokens so both themes are covered with no CSS change.
Ring geometry animates with the renderer's existing `transitionSpeed`, which is
0 when transitions are disabled, and the final state is written directly in
that case.

Opt-outs travel through the free-form layer `options` passthrough:
`options = list(grid = FALSE)` suppresses the grid entirely and
`options = list(gridLevels = n)` changes the ring count. Additive only --
no exported R API change, no renderer contract change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two independent causes, both in the shared scale derivation.

1. `processScales()` tested the configured axis limits for truthiness:
   `chart.config.scales.ylim.min ? +... : y_min - y_buffer`. `0` is falsy, so an
   explicit lower limit of zero was silently discarded on every chart type and
   the buffered data extent was used instead. The grouped bar demo calls
   `setAxisLimits(ylim = list(min = 0))`, the config arrived intact, and the
   derived domain still came out [49.85, 103.15] -- 56 - 0.15*(97-56). The same
   bug applied to `xlim`. Replaced both truthiness tests with a `hasLimit()`
   guard that accepts any finite number including zero and still falls through
   for null/undefined/"".

2. Nothing forced zero into the y domain for length-encoding marks, so without
   an explicit limit every bar-family chart got [min - 15%, max + 15%]. Added a
   `yZeroBaseline` scale hint, declared by BarRenderer, GroupedBarRenderer and
   LollipopRenderer, unioned across layers by `resolveScaleSemantics()` the same
   way `domainMerge` already is, and applied in `processScales()` by clamping
   the extent to include 0 and suppressing the buffer only on the clamped side.
   All-positive data therefore sits on 0, all-negative data hangs from 0, and
   mixed-sign data keeps buffers on both sides. An explicit `setAxisLimits()`
   still wins on either bound.

This also fixes the geometry: `groupedBarHelpers` draws rects from `yScale(0)`,
which with the truncated domain landed ~384px below the plot floor, so the bars
overflowed and were clip-path'd -- they looked zero-based while the axis said
50. Correcting the domain aligns the two.

The hint defaults to false, so line, point, area, candlestick, waterfall,
rangeBar, heatmap, hexbin, dumbbell, beeswarm, quantile_dots and bump domains
are byte-identical. Histograms take the separate `createBins()` path and were
already zero-based. Schema regenerated with `npm run schema`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The violin composite emitted its median sub-layer as a point layer mapped
with only x_var/y_var. PointRenderer draws the median rule only when
layer.mapping.low_y is truthy, and then returns early before the fallback
circles, so the median layer produced zero DOM nodes: the IQR box rendered
with nothing marking the median inside it.

Mirror the boxplot composite, which already works: carry low_y and high_y
columns equal to the median value and include both in the layer mapping.
The renderer's medianLine half-width formula matches RangeBarRenderer's
barWidth/2 and both layers have one row per group, so the rule spans the
box exactly. Values duplicate existing y_var medians, so the y domain is
unchanged. No JS change needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sets

The ridgeline composite places each group's area at an integer baseline
(1, 2, 3, ...) and emitted no y tick-label metadata. The engine's positional
tick-label mechanism existed only for x (derive_positional_x_tick_labels ->
axes.xTickLabels -> updateXAxis), so updateYAxis always fell back to d3's
default ticks and printed the raw baselines (0.8, 1.0, ... 3.6). The
"Density" y title compounded it: it came from the demo and vignette callers,
not the package, so the axis actively claimed to be something it was not.

Add the y-side mirror: derive_positional_y_tick_labels() (gated to ridgeline
only) maps each integer baseline to its group label, a new axes.yTickLabels
config field carries it through Chart.js and facet-panel.js, and updateYAxis
uses tickValues/tickFormat when it is present. The branch requires a scale
with .invert, so band-scale y axes keep the existing path, and out-of-domain
positions are filtered so a narrowed setAxisLimits() cannot push ticks off
plot. The y axis title defaults to the grouping column only when the user has
not set one, and a later setAxisFormat() still wins. Every other chart type
is unaffected because yTickLabels stays NULL for them.

The ridgeline demo, both vignettes and the e2e fixture stop labelling the
axis "Density".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither renderer emitted any numeric text. FunnelRenderer drew only a
.funnel-label containing d.stage; SankeyRenderer drew only node names and
plain <path> ribbons. d.value existed but was consumed exclusively by
formatTooltip, so the magnitudes were invisible unless the user hovered.

Funnel stages now render a second .funnel-value text with the formatted
value and its conversion rate against the first stage. Sankey node labels
append the node total and each link gets a midpoint flow-magnitude label.
Both read layer options valueFormat/percentFormat, falling back to the
existing chart.options.yAxisFormat channel and then to ",".

Placement degrades rather than overlapping: funnel values sit inside the
trapezoid when the text fits, just outside its right edge when it does
not, and are hidden when neither fits or the band is too short; sankey
flow labels are hidden on ribbons thinner than 11px or shorter than the
text. Ink is chosen per mark via the new WCAG helper so the label always
clears 4.5:1 against the fill or ribbon composite it is drawn on.

showValues = FALSE on the layer restores the previous names-only output.
All labels ride the existing transition speed, so duration 0 still
renders the final state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d3-sankey was given .extent([[0,0],[width,height]]) with the same width
the chart-area clip rect uses, so the terminal column landed at x1 ===
width — flush with the clip boundary, with only the 5px default right
margin behind it. With no room to the right, the terminal labels fell
into the "anchor end at x0 - 6" branch and were drawn back inside, on
top of the link ribbons.

The layout now measures the terminal-node labels (the nodes that are
never a source, whose totals equal their incoming sums), reserves that
width as a gutter capped at 30% of the plot, and lays the sankey out in
the remaining space. Terminal labels are anchored "start" in the gutter.

Label contrast: the ink was var(--chart-text-color, #333), resolving to
#6b7280, which measured 2.87-3.42:1 against the ribbon composites it was
drawn over. Labels now use readableTextColor(--chart-bg) plus a 3px
background-coloured halo with paint-order: stroke, making the effective
backdrop the chart background (21:1). Presentation attributes are used
so exported SVG keeps the on-screen colours.

Known limitation: the ink is resolved from --chart-bg at render time, so
a live OS theme flip on a theme = "auto" chart keeps the previous ink
until the next render. ThemeManager only rewrites custom properties and
does not re-render layers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two problems on the regression demo.

The y-axis domain ran to -5 on a chart whose data never dips below
-0.36. The driver was Y_DOMAIN_BUFFER = 0.15 in derive/scales.js applied
symmetrically: 15% of the 36-unit range is 5.4 units of manufactured
empty space below the data. The x axis has always used 0.05; the
asymmetry was unmotivated. Drop Y_DOMAIN_BUFFER to 0.05 and clamp the
padded lower bound at zero when the data minimum is non-negative, so
padding can no longer invent a negative axis. Explicit ylim and the
zero-baseline path are untouched and still win.

composite_regression() handed the single caller `color` to all three
sublayers, so the scatter, the fit line and the CI band rendered in one
hue at similar weight. Split it: the scatter keeps the caller colour, the
trend line and band take a second Okabe-Ito hue, and the band defaults to
0.18 fill opacity (AreaRenderer already honours options.areaOpacity). A
two-element `color` vector controls both explicitly, so the old
single-hue appearance is one argument away.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five sites built a layer label as paste0(label, " - ", <group value>), so
a grouped layer showed up in the legend as "Rankings - Core" and a
grouped bar chart as "Temperature by Month - 5". The layer label IS the
legend text (buildLayerLegendData uses it verbatim), and the inline
legend truncates at 24 characters, so the repeated prefix crowded out the
part that actually distinguishes the series. The chart title and axis
titles already carry the context the prefix was duplicating.

build_grouped_layers(), expand_grouped_df(), and the regression, qq and
survfit composites now label a grouped layer with the bare group value.
Uniqueness is preserved: build_grouped_layers() and expand_grouped_df()
keep a ledger of labels already on the chart and fall back to the old
qualified "<label> - <group>" form on collision, so two grouped layers
over the same levels still get distinct labels instead of erroring.
Explicit labels on ungrouped layers are untouched.

test_layer.R's overlapping-group-values test asserted the old prefixed
form; it now asserts the stronger post-fix contract (bare values for the
first layer, qualified fallback for the second, all still unique).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The y-axis title was anchored at translate(-margin.left + 6, ...) inside a
plot <g> that is itself translated by margin.left, so the anchor always
landed at absolute x = 6 regardless of the configured left margin. Under
rotate(-90) the glyph ascent (~12px at the 13px .myIO-axis-title size) runs
toward smaller x, putting the tops of the letters at x = -6, outside the SVG
root's overflow:hidden clip. Every chart with a y-axis title lost 6px of it.

Move the anchor to margin.left - 14, clamped so a left margin under 20px keeps
today's placement rather than drifting into the tick labels.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.myIO-fab is a 40px button pinned 12px from the container's right edge, and
the default right margin is 5px, so any renderer that paints its full plot
rect ends up underneath it. The widest funnel stage (range [0, width * 0.95],
centred) overlapped the button by 14x18px and the rightmost sankey node and
its label by 11px and 23px respectively.

Both renderers now subtract a shared FAB_GUTTER (56px, i.e. the button plus
4px clearance) from the width they lay out into, less whatever the right
margin already provides. The funnel reserves it symmetrically so the shape
stays centred; the sankey reservation is applied before the terminal-label
gutter so the two do not stack twice. Charts with margin.right >= 56 reserve
nothing and render exactly as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
openPanel had no visibility teardown. A Shiny navbarPage / bslib / Quarto tab
switch does not destroy the widget - the framework only sets display:none on
the pane - so renderValue and destroy never run and closePanel was never
reached. The panel stayed in the DOM with myIO-panel--open, the FAB kept
aria-expanded="true", and its document-level Escape/Tab focus trap stayed
installed. Opening a second chart's panel then left two competing traps live.

While a panel is open, observe the widget root with an IntersectionObserver
and close on a zero-area bounding box. That is the framework-agnostic
signal that an ancestor is display:none, and it does not fire for an element
merely scrolled out of view, which keeps a non-zero box. Focus is not
returned to the now-hidden button. Environments without IntersectionObserver
behave exactly as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
clearBrush() moved the brush to null, which re-dispatched d3's "end" event
with no selection and re-entered clearBrush(), recursing until the stack blew.
The unwind skipped the reset of chart.runtime._brushed, the status-bar removal,
and the emit("brushed", {keys: []}) that linked targets restore their opacity
from — so after fix(1) made propagation work, clearing a brush left every
target chart stuck dimmed and logged a RangeError.

Guard the re-entry so the inner call returns immediately and the outer one
completes. Same path backs the Escape key and the status bar's Clear button.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TreemapRenderer sized the d3.treemap layout to the entire plot rect
(chart.width - margin.left - margin.right), so like FunnelRenderer and
SankeyRenderer before defect 15 it painted every pixel of that rect --
including the top-right corner where the absolutely-positioned .myIO-fab
sits (style.css .myIO-fab, right:12px / top:8px in the non-narrow tier).
Measured live on Specialized > Treemap, the corner leaf rect overlapped
the button by 40.0 x 18.0 px: the full button width and 18px of its
height.

Apply the same reservation defect 15 introduced for funnel and sankey:
import FAB_GUTTER from layout/scaffold.js and subtract
Math.max(0, FAB_GUTTER - margin.right) from the layout width before
handing it to d3.treemap().size(). On the demo's 1144px container this
takes the layout from 1089 to 1038, leaving the rightmost leaf edge 4px
clear of the button -- exactly the clearance FAB_GUTTER (56 = 40 button
width + 12 right offset + 4) was sized for. No new option, no default
change, no transition change; the enter path still sets the final
geometry synchronously so speed 0 renders correctly.

A full FAB-overlap sweep of all 41 demo charts found no fourth instance
of this class -- treemap was the last full-bleed renderer in the
wide/top-right-FAB tier. Three out-of-class overlaps were found and are
reported separately rather than folded in here, since none is fixed by a
horizontal width reservation: narrow-tier bottom-right FAB over x-axis
tick labels (6 charts), addFAB() drawing a 40x40 button on 40px-tall
sparklines, and data points that happen to land in the top-right corner
of their own domain on three cartesian charts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
linkCharts() was completely inert -- no linkCharts() group has ever
cross-selected -- because of three stacked faults, all of which had to be
fixed for it to do anything.

(1) Token mismatch. R/linkCharts.R serialized mode = "bidirectional",
while linked.js gates its outbound brush handler on "source"/"both" and
its inbound subscription on "target"/"both". "bidirectional" satisfies
neither, so bindLinked() constructed a SelectionHandle and then bound
nothing. Verified live: with mode "bidirectional" the two demo charts
report _linkedBrushHandler falsy and a brushed emit dims 0 of 32 marks;
with "both" the handler binds and 30 of 32 dim. Fixed in both places. R
now writes "both", the same token setLinked()'s check_choice() accepts,
so the two entry points stop disagreeing about the same concept; JS
additionally accepts "bidirectional" as an alias, because widgets already
serialized by myIO <= 1.3.0 (saveWidget with a shared libdir, frozen
Quarto output, stored x$config JSON) carry the old token and must keep
working against the new bundle. Note this changes the value of the
internal serialized field x$config$interactions$linked$mode; it is not an
exported-API change, since linkCharts() has no mode parameter, but it is
observable in widget JSON and it required updating test_linkCharts.R.

(2) Missing bus. bindLinked() returned early when the crosstalk global
was absent, and linkCharts() -- unlike setLinked() -- attaches no
crosstalk dependency. So in linkCharts()'s own documented use case, a
static R Markdown or Quarto page, nothing bound even after (1). Rather
than retract the documented "does not require Crosstalk" promise, which
would have meant adding a hard runtime dependency on a Suggests package
to a function that has none and leaving linkCharts() a strictly inferior
setLinked(), linked.js now falls back to an in-page group bus that
implements the slice of crosstalk.SelectionHandle bindLinked() uses, with
crosstalk's dispatch semantics. It activates only when crosstalk is
absent and a group id is set, so no setLinked() widget can reach it and
crosstalk stays preferred on mixed pages.

(3) Wrong match key. The selection path matched on d._source_key, which
ensure_source_key() fills positionally per widget, so linkCharts(on =
"cyl") matched by row ordinal rather than by cyl. cfg.keyColumn already
held the right column name and the selection path never read it -- the
"shared group identifier and key column to coordinate selections" in the
roxygen was unimplemented. Outbound keys and inbound matching now route
through keyColumn when it is set. With keyColumn unset, i.e. every
setLinked() widget, the comparison is byte-identical to before.

Also documents in @details that cross-selection is brush-driven, so at
least one chart in the group must call setBrush().

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
composite_ridgeline() took its group order from unique(data[[mapping$group]]),
which returns first-encounter order. A ridgeline over mtcars with cyl coerced
to character therefore assigned baseline 1 to "6", 2 to "4" and 3 to "8", and
since baseline 1 sits at the bottom of the y scale the axis read 8, 4, 6 from
the top -- an order with no meaning to the reader and one that changes if the
rows are reordered.

Order the group values before assigning baselines. A factor's level order is an
explicit statement of intent and wins (ordered by as.integer(), never by
label); everything else sorts ascending, with character columns sorted via
method = "radix" so the C locale is used and the result does not vary with the
machine's collation. Ascending reads bottom-to-top, matching a discrete y axis
in ggplot2. NAs sort last, preserving today's presence/absence behaviour.

derive_positional_y_tick_labels() already sorts labels by numeric position, so
it needed no change. This is deliberately ridgeline-local: composite_violin,
composite_boxplot and composite_comparison have the same encounter-order
behaviour, but boxplot aligns its boxes, whiskers and medians positionally
against transform_quantiles()/transform_median() output, both of which are
user-reachable transforms, so reordering there requires a coordinated change
with its own review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rotated y-axis title's bounding box overlapped the leftmost y tick label by
2.48px on any chart combining the 50px default left margin with a wide y-axis
format. Two demo charts are affected -- Financial > Candlestick and Basic
Charts > Area -- and the brief's premise that this was candlestick-specific is
wrong: the driver is the y format plus the data magnitude, not the chart type.
Both use setAxisFormat(yAxis = "$,.0f") over three-digit data, whose widest
label "$100" measures 29.23px against a usable band of only 26.75px.

The geometry is fully determined: the tick labels' right edge sits
margin.left - 6.25 from the SVG edge (d3's 3px tickPadding against a negative
grid-line tick size, plus updateYAxis's dx="-.25em" = 3.25px at the 13px
.y-label size), and the rotated title's box ends at Y_AXIS_TITLE_INSET + the
3px font descent = 17. Raising that inset from 6 to 14 to keep the title from
being clipped at the SVG edge shrank the label band from 34.75px to 26.75px
without widening the margin that has to hold both.

Add fitLeftMargin() to layout/axes.js, called from renderCurrentLayers after
syncAxes. It measures the real rendered tick <text> nodes -- so it cannot drift
out of sync with updateYAxis's tick and format logic -- and grows margin.left to
fit them plus the title band. It is grow-only relative to a baseline stashed on
first render, so repeated renders converge in one pass instead of ratcheting,
and it short-circuits once the margin already fits. When it does fire it runs a
single bounded second pass (scaffold, scales, axes) before layers are routed,
rather than recursing, so beforeRender/afterScales do not fire twice. Sparklines
and suppressed axes are excluded.

setMargin() now records layout$marginSet, which suppresses the fit entirely, so
an explicit margin always wins. The flag is absent from a freshly constructed
config, so the default widget serialization is byte-identical and the schema is
unchanged.

Measured across the 44 demo widgets, only areaPlot and candlestickPlot move
(50 -> 57); every other default-margin chart's widest label already fits, and
the four setMargin() charts are excluded outright.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A grouped chart labels each series with the bare group value, so the
Grouped Bar legend reads "5 6 7 8 9" with nothing saying those are
months. The grouping column name was available at split time and was
even serialized as `mapping$group`, but no legend surface had anywhere
to put it: buildLayerLegendData() emitted only {key,label,color,...},
and the inline strip, the FAB panel and the export injector each drew
only swatch+label pairs.

`mapping$group` alone is not a usable carrier, because composite types
(boxplot, violin, ridgeline, comparison) also set it without expanding
one layer per group -- their legend entries are not group values. So
build_grouped_layers() and expand_grouped_df() now stamp a dedicated
`groupVar` marker on exactly those layers whose label IS a value of
that column; the grouped_df path only stamps when the expansion added
one layer per group, which skips composites.

New exported setter `setLegendTitle()` takes a literal string, or TRUE
to derive the title from the grouping column. The derived form is
deliberately conservative: it needs at least two entries all sharing
one grouping column, so a chart mixing grouped series with a
standalone fitted line stays untitled rather than mislabelled.

A single resolveLegendTitle() feeds all three legend surfaces, and
legendTitleText() is shared by the measurement path and every
renderer so a long title can never be drawn wider than it was
measured. On the inline strip the title is a measured row-0 lead
offset rather than its own row -- an extra row would steal 16px from
the plot area and push the strip over the x-axis labels on short
charts. Both placement call sites pass the same titleWidth, so the
1.3.0 one-surface contract still holds at the width boundary: a title
that will not fit relocates the whole legend to the panel instead of
clipping. suppressLegend() continues to hide everything.

The API is opt-in and every chart renders byte-identically until it is
called, so this is a backward-compatible minor addition. The demo app
now calls it on the six charts with grouped legends.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The inline legend strip and the centred x-axis title have always shared a
baseline band; only horizontal distance kept them apart. Adding a legend title
spent 80px of that distance on the narrowest titled chart (Theme Demo scatter,
516px wide), pushing the last entry under "Horsepower" — a 6.8x2.8px text
overlap plus two swatch rects.

Cap the legend's FIRST row at the axis title's left edge less a gap, so it
wraps rather than running on. Wrapped rows sit below the title and keep the
full width, so no chart is forced off the inline surface: the inline-legend
count across the 41 demo charts is unchanged at 13, and themeScatter is the
only chart whose layout moves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
composite_violin() took its x positions from unique(data[[x_var]]) and
composite_boxplot() from unique(x_values) inside transform_quantiles(),
both of which are data-encounter order. composite_comparison() inherits
boxplot's. Ridgeline was fixed in bc6e63f but the other three composites
were left on encounter order, so the same data frame produced a different
axis depending on which chart type you asked for, and the order changed
whenever the rows were re-sorted upstream.

Extract bc6e63f's ordering block into order_group_values() (factor level
order wins, else ascending, character sorting forced to the C locale, NAs
last) and call it from ridgeline, violin and boxplot. The ridgeline
substitution is byte-identical and stays covered by its existing tests.

transform_quantiles() and transform_median() are deliberately NOT
reordered: each carries meta$sourceKeys as a list parallel to its output
rows, and transform_outliers() returns filtered input rows rather than one
row per group, so sorting them would double the desync surface. Instead
composite_boxplot() now indexes their output by group NAME via match()
(quantile_idx / median_idx). A positional read after reordering would have
paired one group's box with another group's median -- silent data
corruption -- so the new test asserts each group's box, whisker and median
values are the ones computed from that group's own rows, using data whose
groups are three orders of magnitude apart.

Blast radius: rendered output changes for boxplot, violin and comparison
charts whose group values do not already appear in sorted order, with no
opt-out; on a violin passed a colour vector the hues re-map, since color is
applied by group position. No exported API change, no argument added or
renamed. Charts whose groups are already sorted -- including every factor
whose rows follow its levels, which covers all three demo charts -- render
exactly as before. Users wanting a specific order can supply a factor.

Note: composite_comparison's significance brackets get their positions from
a second encounter-order table in transform_pairwise_test(); that is fixed
in the immediately following commit and the two must ship together.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
transform_pairwise_test() built its own group-position table from
unique(as.character(x_values)) -- a second, independent data-encounter
ordering, and one that discarded factor level order outright by coercing
before uniquing. composite_comparison() draws its boxes through
composite_boxplot() and its brackets through this transform, so once the
boxes were ordered deterministically the brackets kept pointing at the
old encounter-order positions: a bracket labelled "C vs A" would span the
boxes for A and B. This is the mis-pairing the preceding commit's ordering
change would otherwise have introduced, and the two must ship together.

Both position tables now come from order_group_values(), so bracket
endpoint x1/x2 always resolve to the tick carrying that bracket's own
group1/group2. Everything downstream already keyed off names, so this is a
one-line change.

Side effect: combn() enumerates pairs in sorted order now, and order(spans)
breaks ties by that index, so brackets of equal span may stack at different
vertical levels than before. Cosmetic, and identical for any input whose
groups were already sorted -- including iris, which all three demo charts
use, so their x1/x2/group1/group2 are unchanged.

The new test asserts every bracket endpoint lands on the tick that carries
its own group name; it fails with six errors against a tree that has the
composite ordering fix without this one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
composite_boxplot() built the minmax whisker table with
do.call(rbind, <list of named numeric vectors>), which yields a matrix, then
read it with whisker_df$whisker_low. `$` is invalid on an atomic vector, so
the documented whiskerType = "minmax" option (vignettes/chart-types.Rmd:342)
failed 100% of the time with "$ operator is invalid for atomic vectors".
Nothing covered it -- there was no whiskerType test anywhere.

Read the two columns with whisker_df[, "whisker_low"] / [, "whisker_high"]
instead, which works against both shapes the branch produces: the minmax
matrix and the tukey data.frame row-subset. No change to the default tukey
whiskers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R/setLinked.R has always serialized the SharedData key vector into
config.interactions.linked.key, but no browser-side module read it. The
linked-brushing code matched on `_source_key`, which R/util.R
ensure_source_key() fills positionally as "row_<i>" -- a myIO-private id
space. Both directions therefore spoke row numbers: brushing a myIO chart
put row_1..row_N onto the Crosstalk group where a sibling DT/plotly/leaflet
was speaking row names or an id column, and an inbound selection carrying
real keys matched no mark at all. Two myIO charts stayed self-consistent
only because both ends were equally wrong.

buildKeyMap() now turns cfg.key into a row_<i> -> real-key lookup, built
once per bindLinked() (so it is rebuilt whenever layer data is swapped) and
threaded through linkKeys(), matchKey(), applySelection() and applyFilter().

Reconciled with the three paths dcbcc7d added: cfg.keyColumn still takes
precedence and short-circuits the map entirely, so linkCharts() column
matching is untouched; the LocalSelectionHandle bus carries translated keys
identically because translation happens above the handle.

Backward compatibility is explicit rather than incidental. An absent or
empty cfg.key (a widget serialized by an older myIO, or linkCharts()) yields
a null map and the legacy row_<i> space is used verbatim. If any linkable
layer's row count differs from length(cfg.key) the positional pairing does
not hold, so translation is disabled for the whole chart rather than
mislabelling marks -- fail-closed, and byte-identical to the old behaviour.
No warning is emitted on that path: it is pre-existing behaviour and the
console baseline must stay at zero.

The R-facing input$'myIO-<id>-brushed'$keys is deliberately unchanged --
translation happens downstream of the emit, inside linked.js.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two distinct states collapsed into one message. onBrushEnd fires for a live
brush rectangle that happens to contain no points and emits keys: []; and
clearBrush fires for a removed brush and also emits keys: []. The linked
handler only inspected keys.length, so both landed on sel.clear().
clear() broadcasts null, which applySelection() reads as "no selection" and
restores opacity 1 on every mark of the target. The source meanwhile kept
the dimming onBrush() applied during the drag and which onBrushEnd never
touches. Result: source fully dimmed, target fully bright, for the same
state.

The receiving half was already correct -- Crosstalk propagates [] as a value
distinct from null, and applySelection()'s indexOf branch already dims every
mark for a present-but-empty array. The defect was entirely the outbound
branch choosing clear() over set([]).

brush.js now labels the payload: onBrushEnd emits active: true (a rectangle
exists on screen, whatever it covers), clearBrush emits active: false. The
linked handler branches three ways on that flag instead of guessing from
keys.length. `extent` was not usable as the discriminator -- band and ordinal
scales make invertExtent return {x: null, y: null}, which is still truthy.

The clear path is deliberately untouched: clearBrush keeps the
_brushClearing re-entry guard from 34eb990, its opacity reset, the
_brushFn.move(null) and the emit that 85bfb4b relies on; only the payload
gains a field. The branch tests `e.active === true` strictly, so a
hand-emitted "brushed" payload from user code with no `active` field keeps
today's clear() behaviour. LocalSelectionHandle.set([]) broadcasts []
identically, so the Crosstalk-free linkCharts() path gets the same semantics.

Blast radius: every linked target chart now dims while the source holds an
empty brush, which is user-visible on any existing setLinked()/linkCharts()
page. The new `active` field is additive on the documented
myIO-{id}-brushed Shiny input; both vignette tables are updated. No exported
R API or argument changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fitLeftMargin ran once, before routeLayers. groupedBar's stacked layout
builds its own y scale (groupedBarHelpers.js transitionStacked) and pushes
it through updateYAxis afterwards, so the stacked totals formatted wider
tick labels than the fit had measured and they ran back through the
rotated y-axis title. Two entry points: toggleGroupedLayout, which never
goes through renderCurrentLayers at all, and any redraw while already in
stacked mode, where routeLayers runs after the fit block.

A synchronous re-fit could not simply re-read the DOM: updateYAxis applies
the generator to a d3 transition on every non-initial render, and
transition.text() is a tween, so the new strings are not in the DOM until
the first animation frame. updateYAxis now stashes the strings the
generator is about to render (resolved the same way d3-axis resolves them)
and fitLeftMargin measures those, falling back to the DOM for any caller
that renders an axis without updateYAxis. The suppressed-axis branch
clears the stash so it cannot go stale.

measureLabelWidth gains an optional class argument so the probe inherits
the real .y-label font rather than the UA default.

Both re-fits are bounded, not loops: fitLeftMargin is a pure function of
the measured labels and the stashed baseline, so a second pass over the
same scale computes the same target and returns false.

Also folds in item G's characterisation test. Item G (the fit landing
within 1px of its threshold on three demo charts) was measured and found
not to be a defect -- DPI/zoom variance is 0.006px and the font stack moves
the fit by up to 8px in the correct direction -- so the fit is unchanged.
The test pins fitLeftMargin's purity so nobody later "hardens" it into
path dependence, which would break the convergence this commit relies on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mortonanalytics and others added 11 commits July 29, 2026 12:38
Chart.renderCurrentLayers returns early for facets, before it reaches
fitLeftMargin, and FacetPanel.renderPanel called syncAxes directly without
ever running the fit. A faceted chart with a wide y format therefore kept
the configured 50px left margin and drew its tick labels through the
rotated axis title and past the SVG's left edge, where the identical
unfaceted chart grew the margin and laid out correctly.

Calling fitLeftMargin on the panel pseudo-chart as-is would have been
wrong: buildPanelChart shallow-copies the parent config, so
panelConfig.layout IS the parent's layout object and its margin IS the
parent's margin. The fit mutates config.layout.margin.left, so it would
have written into the parent and through it into every sibling panel --
which the new sibling test and the existing uniform-geometry test both
catch. Each panel now gets its own layout object pointing at the margin
instance buildMargin already returns per panel, restoring the
one-object invariant (config.layout.margin === chart.margin) that
fitLeftMargin and derive/scales.js assume.

layout.marginSet rides along on the copy, so an explicit setMargin()
suppresses the panel fit exactly as it does for a non-faceted chart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Financial > Waterfall was the last console warning in the demo set:
"[myIO] Layer 'Revenue Bridge' field 'value' contains 1 null/NaN values."
The validator was right -- the serialised layer genuinely carried
value: null on the total row -- so the fix is the cause, not the check.

transform_cumulative consumes the NA that declares a total row: it zeroes
it so the running cumsum lands on the right answer, then overwrites that
row's cumulative with the final total and anchors its base at 0. But it
copied the input frame verbatim, so the y_var column still held the NA it
had already resolved. The transform now writes the resolved magnitude back
-- cumulative minus base for total rows, the zeroed value elsewhere -- so
the payload ships a number.

That fixes a second, previously unreported symptom of the same cause:
WaterfallRenderer.formatTooltip concatenates the raw y value, so hovering
the End bar rendered the literal string "Delta: null, Total: 110". Both are
covered by tests.

Non-total rows are deliberately not recomputed as cumulative - base: that
is algebraically the same number but introduces floating-point drift
(0.2 becoming 0.20000000000000004) which would change tooltip text on
existing charts.

Blast radius: the value column of a waterfall layer's serialised data, and
therefore its exported CSV, changes -- a total row's cell now holds the
total rather than being empty, and an NA elsewhere reads 0, which is the
height it has always been drawn at. No rendering change, no exported R API
change; transform_cumulative is internal.

derive/validate.js is deliberately untouched -- special-casing _is_total
there would suppress a genuine class of user error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…om.element

The reported Sankey asymmetry is not a defect: every renderer in src/
draws with chart.element.id, and 12 of the 13 that use an id when
cleaning up read chart.dom.element.id. SankeyRenderer follows both
conventions and is not the outlier.

BracketRenderer.remove was the one exception, reading the chart.element
alias. chart.element is re-derived from chart.dom.element by
syncLegacyAliases on every mutation; chart.dom.element is assigned once
in the constructor and never reassigned. Today the two are the same node
so this is a runtime no-op, but a remove() that reads the alias fails
silently the moment anything reassigns dom.element without re-syncing --
the selector just matches nothing and the marks are orphaned. The
convention is now 13 of 13 and greppable.

Three tests: a red-then-green bracket test that renders under the live
element then staleness the alias before removing; a lint-style guard over
every renderer's remove() body that names the offending file (it reported
BracketRenderer.js before this change); and a characterisation test
pinning chart.element === chart.dom.element across construct, sync,
capture, re-render and updateData. That last one passes today by
construction -- it exists to make the invariant that keeps the
render/remove asymmetry harmless explicit and enforced.

The render()-side chart.element.id usages are deliberately untouched: a
28-file rename with no behavioural benefit.

Also extracts the jsdom SVGGraphicsElement.transform shim added in
fix(E) into tests/js/support/. jsdom does not implement it, so any test
that renders an axis through a transition throws on the first animation
frame; this commit's lifecycle test is the third caller, which is what
justified pulling it out of the two test files that had it inline.

No user-visible behaviour change, so no NEWS entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
inst/myio-schema.json and mcp/myio-schema.json are byte-identical by
convention only -- writeSchemas() emits one JSON string to both paths --
and nothing compared them. mcp/lib/validate.mjs loads the mcp copy at
runtime, so a hand-edit or a partial regen would leave the MCP validator
running an older contract with the whole suite still green.

The existing freshness test cannot cover this. It reads only inst/, and
it is test.skipIf(!hasRscript): no workflow that runs vitest installs R
(js-coverage.yaml is checkout/setup-node/npm ci/test), R-CMD-check.yaml
runs no npm, and e2e.yaml runs playwright rather than vitest. So it is
skipped in 100% of CI runs and the drift guard was absent entirely.

A filesystem comparison needs no R and therefore never skips, so it runs
on every push and PR through the existing js-coverage workflow. Verified
red-then-green by perturbing the mcp copy and restoring it.

Not done here, and left as a separate decision: adding an
r-lib/actions/setup-r step to js-coverage.yaml so the freshness test
stops skipping. That costs 1-2 min of CI per run and is not needed to
close the drift-between-copies gap this test covers.

Test-only; no source, workflow or NEWS change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
addFAB() was called unconditionally from renderCurrentLayers() and
addButtons(), and its only early return was the empty-chart guard.
applySparklineOverrides() strips margins, legend, axes, brush, annotation,
linked, sliders, drag, reference lines and ARIA, but never touched the
button, so every sparkline got a 40x40 overlay on a 60px inline chart --
about 8% of the widget, sitting on top of the last data points and
intercepting the pointer over that corner so those points had no tooltip.

Guard on chart.config.sparkline inside addFAB() rather than at the call
sites, so renderCurrentLayers(), addButtons() and any future caller are
covered once. clearEmptyState()/renderEmptyState() select .myIO-fab and
set display, which is a no-op on an empty selection.

Blast radius: the export menu (CSV, PNG, SVG, PDF, clipboard) is no longer
reachable from a sparkline. The panel's legend was already suppressed in
sparkline mode -- resolveLegendPlacement returns {inline:false, panel:false}
-- so no legend surface is lost, and this matches the documented sparkline
contract of stripping axes, legend and all interactions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.myIO-fab is position:absolute right:12px bottom:12px, and only
.myIO-container:not(.myIO-container--narrow) lifted it to top:8px. A
container 600px wide or less therefore kept the button in the band
[height-52, height-12], which is inside the default 60px bottom margin
where the x tick labels live -- measured overlaps on all six Theme Demo
charts and on both Linked Brushing charts.

The bottom band cannot be salvaged: at narrow widths the x tick labels are
rotated -65deg, the x-axis title is centred under them and the inline legend
sits below that, so there is no free 40px strip along the bottom edge.
FAB_GUTTER is a horizontal reservation and does nothing for a vertical
collision. Pinning the button to the top on both tiers is the only placement
with a free band, but top:8px + 40px tall ends at y=48 against a default
margin.top of 30, so the CSS move alone leaves it over the plot -- a rising
series that peaks at the last x still hits it.

So pair the CSS change with fitTopMargin(), which reserves the band as top
margin for narrow axes charts. It mirrors fitLeftMargin exactly: no-op once
setMargin() has been called, exempt for sparklines, and computed against a
stashed baseline so the margin goes back when the container widens. The
state.axesChart gate is deliberate -- funnel, sankey and treemap already
vacate the whole right band via FAB_GUTTER, which covers the button at any
height. In Chart.js the two fits are called into locals rather than || so
the second is not short-circuited away.

The button stays hit-testable and is still the only route to the legend on
panel-legend charts; only its corner moves.

Blast radius: a narrow axes chart that has not called setMargin() gains 18px
of top margin and loses 18px of plot height. Wide charts, setMargin() charts,
sparklines and non-axes charts are unchanged, and defect 15/18's FAB_GUTTER
work is untouched. Residual, measured at zero occurrences across the demo
set: on the wide tier the button still overlays plot y 30-48 in the top-right
corner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The funnel caps its own geometry with fabGutter so no stage is drawn under
the button, but placeValueLabel tested a label that no longer fits inside
its trapezoid against the FULL plot width. That width extends fabGutter px
further right than the band the stages were pushed out of, so an outside
value label could be painted beneath the button.

The fix is y-aware, not a blanket horizontal cap. A blanket cap would
suppress labels further down the funnel that are nowhere near the button and
were rendering correctly. Instead the label's own text box decides: a label
whose box sits below the button's band keeps the full plot width, and only
one that actually overlaps the band stops at fabLeft.

The identity that makes it correct in both directions: fabBandBottom is
FAB_BAND_BOTTOM - margin.top, so once margin.top reaches 48 the band is
above the plot, fabBandBottom goes negative, every label clears it and the
full-width limit is always used. In practice only the first stage can ever
be capped, and only when the top margin is small enough for its label to
reach the band.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
twoLine was the only gate on value visibility: as soon as a stage band
dropped below 34px every value went to fill-opacity 0, including values that
would have fitted perfectly well outside the trapezoid on the stage name's
own baseline. The chart gave no indication values existed -- only the
tooltip carried them. It triggers whenever the plot height is under 40px per
stage, so any funnel with more than a handful of stages in a normal-height
container.

Replace the boolean with a three-tier valueLayout:
  >= 34px  two lines -- name above the band's centre, value below it
  >= 18px  one line  -- name centred, value just outside the right edge
  <  18px  name only -- the value stays on the tooltip

The one-line tier reuses the existing outsideX/outsideFits machinery rather
than concatenating name and value into one string: no re-measurement of a
combined string, it cannot overlap the centred name because outsideX is
beyond the trapezoid's right edge by construction, and it inherits the
button-awareness added in the previous commit for free.

18px is the 12px value type's own extent plus enough clearance that two
adjacent stages' outside values cannot touch -- a band of 18 means a stage
pitch of 24 at the default 6px gap, against about 16.5px of text.

showValues = FALSE is unchanged: valueLayout "none" reproduces it exactly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every flow label was placed at the midpoint between the two columns and its
visibility decided by two per-link rules only -- ribbon thickness and the
column gap being wider than the text. Neither rule looks at any other label,
and all links in the same column gap share an identical x, so two links whose
endpoint means coincide are painted on exactly the same point. Reproduced at
the renderer's own layout parameters with the crossing pattern
A->X 50, A->Y 14, B->X 18, B->Y 50: A->Y and B->X both land at (129, 105)
and both clear the thickness and span gates, so "14" and "18" were drawn on
the same pixel.

Decide the keep/drop set once, up front, in a single greedy pass over
graph.links in array order: first label placed wins, later ones whose box
intersects it are dropped. d3-sankey preserves input link order, so the
result is deterministic and stable across re-renders regardless of DOM
enter/update order -- no jitter and no layout solver. The attr callback
becomes a lookup.

Deliberately flow-vs-flow only, not flow-vs-node-label: the closest such
pair in the demo set clears by 1.4px vertically, so including node labels
with any y padding would suppress a label that renders correctly today. The
x padding is kept at 2px and the half-height at 7px for the same reason.

The measuring probe is appended and removed within the same tick, mirroring
measureLabelWidth, so it cannot leak into exported SVG. A suppressed value
is still on the link's tooltip and in the data-table fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(C) reserved the button's top-right band as top margin only on the narrow
tier, on the reasoning that no wide demo chart had a mark in that corner. That
made the defect data-dependent rather than fixed: a sweep of the 41 demo charts
found regressionPlot drawing a data point 6.5x4.4px underneath the button.

The button is pinned to the same corner on both tiers, so reserve the band on
both. After the change the sweep finds no painted mark under the button on any
chart — only the brush capture rect, which is transparent by design.

Costs 18px of plot height on wide axes charts that have not called setMargin().

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The re-fit in toggleGroupedLayout grew margin.left and called draw(), which
routes to transitionStacked/transitionGrouped. Those only touch the y axis
itself and never re-run renderAxisTitles, so the plot group moved to the new
margin while the rotated title stayed at the old one — the tick labels still
ran through the title, just 10px further right. Live measurement: clearance
-5.83px after the fix, versus -5.82px before it.

Call syncAxes before the redraw, matching the re-fit in renderCurrentLayers.

Also trims a chart title that is too long for its container, which could
otherwise render underneath the legend button now that the button is
top-anchored on every width tier. Measured on a 326px container: the title
overlapped the button by ~30px.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mortonanalytics
mortonanalytics merged commit d46b777 into main Jul 29, 2026
11 checks passed
@mortonanalytics
mortonanalytics deleted the fix/visual-review-defects branch July 29, 2026 21:05
mortonanalytics added a commit that referenced this pull request Jul 29, 2026
1.3.0 was tagged but never submitted to CRAN, so the visual-review fixes
merged in #102 ship as part of that release rather than a follow-on. Folds
the (development version) section into 1.3.0, merging the two "New features"
lists and carrying "Bug fixes" and "User-visible changes" over intact. All
131 entries preserved; no version change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mortonanalytics added a commit that referenced this pull request Jul 29, 2026
Re-measured against the final .Rcheck install: 4.2 MB total, 2.5 MB in
htmlwidgets/. The prior 4.1/2.4 figures predated the #102 merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant