Skip to content

[WC-3448]: Add datamatrix option for Barcode Generation#2319

Open
samuelreichert wants to merge 11 commits into
mainfrom
WC-3448/add-datamatrix-generation
Open

[WC-3448]: Add datamatrix option for Barcode Generation#2319
samuelreichert wants to merge 11 commits into
mainfrom
WC-3448/add-datamatrix-generation

Conversation

@samuelreichert

Copy link
Copy Markdown
Contributor

Pull request type

New feature (non-breaking change which adds functionality)


Description

@samuelreichert
samuelreichert force-pushed the WC-3448/add-datamatrix-generation branch from 431b188 to d9b57b4 Compare July 10, 2026 15:26
@samuelreichert
samuelreichert marked this pull request as ready for review July 16, 2026 07:40
@samuelreichert
samuelreichert requested a review from a team as a code owner July 16, 2026 07:40
@github-actions

This comment has been minimized.

samuelreichert and others added 11 commits July 20, 2026 10:00
Adds the tree-shakeable bwip-js browser entry, the only maintained
library with native GS1 Data Matrix support, for upcoming Data Matrix
generation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds "Data Matrix" to the top-level barcode format and a new
"Advanced Data Matrix Settings" group (GS1 mode toggle, square/rectangle
shape, size). Regenerates the widget prop typings to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds DataMatrixTypeConfig to the BarcodeConfig union with a
format === "DataMatrix" branch in barcodeConfig(), plus a DataMatrix
value check and a loose GS1 Application Identifier syntax validator.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds DataMatrixRenderer as a third render path. Selects the bwip-js
encoder by GS1 mode and shape (datamatrix / gs1datamatrix /
*rectangular), renders inline SVG, and reuses the existing SVG->PNG
download pipeline. Wires the dispatch in BarcodeGenerator and the
download filename prefix for the datamatrix type.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Renders a representative Data Matrix glyph in the Studio Pro page
editor when the Data Matrix format is selected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds config-mapping and validation unit tests plus render tests
asserting encoder selection (plain vs GS1, rectangular) and the
malformed-GS1 error path. Mocks bwip-js and adds the new required
Data Matrix props to the shared test props builder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Proposal, design, specs, and task checklist for adding Data Matrix
and GS1 Data Matrix generation to the barcode generator widget.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move openspec change/archive folders under each widget's own
package directory instead of the repo-root openspec/ folder.
bwip-js SVGs carry only a viewBox, no width/height attributes, so the
rendered element defaulted to intrinsic sizing instead of the
configured size. Derive pixel dimensions from the viewBox and apply
them via inline style.
…class, tighten memo deps

Data Matrix code value comes from a Mendix DynamicValue and reaches the
DOM via dangerouslySetInnerHTML; sanitize the bwip-js SVG output with
DOMPurify before injecting it. Also prefix the preview image class per
widget style convention and destructure encodeDataMatrix's params so the
useMemo dependency list stays exhaustive without a lint suppression.
@samuelreichert
samuelreichert force-pushed the WC-3448/add-datamatrix-generation branch from 312ba29 to f568c62 Compare July 20, 2026 08:01
@github-actions

Copy link
Copy Markdown
Contributor

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed


What was reviewed

File Change
src/components/DataMatrix.tsx New DataMatrix renderer component (bwip-js seam, DOMPurify, download integration)
src/components/preview/DataMatrixPreview.tsx New editor preview component for DataMatrix format
src/config/Barcode.config.ts DataMatrixTypeConfig added to discriminated union; barcodeConfig() dispatch extended
src/config/validation.ts DataMatrix case in validateBarcodeValue; new validateGs1DataMatrixValue
src/BarcodeGenerator.tsx Dispatch extended to route datamatrix config type to new renderer
src/BarcodeGenerator.editorPreview.tsx Preview dispatch updated for DataMatrix format
src/BarcodeGenerator.xml New DataMatrix enum value; new Advanced Data Matrix Settings property group
typings/BarcodeGeneratorProps.d.ts CodeFormatEnum + DmShapeEnum + three new container/preview props
src/utils/download-code.ts datamatrix filename-prefix branch added
src/ui/BarcodeGenerator.scss .datamatrix-svg and .barcode-generator-datamatrix-preview-image styles
src/__tests__/BarcodeGenerator.spec.tsx DataMatrix describe block (4 tests, bwip-js mock)
src/config/__tests__/Barcode.config.spec.ts New config-mapping tests for DataMatrix
src/config/__tests__/validation.spec.ts New validation unit tests for plain + GS1 DataMatrix
CHANGELOG.md [Unreleased] entry added
package.json @bwip-js/browser ^4.11.2 and dompurify ^3.4.11 added
openspec/changes/add-datamatrix-generation/** Design/spec/task artifacts — informational, not code-reviewed
document-viewer-web/openspec/changes/archive/** File renames only

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


Findings

🔶 Medium — E2E tests absent; gap undocumented in PR description

File: openspec/changes/add-datamatrix-generation/tasks.md line 260
Problem: Task 6.2 is explicitly marked [ ] with the reason "blocked: existing e2e spec is a placeholder; needs the external Mendix testProjects page configured". For a new rendering feature with an interactive download flow, no E2E test means the render→download path is only verified by human walkthrough (task 7.3). The PR description does not mention this gap, so reviewers and merge gates have no visibility.
Fix: Either (a) unblock the placeholder spec and add a DataMatrix render + download scenario before merge, or (b) open a follow-up issue, link it in the PR body, and add a // TODO: [ISSUE-NNN] comment in the placeholder spec so the gap is tracked. The skill guidelines treat a feature/fix without E2E as medium when interactive behaviour is involved.


⚠️ Low — DataMatrixPreview.tsx calls bwip-js live at editor load time

File: src/components/preview/DataMatrixPreview.tsx lines 1–8, 42–49
Note: BarcodePreview and QRCodePreview use pre-rendered static SVG assets so the editor bundle stays light and preview is immune to library failures. DataMatrixPreview imports and calls datamatrix/gs1datamatrix synchronously on every Studio Pro canvas render (memoised on gs1Mode, but still executed on first mount). This adds the bwip-js DataMatrix encoder to the editor bundle and any bwip-js error surfaces as the alert-danger fallback. Consider following the existing pattern — generate a static SVG asset at build time and embed it as a data URI or imported SVG, removing the runtime bwip-js dependency from the preview path entirely.


⚠️ Low — datamatrix-renderer and datamatrix-svg classes lack widget-name prefix

File: src/components/DataMatrix.tsx line 604; src/ui/BarcodeGenerator.scss line 897
Note: Frontend guidelines require custom classes to be prefixed with the widget name to avoid collisions (e.g. barcode-generator-datamatrix-renderer). The new runtime classes datamatrix-renderer and datamatrix-svg are unprefixed, while the preview class barcode-generator-datamatrix-preview-image is correctly prefixed. The existing barcode-renderer class is also unprefixed (a pre-existing inconsistency), so this is not a regression — but new classes added in this PR should follow the guideline. Worth aligning as a follow-up if not addressed now.


⚠️ Low — No unit test for DataMatrix download filename prefix

File: src/utils/download-code.ts lines 931–932
Note: The new else if (config.type === "datamatrix") branch sets the filename prefix (datamatrix / datamatrix_gs1). The existing suite tests download filename logic for barcode and QR formats, but there is no test asserting the DataMatrix branch produces the expected prefix. Given download is a user-facing feature, a targeted test in the config or download-code spec would close this gap.


Positives

  • dangerouslySetInnerHTML is guarded by DOMPurify.sanitize with the correct USE_PROFILES: { svg: true, svgFilters: true } — this is the right pattern and is applied consistently in both the runtime and preview components.
  • useMemo in DataMatrix.tsx has complete dependencies (codeValue, size, gs1Mode, shape, margin, logLevel) — no stale-closure risk.
  • The two-pass GS1 validation (structure regex + empty-AI lookahead regex) covers the key failure modes cleanly, and all branches are covered by validation.spec.ts.
  • Tree-shakeable named imports from @bwip-js/browser (datamatrix, datamatrixrectangular, gs1datamatrix, gs1datamatrixrectangular, drawingSVG) — no toSVG() / full-encoder import, matching the design decision.
  • DataMatrixTypeConfig discriminated union is clean and follows the existing BarcodeTypeConfig / QRCodeTypeConfig pattern exactly — dispatch in BarcodeGenerator.tsx reads naturally.
  • XML property keys (dmGs1Mode, dmShape, dmSize) are lowerCamelCase and match TypeScript props exactly.
  • CHANGELOG entry is user-facing and behaviour-focused, not implementation-focused.
  • bwip-js mock in the test file correctly uses { virtual: true } to handle the ESM-only library, and jest mock hoisting is handled correctly with the import * as bwip pattern.

return { valid: true };
case "DataMatrix":
// DataMatrix: encoder handles the heavy lifting; guard extremely long static values
if (value.length > 2000) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't this value to be the same as QRCode?
let say we use 2000, can we put it as constant or static number configurations?

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.

2 participants