Skip to content

Conversation

@bobbwal
Copy link
Contributor

@bobbwal bobbwal commented Jan 26, 2026

Description

Make rich‑text editor field types valid and fully wired for quill, tinymce, and easymde (and ensure existing markdown renders with the rich‑text container).

Fixes #569

Changes

  • Add quill, tinymce, and easymde to FieldType.
  • Render tinymce, easymde, and markdown with the rich‑text container and plugin fallbacks.
  • Persist new field types in schema creation in admin-collections.ts.
  • Rebuild core dist output.

Testing

  • npm run type-check
  • npm test
  • npm run e2e

Unit Tests

  • Added/updated unit tests
  • All unit tests passing

E2E Tests

  • Added/updated E2E tests
  • All E2E tests passing

Checklist

  • Code follows project conventions
  • Tests added/updated and passing
  • Type checking passes
  • No console errors or warnings
  • Documentation updated (if needed)

———

Generated with Claude Code in Conductor

@lane711
Copy link
Collaborator

lane711 commented Jan 26, 2026

PR #570 Review: Fix/richtext field types

Author: Rob Walton (@bobbwal)
Fixes: #569 - Missing rich-text field types in FieldType union

Summary

This PR adds support for quill, tinymce, and easymde field types to make rich-text editor fields valid in collection schemas. It addresses a TypeScript type-checking error when using these editor types.


Changes Analysis

1. packages/core/src/types/collection-config.ts

✅ Good: Adds the three missing field types to the FieldType union:

| 'tinymce'
| 'quill'
| 'easymde'

This directly fixes issue #569.


2. packages/core/src/routes/admin-collections.ts (lines 699-704)

✅ Good: Properly wires up the new field types in the schema creation logic:

} else if (fieldType === 'tinymce') {
  fieldConfig.type = 'tinymce'
} else if (fieldType === 'easymde') {
  fieldConfig.type = 'easymde'
} else if (fieldType === 'markdown') {
  fieldConfig.type = 'markdown'
}

3. packages/core/src/templates/components/dynamic-field.template.ts

⚠️ Issue 1 - Inconsistent plugin status check (line 111):

} else if ((field.field_type === 'mdxeditor' || field.field_type === 'easymde' || field.field_type === 'markdown') && !pluginStatuses.mdxeditorEnabled) {

This bundles easymde and markdown with mdxeditor plugin status check, but:

  • easymde is a separate editor (EasyMDE) from MDXEditor
  • The warning message says "EasyMDE plugin is inactive" but checks mdxeditorEnabled
  • Should there be a separate easymdeEnabled plugin status?

Question: Is there an actual EasyMDE plugin, or do easymde and markdown field types intentionally fall back to MDXEditor?

⚠️ Issue 2 - Rendering case statement (lines 284-297):

case 'mdxeditor':
case 'tinymce':
case 'easymde':
case 'markdown':

The comment says these render the same container as richtext, but:

  • tinymce has its own separate plugin and initialization
  • The current code on main already handles tinymce fallback separately (line 114-116)
  • Grouping tinymce with mdxeditor/easymde/markdown may cause the TinyMCE plugin initialization to fail

Recommendation: tinymce should likely have its own case statement since it uses a different initialization path.


Missing Items

  1. No unit tests added - The PR modifies field type handling but doesn't add tests for the new field types
  2. No e2e tests added - User workflows with these field types aren't tested
  3. Documentation not updated - The PR checklist marks "Documentation updated (if needed)" as unchecked

Questions for Author

  1. What's the relationship between easymde, markdown, and mdxeditor? Are they meant to use the same plugin?
  2. Should tinymce really share the same rendering case as mdxeditor?
  3. Was manual testing done with each editor type to verify they initialize correctly?

Recommendation

Request changes - The core type fix is correct, but the rendering logic changes need clarification:

  1. Clarify the plugin relationship for easymde/markdown/mdxeditor
  2. Verify tinymce rendering is correct (shouldn't be grouped with mdxeditor)
  3. Consider adding at least one unit test for the new field types

The type fix itself (collection-config.ts) and the admin-collections wiring look good and can be approved independently.

@bobbwal bobbwal marked this pull request as draft January 27, 2026 11:37
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.

Missing rich‑text field types in FieldType union

2 participants