Skip to content

Commit cd6e358

Browse files
committed
chore(chart-playground-web): add openspec change for WC-3348
1 parent 64d8b4a commit cd6e358

4 files changed

Lines changed: 188 additions & 0 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: tdd-refactor
2+
created: 2026-07-06
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
## Test Cases
2+
3+
All unit tests, `@testing-library/react`, in
4+
`packages/pluggableWidgets/chart-playground-web/src/components/__tests__/CodeEditor.spec.tsx`
5+
(new file — `CodeEditor` has no tests today).
6+
7+
The editor is `react-simple-code-editor`, which renders a real `<textarea>` under a highlighted
8+
`<pre>`. Query the textarea via `role="textbox"` / value; assert highlight + lint via rendered
9+
output. These tests define the contract before the textarea → highlighted-editor swap.
10+
11+
### Reproduction Tests
12+
13+
- **Renders JSON with syntax highlighting** - the editor highlights the value instead of showing
14+
plain unstyled text (the regression the textarea introduced). (unit)
15+
- **Given**: `CodeEditor` rendered with `value='{"type":"scatter","x":[1,2,3]}'`.
16+
- **When**: component mounts.
17+
- **Then**: the rendered output contains `highlight.js` token markup (at least one
18+
`.hljs-*` span, e.g. `hljs-attr` / `hljs-string` / `hljs-number`) wrapping parts of the
19+
value — not a bare unstyled text node.
20+
21+
- **Surfaces invalid JSON** - malformed JSON is flagged, not silently accepted (the DX gap the
22+
textarea left; ties to Leonardo's "no silent catch" note). (unit)
23+
- **Given**: `CodeEditor` rendered with `value='{"type": '` (truncated / invalid JSON).
24+
- **When**: component mounts (or value changes to invalid).
25+
- **Then**: an error indication is shown (an element with the error class / role carrying the
26+
`JSON.parse` message); the editor still renders the raw text (does not blank out or throw).
27+
28+
- **Valid JSON shows no error** - complement to the above. (unit)
29+
- **Given**: `CodeEditor` with `value='{"a":1}'`.
30+
- **When**: component mounts.
31+
- **Then**: no error indication element is present.
32+
33+
### Edge Cases
34+
35+
- **Empty value renders no error and no crash** (unit)
36+
- **Given**: `CodeEditor` with `value=''`.
37+
- **When**: mounts.
38+
- **Then**: renders an empty editor, no error indication (empty ≠ invalid), no throw.
39+
40+
- **onChange fires with new text on edit** (unit)
41+
- **Given**: `CodeEditor` with `value='{}'` and a jest mock `onChange`.
42+
- **When**: user types into the textbox (`fireEvent.input` / `userEvent.type`).
43+
- **Then**: `onChange` is called with the updated string.
44+
45+
- **readOnly blocks edits** - modeler panel uses `readOnly`. (unit)
46+
- **Given**: `CodeEditor` with `readOnly` and a mock `onChange`.
47+
- **When**: user attempts to type in the textbox.
48+
- **Then**: the textbox is non-editable (disabled/readonly attribute) and `onChange` is not
49+
called.
50+
51+
- **Highlighter degrades gracefully on throw** - a highlight failure must not break the editor. (unit)
52+
- **Given**: highlighting a value that would make `hljs.highlight` throw (illegal sequence).
53+
- **When**: mounts.
54+
- **Then**: falls back to rendering the raw code (no crash), matching the rich-text pattern's
55+
try/catch + `console.warn`.
56+
57+
### Regression Tests
58+
59+
- **Prop contract unchanged** - `ComposedEditor` calls `CodeEditor` with `value/onChange/height`
60+
(editable panel) and `readOnly/value/height` (modeler panel). Both must keep working. (unit)
61+
- **Given**: `CodeEditor` rendered with `height="var(--editor-h)"`.
62+
- **When**: mounts.
63+
- **Then**: renders without error and applies the `height` (editor container reflects the
64+
passed height, as the textarea did).
65+
66+
- **No CodeMirror dependency reintroduced** - the whole point of WC-3348. (build/lint guard)
67+
- **Given**: the widget package after the change.
68+
- **When**: inspecting imports/deps of `chart-playground-web`.
69+
- **Then**: no `codemirror`/`@codemirror/*` import or dependency is present; only
70+
`react-simple-code-editor` + `highlight.js` added.
71+
72+
- **preview.spec snapshot** - existing `ChartPlayground.editorPreview` snapshot still passes
73+
(design-time preview is unaffected — it renders a static Toggle button, not `CodeEditor`). (unit)
74+
- **Given**: existing `src/__tests__/preview.spec.tsx`.
75+
- **When**: test suite runs.
76+
- **Then**: snapshot matches (or is intentionally updated only if the preview markup changes).
77+
78+
## Notes
79+
80+
- `react-simple-code-editor` renders a genuine textarea → RTL `getByRole("textbox")` works;
81+
editing via `fireEvent.input(textarea, { target: { value } })`.
82+
- Import `highlight.js/lib/core` + register only `languages/json` (tree-shaken) — assert token
83+
markup, not a specific theme.
84+
- The "No CodeMirror" check can be a dependency/import assertion or a package.json test rather
85+
than a runtime RTL test — decide at tasks.md time.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
## Why
2+
3+
The Charts playground editor (WC-3348) lost its real code editor. In Charts v6.3.0 the
4+
playground shipped a CodeMirror-based editor that threw a bundling error at load, breaking
5+
the playground in both dojo and React runtimes. To unblock the release of WC-3345, CodeMirror
6+
was replaced with a bare `<textarea>` and shipped as charts-web 6.3.1.
7+
8+
The textarea works but is a developer-experience regression: no syntax highlighting and no
9+
feedback when the JSON a developer types is invalid. The playground is a developer-facing tool
10+
where authors hand-edit Plotly trace/layout/config JSON, so highlighting and lint matter.
11+
12+
## Root Cause
13+
14+
CodeMirror could not be bundled by the widget build (Rollup) and threw at runtime. The
15+
workaround removed CodeMirror entirely (0 refs remain in `chart-playground-web`, `shared/charts`,
16+
or its deps). The remaining work is to restore a real editor **without** reintroducing a
17+
CodeMirror-class bundling dependency.
18+
19+
## What Changes
20+
21+
Replace the plain `<textarea>` in `CodeEditor.tsx` with a lightweight highlighted editor,
22+
adapting the proven pattern from `rich-text-web`'s `HighlightedCodeEditor.tsx`
23+
(`react-simple-code-editor` + `highlight.js`), tuned for JSON:
24+
25+
- `packages/pluggableWidgets/chart-playground-web/src/components/CodeEditor.tsx`
26+
- Render `react-simple-code-editor` with a `highlight.js` JSON highlighter instead of a raw textarea.
27+
- Preserve the existing prop contract exactly: `value`, `onChange?`, `readOnly?`, `height?`.
28+
- Surface JSON validity: when `value` fails `JSON.parse`, show a non-blocking error indication
29+
(do not swallow silently — the prior review flagged silent empty-catch blocks).
30+
- Keep Tab-in-editor behavior compatible with the surrounding `TabGuard` / "Esc + Tab to move
31+
focus" hint (react-simple-code-editor `ignoreTabKey={false}`).
32+
- `packages/pluggableWidgets/chart-playground-web/package.json`
33+
- Add `react-simple-code-editor` and `highlight.js` (**new deps — approved with user**).
34+
- Add unit tests for `CodeEditor` (none exist today).
35+
- Changelog entry (user-facing): playground editor restores syntax highlighting and invalid-JSON
36+
feedback.
37+
38+
Not in scope: Leonardo de Souza's review findings on the already-merged custom-chart / playground
39+
rework (`computed().get()` misuse, dead `containerStyle`, hardcoded `layoutOptions/configOptions`,
40+
`store.data` cast, `@types/jest` in deps, deleted `mergeChartProps.spec.ts`, and related minors).
41+
Those are real, still-live issues but sit in adjacent files (`useCustomChart.ts`,
42+
`CustomChartControllerHost.ts`, `useComposedEditorController.ts`, `shared/charts`) and are tracked
43+
in follow-up ticket WC-3488 so this change stays a single-purpose editor restore. The two
44+
JSON-safety items from that review that DO belong here — silent empty-JSON catch blocks and the
45+
"CodeMirror is a UX regression" note — are folded into the JSON-lint work above.
46+
47+
## Impact
48+
49+
- **Consumers:** `ComposedEditor.tsx` uses `CodeEditor` in two spots — an editable panel
50+
(`value`/`onChange`/`height`) and a read-only modeler panel (`readOnly`/`value`/`height`).
51+
The prop contract is unchanged, so both keep working. Not breaking.
52+
- **Must NOT break:** no CodeMirror (or CodeMirror-class heavy dep) reintroduced; widget bundle
53+
stays small; the runtime "Toggle Editor" sidebar keeps working; read-only panel stays read-only.
54+
- **New dependencies:** two, both lightweight and used in-repo pattern (`rich-text-web`). No
55+
CodeMirror bundling risk.
56+
- **Users:** Charts playground authors get syntax highlighting + invalid-JSON feedback back.
57+
Version bump deferred to release time per repo convention.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
## 0. Dependencies
2+
3+
- [x] 0.1 Add `react-simple-code-editor` and `highlight.js` to `chart-playground-web/package.json` (approved with user); run install
4+
- [x] 0.2 Confirm no `codemirror` / `@codemirror/*` present in the package
5+
6+
## 1. Test Setup
7+
8+
<!-- RED: Write failing tests first -->
9+
10+
- [x] 1.1 Create `src/components/__tests__/CodeEditor.spec.tsx`; write failing test: renders JSON with `.hljs-*` token markup (not plain text)
11+
- [x] 1.2 Add failing test: invalid JSON surfaces an error indication; valid JSON shows none; empty value shows none
12+
- [x] 1.3 Add edge tests: `onChange` fires with new text on edit; `readOnly` disables editor. (Note: "highlighter throw" dropped — can't trigger a genuine hljs throw with `ignoreIllegals:true` without a mock that tests implementation, not behavior; try/catch remains as defensive code.)
13+
- [x] 1.4 Add regression tests: `height` prop honored; no CodeMirror dep (package.json assertion); existing `preview.spec` snapshot still passes
14+
15+
## 2. Implementation
16+
17+
<!-- GREEN: Make tests pass with minimal code -->
18+
19+
- [x] 2.1 Replace `<textarea>` in `CodeEditor.tsx` with `react-simple-code-editor` + `highlight.js` JSON highlighter (import `highlight.js/lib/core` + register `languages/json`); keep prop contract `value/onChange?/readOnly?/height?`
20+
- [x] 2.2 Add JSON lint: `JSON.parse(value)` in try/catch, surface error message (empty value = not an error); no silent catch
21+
- [x] 2.3 Wrap highlight call in try/catch → fall back to raw code + `console.warn` (rich-text pattern)
22+
- [x] 2.4 Honor `height` prop and `readOnly` (disabled); keep Tab behavior compatible with `TabGuard` (`ignoreTabKey={false}`)
23+
24+
## 3. Refactoring
25+
26+
<!-- REFACTOR: Clean up while keeping tests green -->
27+
28+
- [x] 3.1 Clean up component; highlight theme (`atom-one-light`) imported once in component; added scoped `.widget-charts-playground-code-editor` + error SCSS
29+
- [x] 3.2 `highlight` + `jsonError` extracted as local module functions (kept local — no adjacent rework files touched)
30+
31+
## 4. Verification
32+
33+
- [x] 4.1 All new `CodeEditor` tests passing (8 tests)
34+
- [x] 4.2 Full `chart-playground-web` suite passes (9 tests, preview snapshot green); `pnpm build` produces MPK with no bundling error
35+
- [x] 4.3 Add user-facing changelog entry (highlighting + invalid-JSON feedback restored)
36+
- [ ] 4.4 `/code-review` before PR; do not regress Leonardo's prior review; PR ready
37+
38+
## Notes
39+
40+
<!-- Track test failures, refactoring decisions, blockers. -->
41+
42+
- "No CodeMirror" test decided as a package.json / import assertion (per design.md open question), not a runtime RTL test.
43+
- Scope guard: this change touches only `CodeEditor.tsx`, its test, and `package.json`. Adjacent files (`useCustomChart.ts`, `CustomChartControllerHost.ts`, `useComposedEditorController.ts`, `shared/charts`) are reserved for the follow-up ticket (`followup-ticket-draft.md`).
44+
- Version bump deferred to release time per repo convention.

0 commit comments

Comments
 (0)