Skip to content

[WC-3612]: Fix axis title config shape in charts - #2470

Open
r0b1n wants to merge 2 commits into
mainfrom
fix/WC-3612-axis-title-config
Open

r0b1n wants to merge 2 commits into
mainfrom
fix/WC-3612-axis-title-config

Conversation

@r0b1n

@r0b1n r0b1n commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Pull request type

Bug fix (non-breaking change which fixes an issue)

Description

Fixes the axis title structure in the generated chart layout configuration, so axis titles are passed through as { text: "..." } instead of being wrapped an extra level.

Adds unit tests covering the shape of the generated layout, config and series options.

What should be covered while testing?

  • Set X-axis and Y-axis labels on a chart widget and verify they render correctly.
  • Verify charts without axis labels still render as before.

@r0b1n
r0b1n requested a review from a team as a code owner October 8, 2026 14:50
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/shared/charts/src/utils/configs.ts Removed double-wrapping of axis label: { text: xAxisLabel } → xAxisLabel directly
packages/shared/charts/src/utils/__tests__/configs.spec.ts New unit test file covering layout, config, and series option merging
packages/pluggableWidgets/bubble-chart-web/e2e/…-snapshots/*.png Updated visual baselines (2 files)
packages/pluggableWidgets/line-chart-web/e2e/…-snapshots/*.png Updated visual baselines (3 files)

Skipped (out of scope): dist/, pnpm-lock.yaml

CI checks: could not fetch automatically — please confirm all checks pass before merging.


Findings

⚠️ Low — Missing CHANGELOG entries for bubble-chart-web and line-chart-web

Files: packages/pluggableWidgets/bubble-chart-web/CHANGELOG.md, packages/pluggableWidgets/line-chart-web/CHANGELOG.md
Note: Both widget packages had their E2E visual baselines updated because the axis label rendering changed (the core fix lives in shared/charts, but it is observable behaviour in these widgets). The [Unreleased] sections in both CHANGELOGs are empty. A user-facing bug fix entry belongs there. The shared/charts package is private and has no CHANGELOG, so no entry is needed there.
Fix: Add a ### Fixed entry under [Unreleased] in each widget's CHANGELOG.md, e.g.:

### Fixed

- We fixed an issue where X-axis and Y-axis labels were not rendered correctly on charts.

Positives

  • The root cause is cleanly identified: callers already pass { text: "…" } objects (confirmed across all seven widget consumers), so the extra wrapping in getCustomLayoutOptions was creating { text: { text: "…" } } — the two-line fix is exactly the right scope.
  • The new unit tests directly assert the shape of the generated Plotly Layout object at each level of merging (getCustomLayoutOptions, getModelerLayoutOptions, getModelerConfigOptions, getModelerSeriesOptions), closing the gap that allowed this bug to go undetected.
  • Parametrised it.each for the four gridLinesMode values is a clean and exhaustive approach.
  • Updated visual baselines are committed alongside the fix — reviewer can visually verify the label rendering is now correct.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant