Skip to content

[WC-3471]: fix(image-web): migrate image source properties to allowUpload#2354

Open
yordan-st wants to merge 1 commit into
mainfrom
feat/WC-3471_image-allowupload
Open

[WC-3471]: fix(image-web): migrate image source properties to allowUpload#2354
yordan-st wants to merge 1 commit into
mainfrom
feat/WC-3471_image-allowupload

Conversation

@yordan-st

Copy link
Copy Markdown
Contributor

Pull request type

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


Description

imageObject and defaultImageDynamic in the Image widget were declared as type="image" without allowUpload, which Mendix has deprecated — DynamicValue<WebImage> is removed in Mendix 12, and Studio Pro 11.8+ already shows a design-time deprecation warning for this pattern.

This adds allowUpload="true" to both properties and regenerates typings accordingly (EditableImageValue<WebImage>). The widget's read logic in Image.tsx is unchanged — EditableImageValue exposes the same status/value.uri shape the widget already consumes, so this is a display-only migration with no new upload UI.

marketplace.minimumMXVersion is bumped to 11.12.0 since the widget XML now requires allowUpload support, which older Studio Pro versions cannot parse (verified: Studio Pro <11.8 fails to load the widget package with "The 'allowUpload' attribute is not declared").

Manually verified in Studio Pro 11.12 that both Static and Dynamic image-source configuration remain available in the property editor after this change — existing apps using a static image are not broken.

Widget version is not bumped in this PR (bumped at release time per repo convention); this change will require a major bump (2.0.0) on release due to the minimumMXVersion increase.

What should be covered while testing?

  • Existing Image widget configurations (static image, dynamic/attribute-bound image, icon, image URL datasources) continue to render identically.
  • No upload/file-picker UI appears anywhere on the widget.
  • Widget can be added to a page in Studio Pro 11.12+; "Image source" and "Default image" properties still offer both Static and Dynamic configuration.

@yordan-st
yordan-st requested a review from a team as a code owner July 23, 2026 13:50
@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/pluggableWidgets/image-web/src/Image.xml Added allowUpload="true" to imageObject and defaultImageDynamic properties
packages/pluggableWidgets/image-web/typings/ImageProps.d.ts Regenerated: DynamicValue<WebImage>EditableImageValue<WebImage> for both image props
packages/pluggableWidgets/image-web/src/__tests__/Image.spec.tsx New unit test file covering container-level image rendering
packages/pluggableWidgets/image-web/CHANGELOG.md [Unreleased] entry added under ### Fixed
packages/pluggableWidgets/image-web/package.json marketplace.minimumMXVersion bumped 9.24.0 → 11.12.0
openspec/changes/fix-image-allow-upload-deprecation/ OpenSpec planning artifacts (proposal, tests, schema)

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

⚠️ CI check results could not be retrieved during this review — verify all checks are green before merging.


Findings

⚠️ Low — proposal.md documents wrong generic type

File: openspec/changes/fix-image-allow-upload-deprecation/proposal.md line 8
Note: The proposal says the typings change from DynamicValue<WebImage> to EditableImageValue<ImageValue>, but the actual generated ImageProps.d.ts and the Mendix API both use EditableImageValue<WebImage>. The runtime code and typings are correct; only the design doc is inconsistent. Worth correcting before this change is archived.


⚠️ Low — New container tests don't cover ValueStatus.Loading

File: packages/pluggableWidgets/image-web/src/__tests__/Image.spec.tsx lines 43–75
Note: The test guidelines require all Mendix data states to be covered (Available, Loading, Unavailable). The new spec covers Available and Unavailable but omits Loading. In Image.tsx a Loading imageObject silently falls through to an empty src — a test pin here would prevent regressions if that fallback is ever changed.

it("renders no image src when imageObject is Loading", () => {
    const { container } = render(
        <Image {...makeProps({ imageObject: makeImageValue({ status: ValueStatus.Loading, value: undefined }) })} />
    );
    const image = container.querySelector("img");
    expect(image?.getAttribute("src")).toBeFalsy();
});

⚠️ Low — Manual mock object instead of repo builders

File: packages/pluggableWidgets/image-web/src/__tests__/Image.spec.tsx lines 8–19
Note: The repo convention (per AGENTS.md and the test guidelines) is to use builders from @mendix/widget-plugin-test-utils rather than hand-rolled mock objects. EditableImageValue isn't directly covered by an existing builder, but the manual helper is fragile — if the EditableImageValue interface gains a new required field (e.g. Mendix 12 migration) the tests will silently cast around the gap via as ImageProp. At minimum, add a // EditableImageValue builder not yet available in test-utils comment so future authors know this was a conscious choice, or open a follow-up to add the builder to the shared package.


Positives

  • Surgical change: only XML and generated typings modified; Image.tsx runtime logic left completely untouched — no risk of regressions in rendering or click behaviour
  • CHANGELOG entry correctly calls out the raised minimum Mendix version, giving upgraders an immediate heads-up about the breaking floor change
  • New container test file uses the factory-helper pattern (makeImageValue / makeProps) consistently — tests are readable and focused
  • allowUpload="true" is the minimal, Mendix-prescribed fix for the deprecation; no new upload UI was introduced, keeping widget scope unchanged
  • PR description documents manual Studio Pro 11.12 verification and explains why the Static image-source option is preserved — useful context for reviewers and future readers

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