Skip to content

[WC-3594] Charts: keep design preview within its parent - #2473

Open
yordan-st wants to merge 2 commits into
mainfrom
fix/WC-3594_chart-preview-overlaps-widgets
Open

yordan-st wants to merge 2 commits into
mainfrom
fix/WC-3594_chart-preview-overlaps-widgets

Conversation

@yordan-st

Copy link
Copy Markdown
Contributor

Pull request type

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


Description

The design mode preview of the chart widgets ignored the width and height settings and always rendered at a fixed 300×232px (385px wide with legend) inside an inline-flex wrapper. In a parent narrower than that, for example a layout grid column with a manual width, the preview spilled over neighbouring widgets.

The shared ChartPreview now applies width unit, width, height unit and height the same way the runtime does, and is capped at 100% of its parent, so it follows the configuration and never grows past its container. This covers the area, bar, bubble, column, heatmap, line, pie/doughnut and time series charts. Custom chart has its own preview and is not changed.

Note: the recording on the ticket was made with the legacy Charts module (1.4.11, com.mendix.widget.custom.*), which is not in this repo and is not affected by this change.

Ticket: https://mendix.atlassian.net/browse/WC-3594

No XML changes, no docs PR needed. Changelog entries added to the eight affected chart widgets.

What should be covered while testing?

All checks are in Studio Pro design mode, no need to run the app.

  1. Add a layout grid with four columns and set one column to a manual width. Place a chart in it: the preview stays inside the column, with and without legend.
  2. Width: with unit Percentage, set 100 and 50, the preview takes that share of its parent. With unit Pixels, set a value larger than the parent, the preview stops at the parent edge.
  3. Height: check Percentage of width (height follows the width), Pixels and Percentage of parent.
  4. Toggle "Show playground slot" and check the preview still stays within its parent.
  5. Repeat a quick check for each chart type: area, bar, bubble, column, heatmap, line, pie/doughnut, time series.
  6. Custom chart preview is unchanged.

@yordan-st yordan-st changed the title [WC-3594]: Keep chart design preview within its parent [WC-3594] Charts: keep design preview within its parent Oct 13, 2026
@yordan-st
yordan-st marked this pull request as ready for review October 13, 2026 12:42
@yordan-st
yordan-st requested a review from a team as a code owner October 13, 2026 12:42
@yordan-st
yordan-st force-pushed the fix/WC-3594_chart-preview-overlaps-widgets branch from dc7d9d8 to 89b207d Compare October 13, 2026 12:42
Comment thread packages/shared/charts/src/components/ChartPreview.tsx Outdated
@yordan-st
yordan-st force-pushed the fix/WC-3594_chart-preview-overlaps-widgets branch from 26b9e3f to 03edcd8 Compare October 14, 2026 09:32
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@yordan-st
yordan-st force-pushed the fix/WC-3594_chart-preview-overlaps-widgets branch from 03edcd8 to 6bfc334 Compare October 22, 2026 09:06
@github-actions

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/components/ChartPreview.tsx Added widthUnit, width, heightUnit, height props; replaced fixed-size inline styles with getPreviewDimensions; outer wrapper changed from inline-flex to flex with maxWidth/maxHeight: 100%
packages/shared/charts/src/components/__tests__/ChartPreview.spec.tsx New unit test file covering dimension logic, image shrink, and legend toggle
packages/pluggableWidgets/*/CHANGELOG.md × 8 [Unreleased] ### Fixed entry added to each affected chart widget

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

CI note: build jobs were CANCELLED (not failed — the run was superseded by the in-progress run). Snyk security check is FAILURE; code and license checks are SUCCESS. The Snyk security failure predates this PR (no new packages or deps were introduced) and is likely a pre-existing finding unrelated to these changes.


Findings

⚠️ Low — width: 0 and height: 0 are silently skipped

File: packages/shared/charts/src/components/ChartPreview.tsx lines 60, 64
Problem: The guards use truthy checks (if (props.width) / if (props.height)), so a value of 0 is treated the same as null — no width/height style is applied. In the Mendix XML the defaults are non-zero, but a user could type 0 in Studio Pro and get an unexpected result (the chart renders at its natural size rather than collapsing or showing a clear error). The runtime widget itself likely has the same edge-case, but the preview should be consistent.
Fix:

if (props.width !== null && props.width !== undefined) {
    style.width = props.widthUnit === "percentage" ? `${props.width}%` : `${props.width}px`;
}
if (props.height !== null && props.height !== undefined) {
    // ...
}

⚠️ Low — test missing a case for width: null / height: null (no dimensions set)

File: packages/shared/charts/src/components/__tests__/ChartPreview.spec.tsx
Problem: getPreviewDimensions has explicit if (props.width) / if (props.height) branches for null inputs, but there is no test verifying that passing null for both leaves style.width and style.height empty. This makes the null-path invisible to regressions.
Fix: Add a case:

it("applies no explicit dimensions when width and height are null", () => {
    const chart = renderChart({ width: null, height: null });
    expect(chart.style.width).toBe("");
    expect(chart.style.height).toBe("");
    expect(chart.style.aspectRatio).toBe("");
});

⚠️ Low — outer wrapper still grows to the height of its children

File: packages/shared/charts/src/components/ChartPreview.tsx line 19
Problem: The outer <div> has maxHeight: "100%" but no explicit height or overflow: hidden. When heightUnit is pixels and the configured height is large, the outer wrapper grows to accommodate the inner Chart div (whose pixel height is uncapped by the parent max-height), which may still spill in some browser layout contexts. The inner Chart div has overflow: hidden, so this is a belt-and-suspenders note rather than a confirmed breakage.
Note: Consider adding overflow: hidden to the outer wrapper as well, mirroring the inner Chart div.


Positives

  • The getPreviewDimensions helper cleanly mirrors the three heightUnit enum values from the XML, making the logic easy to verify against the widget's own runtime behaviour.
  • Replacing the fixed 300px/385px/232px constants with flex: "1 1 0" on PlotImage and flex: "0 1 85px" on PlotLegend is the right approach — the image naturally fills available space instead of overflowing.
  • Changelog entries are user-facing and accurately describe the observable behaviour change without exposing implementation details.
  • The renderChart factory helper in the test file follows the repo convention (shared defaultProps-equivalent + spread overrides), keeping individual test cases minimal.
  • All eight affected widget CHANGELOGs were updated consistently, and custom-chart-web (which has its own preview) was correctly left untouched.

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.

2 participants