fix #332 - feat: Create the text-editor package - #386
Conversation
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
…te-the-text-editor-package Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
✅ Deploy Preview for openworkflow-editor canceled.
|
There was a problem hiding this comment.
🟡 Changes recommended
There is at least one TypeScript-breaking issue in the new Storybook helper plus documentation/package-manifest inconsistencies that should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a new workspace package, packages/text-editor (@openworkflowspec/text-editor), establishing the monorepo foundation for a Monaco-based controlled React text editor along with Storybook showcase and test setup.
Changes:
- Add
@openworkflowspec/text-editorpackage scaffolding (Vite/Vitest/TS configs), aTextEditorcomponent, and package exports. - Add Storybook stories + story tests and Playwright e2e smoke test for Monaco rendering.
- Add
monaco-editorto the pnpm catalog and lockfile, wiring the dependency into the new package.
File summaries
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds monaco-editor to the workspace catalog for consistent dependency management. |
| pnpm-lock.yaml | Locks monaco-editor (and transitive deps) and adds the new packages/text-editor importer entry. |
| packages/text-editor/vitest.config.ts | Establishes Vitest configuration with unit + Storybook test projects (including browser tests via Playwright). |
| packages/text-editor/vite.config.ts | Adds Vite library build config for the new package output. |
| packages/text-editor/tsconfig.test.json | Adds TS compiler options tailored for tests (Vitest + Testing Library). |
| packages/text-editor/tsconfig.json | Adds package TS build config for declaration emit and output layout. |
| packages/text-editor/tests/text-editor/TextEditor.test.tsx | Adds unit tests verifying controlled content behavior and lifecycle. |
| packages/text-editor/tests/text-editor/TextEditor.story.test.tsx | Adds tests that validate Storybook stories configure Monaco as expected. |
| packages/text-editor/tests/setupTests.ts | Sets up Testing Library cleanup and Vitest mocks for Monaco modules. |
| packages/text-editor/tests/mocks/monaco-editor.ts | Provides a Monaco mock used by unit/story tests. |
| packages/text-editor/tests-e2e/text-editor.spec.ts | Adds Playwright e2e smoke test to verify Monaco renders in Storybook. |
| packages/text-editor/stories/samples/index.ts | Exports sample JSON/YAML documents for stories. |
| packages/text-editor/stories/samples/hello-world.yaml | Adds a YAML sample Open Workflow document for stories. |
| packages/text-editor/stories/samples/hello-world.json | Adds a JSON sample Open Workflow document for stories. |
| packages/text-editor/stories/introduction/Welcome.mdx | Adds Storybook introduction content for the new Text Editor package. |
| packages/text-editor/stories/helpers.ts | Adds a helper for generating stories with default args. |
| packages/text-editor/stories/features/TextEditor.tsx | Adds a Storybook wrapper component around the package TextEditor. |
| packages/text-editor/stories/features/TextEditor.stories.tsx | Adds feature stories demonstrating JSON/YAML/read-only modes. |
| packages/text-editor/src/TextEditor.tsx | Introduces the Monaco-backed controlled TextEditor React component. |
| packages/text-editor/src/index.ts | Adds the package entry point exporting TextEditor. |
| packages/text-editor/README.md | Adds package-level documentation and usage example. |
| packages/text-editor/playwright.config.ts | Adds Playwright config for running the package e2e test against Storybook. |
| packages/text-editor/package.json | Adds the new package manifest, scripts, and dependency declarations. |
| packages/text-editor/.storybook/preview.tsx | Adds Storybook preview configuration (controls/a11y/options/globalTypes). |
| packages/text-editor/.storybook/main.ts | Adds Storybook main config (addons, stories globs, TS checking, telemetry off). |
| packages/text-editor/.oxlintrc.json | Inherits repo oxlint configuration for the new package. |
| packages/text-editor/.oxfmtrc.json | Inherits repo oxfmt configuration for the new package. |
| .changeset/add-text-editor-package.md | Adds a changeset entry announcing the new text-editor package. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 27/28 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The Playwright e2e test currently doesn’t assert any interaction despite claiming interactivity, leaving a key behavior unverified.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/text-editor/tests-e2e/text-editor.spec.ts:27
- The test name says the Monaco editor is "interactive", but the assertions only check that the DOM elements exist. Add at least one interaction assertion (e.g., focus the hidden textarea) so the test verifies basic interactivity rather than just rendering.
- Files reviewed: 28/29 changed files
- Comments generated: 0 new
- Review effort level: Lite
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new Playwright E2E test uses a platform-specific keyboard shortcut (Control+A) that can fail on macOS and should be made cross-platform.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 28/29 changed files
- Comments generated: 1
- Review effort level: Lite
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The new docs currently claim completions/diagnostics via language-service despite that integration being explicitly deferred to a follow-up, which can mislead consumers.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
packages/text-editor/README.md:28
- README claims language-service completions/diagnostics are provided by @openworkflowspec/language-service, but this package currently only depends on monaco-editor and does not integrate the language service yet (per PR description, that work is planned separately). This is misleading for consumers of the package docs.
`TextEditor` is a controlled component that provides:
- JSON and YAML syntax highlighting;
- read-only mode;
- controlled content updates;
- language-service features, such as completions and diagnostics, provided by `@openworkflowspec/language-service`.
packages/text-editor/stories/introduction/Welcome.mdx:30
- This intro page lists completions/diagnostics as supported features, but the current package implementation only wires Monaco + JSON/YAML registration and does not yet include the planned language-service integration (#331). This reads like functionality is already available.
It is powered by [Monaco Editor](https://github.com/microsoft/monaco-editor) and supports:
- JSON and YAML syntax highlighting;
- editable and read-only modes;
- controlled content updates;
- Monaco language features such as completions and diagnostics.
- Files reviewed: 28/29 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
#386 (review) |
…te-the-text-editor-package Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new TextEditor component and Vitest setup include correctness/operational issues (resource cleanup and test-project separation) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/text-editor/src/TextEditor.tsx:92
isApplyingExternalContentRefis set back tofalseimmediately aftereditor.setValue(content). IfsetValuethrows (or if future changes add early returns), the flag can remain stucktrueand permanently suppress user-drivenonContentChangeevents.
isApplyingExternalContentRef.current = true;
editor.setValue(content);
isApplyingExternalContentRef.current = false;
}, [content]);
- Files reviewed: 28/29 changed files
- Comments generated: 2
- Review effort level: Lite
Closes #332
Description
Create
packages/text-editor(@openworkflowspec/text-editor) as the package foundation for the Monaco-based Open Workflow text editor.Motivation
Provide the common package infrastructure required to implement a Monaco for Open Workflow text editing.
Proposed Implementation
@openworkflowspec/text-editorworkspace package based onmonaco-editor.The package should remain private for now.
Definition of Done
Preview:
https://fantonangeli.github.io/open-workflow-specification-editor/issue-332