Skip to content

Refactor: Block Editor#35257

Open
rjvelazco wants to merge 56 commits intomainfrom
refactor-dotcms-block-editor
Open

Refactor: Block Editor#35257
rjvelazco wants to merge 56 commits intomainfrom
refactor-dotcms-block-editor

Conversation

@rjvelazco
Copy link
Copy Markdown
Member

@rjvelazco rjvelazco commented Apr 8, 2026

WIP

Videos

video.mov
video.mov

Note

High Risk
High risk because this is a large refactor that swaps the block editor app to a new standalone editor implementation, upgrades TipTap/ngx-tiptap major versions, and adds dotCMS upload/search integrations including a hardcoded auth token/base URL in code.

Overview
Replaces the dotcms-block-editor app’s NgModule-based bootstrap with Angular standalone bootstrapApplication and points the app at the new EditorComponent exported from @dotcms/new-block-editor, alongside updated Angular build target config (new executor/outputPath structure, dev config, baseHref).

Adds a new new-block-editor library implementing an experimental TipTap v3-based editor with slash menu, toolbar, drag-handle gutter, link/image/video/table dialogs, emoji picker, upload placeholders, and dotCMS-backed asset/content-type search + upload services.

Updates global styling for the new editor UI (Tailwind layers, typography plugin, Material Symbols, TipTap/table/link/upload-placeholder styles) and bumps editor-related dependencies (TipTap v3, ngx-tiptap, Floating UI, emoji-mart); also removes a shared SCSS code style block.

Reviewed by Cursor Bugbot for commit 77fd1d1. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

  • This PR fixes #123 or Fixes: #123

  • This PR closes #123 or Closes: #123

  • This PR resolves #123 or Resolves: #123

  • Other supported keywords: fix, fixed, close, closed, resolve, resolved
    Option 2: Link via GitHub UI (Note: won't clear the failed check)

  1. Go to the PR → Development section (right sidebar)

  2. Click "Link issue" and select an existing issue

  3. Push a new commit or re-run the workflow to clear the failed check
    Option 3: Use branch naming
    Create a new branch with one of these patterns:

  • 123-feature-description (number at start)

  • issue-123-feature-description (issue-number at start)

  • feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.---

This comment was automatically generated by the issue linking workflow

@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Apr 8, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 8, 2026

Claude finished @rjvelazco's task in 2m 9s —— View job


PR Review

  • Read key changed files
  • Analyze security, correctness, and design issues
  • Post findings

Issues

Blockers

1. app.spec.ts fails CI — libs/new-block-editor/src/lib/app.spec.ts:22
Asserts h1 contains 'Hello, block-editor'. App renders <dot-block-editor /> — no h1 exists. Leftover Nx scaffold. Delete the file (and app.routes.ts, app.config.ts — none of them are imported by anything).

2. allowedBlocks field variable has no effect on extensions — editor.component.ts:308
createEditorExtensions() is called as a class field initializer before Angular resolves inputs:

readonly editor: Editor = new Editor({
    extensions: createEditorExtensions(
        this.menuService,
        parseAllowedBlocks(this.field()),   // this.field() is always undefined here
        this.injector
    )
});

parseAllowedBlocks(undefined) returns undefined, so has() is always true and every extension is loaded unconditionally. The constructor's effect(() => this.store.setAllowedBlocks(...)) only affects the slash menu filter — it does nothing to restrict which TipTap extensions are actually registered. The "Allowed Blocks" content-type setting is silently broken. Fix this →

High Risk

3. Local filesystem path committed — libs/new-block-editor/src/feature.md:9

/Users/rjvelazco/Desktop/dotcms/core/core-web/libs/sdk/react/...

Personal dev reference; remove before merge.

4. Experimental flag committed to workspace settings — core-web/.claude/settings.json:11
CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 propagates to every developer's Claude Code session via project settings. An experimental flag should not be committed.

5. No tests — 7,000+ lines added, one broken spec. CVA contract, slash menu, upload pipeline, and all dialogs are untested. Already flagged by prior review; still a blocker per core-web/CLAUDE.md standards.

Correctness

6. Null-unsafe outputToObservable call — dot-content-compare-block-editor.component.ts:35

outputToObservable(this.blockEditorCompare?.valueChange)

this.blockEditorCompare is a @ViewChild which is undefined before ngAfterViewInit — optional chaining passes undefined into outputToObservable, which will throw. Add a null guard or restructure.

7. fetchByType injects variable into Lucene without escaping — dot-contentlet.service.ts:156

query: `+contentType:${variable} ...`

The rest of the service calls escapeLuceneToken() for user-supplied text; variable here comes from the slash-menu catalog's keywords[0] which is set from the API response ct.variable. Low exploitability, but inconsistent.

8. publishImageAsset and publishVideoAsset are identical — dot-upload.service.ts:84–176
Both methods contain the same POST body, same response mapping, same error messages. They differ only in the satisfies clause. Any future fix to one must be copied to the other.

Minor

9. dot-content-compare-block-editor.component.ts not modernized
Uses @Input(), @ViewChild, AfterViewInit and @Input decorators. core-web/CLAUDE.md requires input() and output() signals for all modified files.

10. console.error in upload failure handlers — editor.utils.ts:61,90
Upload failures silently log to console. Should surface via DotHttpErrorManagerService so the user gets feedback.

11. PORTING_CHECKLIST.md committed to library source
Dev scaffolding. Remove or .gitignore before merge.


The allowedBlocks regression (#2) is the most consequential new finding — the field variable setting appears to work (slash menu respects it) but the TipTap extensions load everything regardless, so content authored with a "restricted" editor still has access to all block types via toolbar buttons and drag-and-drop.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 8, 2026

Rollback Safety Analysis - Safe to Roll Back. All 9 changed files are frontend Angular config only (new-block-editor library scaffold). Label AI: Safe To Rollback applied.

@rjvelazco rjvelazco mentioned this pull request Apr 8, 2026
5 tasks
@rjvelazco rjvelazco linked an issue Apr 8, 2026 that may be closed by this pull request
5 tasks
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Fix All in Cursor

Bugbot Autofix is kicking off a free cloud agent to fix these issues. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 77fd1d1. Configure here.

Comment thread core-web/libs/new-block-editor/src/lib/editor/services/dot-cms.config.ts Outdated
this.commandFn = null;
// isOpen and clientRectFn unchanged — menu is already visible and positioned
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Commented-out state updates cause stale menu display

Medium Severity

In openSubmenu(), the lines this.items.set([]), this.activeIndex.set(0), and this.isLoading.set(true) are all commented out. Similarly in setItems(), this.activeIndex.set(0) and this.isLoading.set(false) are commented out. This means the loading spinner never appears during async content-type fetches, stale items from the previous menu remain visible while loading, and activeIndex isn't reset when new sub-menu items arrive — risking an out-of-bounds selection on Enter.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 77fd1d1. Configure here.

}
})
]
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unused library files: app config, routes, and spec

Low Severity

app.config.ts, app.routes.ts, and app.spec.ts in libs/new-block-editor/src/lib/ are never imported by anything in the codebase. The app.config.ts sets up provideRouter with empty routes and PrimeNG theming that no consumer uses. The app.spec.ts tests App for an h1 containing "Hello, block-editor" which doesn't exist in the actual App template. These appear to be leftover scaffolding files.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 77fd1d1. Configure here.

user-select: none !important;
}

code {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shared global code CSS removed affecting other consumers

Medium Severity

The global code element styling (color, background, padding, font-family, line-break) was removed from the shared styles.scss in libs/dotcms-scss/angular/. This is a shared stylesheet imported by multiple applications, not just the block editor. Removing this rule may break code element rendering across all consuming apps that relied on this global style.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 77fd1d1. Configure here.

…o dialogs, enhancing search and display functionality
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 10, 2026

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 10, 2026

Rollback Safety Analysis

  • Read rollback-unsafe categories reference
  • Analyze PR diff against unsafe categories
  • Post results and apply labels

View job run

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 10, 2026

Rollback Safety Analysis - Safe to Roll Back. All 57 changed files are frontend Angular/TypeScript only (new-block-editor library scaffold, block-editor app refactor, SCSS, package.json). No database migrations, Elasticsearch mapping changes, API contract changes, or any backend code modified. Label AI: Safe To Rollback applied.

@semgrep-code-dotcms-test
Copy link
Copy Markdown

Legal Risk

The following dependencies were released under a license that
has been flagged by your organization for consideration.

Recommendation

While merging is not directly blocked, it's best to pause and consider what it means to use this license before continuing. If you are unsure, reach out to your security team or Semgrep admin to address this issue.

GPL-2.0

MPL-2.0

…proved store management, and updated slash menu functionality

- Added a comprehensive porting checklist for tracking features in the new block editor.
- Introduced EditorStore for managing editor state, including allowed block types and language ID.
- Updated slash menu service to utilize the new store for block type filtering.
- Refactored image and video extensions to use consistent node names.
- Improved editor component with additional input properties for better integration with dotCMS content types.
- Cleaned up unused code and comments in editor click handling.

This commit enhances the overall functionality and maintainability of the new block editor.
- Replaced `EditorComponent` with `DotCMSEditorComponent` across various files for consistency.
- Removed unused global styles from `styles.css` and added new styles to `editor.component.css`.
- Introduced a new block options constant for better management of allowed blocks in the editor settings.
- Updated toolbar icons to use the correct Material Icons set.
- Improved the handling of editor content normalization and state management.

This refactor aims to streamline the block editor's structure and improve maintainability.
…date references

- Added support for the new "Material Symbols Outlined" font, including its CSS and SCSS definitions.
- Updated existing components to utilize the new font for toolbar icons, replacing legacy Material Icons references.
- Renamed legacy Material Icons to clarify their status as outdated.

This update enhances the iconography in the block editor, aligning with the latest design standards.
…ments

- Introduced functions to stringify editor content as ProseMirror JSON and check for content matches.
- Updated the editor to emit JSON on changes instead of HTML, aligning with the new data handling approach.
- Enhanced the toolbar component with PrimeNG's Select for block type selection, improving user experience.
- Updated slash menu icons to use Material Symbols for better visual consistency.
- Adjusted styles for toolbar and menu items for improved layout and accessibility.

This update improves the editor's functionality and user interface, preparing for future enhancements.
…omponents

- Introduced auto-focus functionality for the first focusable element in the editor dialog after rendering.
- Removed unnecessary focus methods from image, link, table, and video dialog components, centralizing focus logic in the editor dialog.
- Updated documentation to reflect changes in the dialog's opened output behavior, allowing for more flexible post-open logic.
This refactor improves the user experience by ensuring that dialogs are more accessible and easier to navigate upon opening.
- Introduced a new `data` attribute for the DotContentlet node, consolidating contentlet attributes into a single object for improved serialization and management.
- Updated the editor component to utilize the new `ContentletData` structure, simplifying data handling and enhancing performance.
- Refactored the slash menu to insert contentlets using the new data format, ensuring backward compatibility with legacy attributes.
- Enhanced the editor toolbar state service to accommodate the new data structure, improving the selection and management of contentlets.

This update streamlines contentlet integration within the editor, preparing for future enhancements and improving overall code maintainability.
- Introduced a new Angular node view for contentlets, enhancing the rendering and interaction within the editor.
- Refactored the contentlet extension to utilize the new node view, improving the structure and maintainability of the code.
- Updated the editor component and extensions to accommodate the new contentlet structure, ensuring seamless integration with existing features.
- Removed the legacy contentlet extension file, consolidating functionality into the new structure.

This update enhances the editor's capability to manage contentlets, paving the way for future improvements and a more robust editing experience.
…ling

- Added new dialog components for image, video, link, table, and emoji selection, improving the user experience for media insertion.
- Refactored existing editor components to utilize the new dialog structure, ensuring a more modular and maintainable codebase.
- Updated the slash menu to support new content types and improved integration with the editor's dialog management system.
- Enhanced focus handling and accessibility features across all dialog components, ensuring a smoother user interaction.

This update significantly enhances the media handling capabilities of the editor, paving the way for future improvements and a more robust editing experience.
- Added a new AI content dialog component for generating text using AI, enhancing the editor's capabilities.
- Integrated the AI content node into the editor, allowing users to insert AI-generated content seamlessly.
- Refactored existing services to support the new AI functionality, including the addition of a dedicated AI service for handling requests.
- Updated the slash menu to include an option for AI content generation, improving accessibility and user experience.

This update significantly enhances the editor's functionality by enabling AI-driven content creation, paving the way for more dynamic and engaging user interactions.
@zJaaal
Copy link
Copy Markdown
Member

zJaaal commented Apr 28, 2026

Code review (AI-assisted, posted by @zJaaal via Claude)

Review generated by Claude Opus 4.7 at Jalinson's request — flagging items worth a human pass before this lands.

Overview

Standalone-bootstraps dotcms-block-editor, introduces the new @dotcms/new-block-editor lib, upgrades TipTap 2.27 → 3.22 + ngx-tiptap 12 → 14, and rewires the editor into edit-content, EMA sidebar, and content-compare consumers.

What looks good

  • No hardcoded tokens / base URLs — all calls are same-origin with withCredentials: true. dot.config.ts explicitly forbids it.
  • Lucene injection guarded via escapeLuceneToken in DotContentletService.
  • Link dialog blocks javascript: URLs (Validators.pattern(/^https?:\/\//)); Link extension forces rel="noopener noreferrer".
  • Modern Angular throughout (signals, input()/output(), inject(), OnPush, standalone) — matches core-web/CLAUDE.md.
  • Good service-per-resource separation; AI dialog [innerHTML] is auto-sanitized by Angular.

Blockers

  1. app.spec.ts will fail in CI — asserts h1 contains 'Hello, block-editor', but App renders <dot-block-editor />. Looks like leftover nx g app scaffolding (along with the empty app.routes.ts and unused app.ts). Delete or fix.
  2. Allowed-block name mismatch breaks the content-type "Allowed Blocks" setting. BLOCK_OPTIONS writes codes like dotContent, heading1heading6, aiContentPrompt, but editor-extensions.ts gates on has('contentlet'), has('heading'), has('emoji'), has('link'). Result: settings have no effect on those blocks. Normalize the vocabulary or translate at the boundary.
  3. Personal/local content committed: feature.md hardcodes /Users/rjvelazco/Desktop/...; libs/new-block-editor/CLAUDE.md reads as personal Claude config rather than lib docs; .claude/settings.json enables CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 for everyone — confirm intent.

Strong concerns

  1. Test coverage is ~0. A 7k-line library lands with one (broken) auto-generated spec. None of the services, store, slash menu, dialogs, extensions (UploadPlaceholderExtension, DotImage, contentlet, grid, slash-command), or the DotCMSEditorComponent CVA contract are tested. Below the bar set in core-web/CLAUDE.md.
  2. Service duplication vs libs/data-access. DotAiService/contentlet/content-type/language services are reimplementations — the file comment acknowledges it. Two divergent impls of the same dotCMS endpoints will drift. Either extract a shared HTTP core or keep the data-access dependency.
  3. TipTap v3 storage round-trip. Major-version bump with breaking API changes (e.g. valueChange is now output<>() requiring outputToObservable in dot-content-compare-block-editor.component.ts). Need a load/save plan against existing stored block-editor JSON before flipping consumers.
  4. Bundle / runtime asset risk. emoji-mart + @emoji-mart/data is ~700KB; consider lazy-loading. Material Symbols loaded from fonts.googleapis.com in the demo index.html — confirm the embedded editor inside dotcms-ui doesn't depend on the external CDN at runtime (on-prem).

Smaller items

  • editor.component.ts: Editor is constructed as a class field with createEditorExtensions(..., this.allowedBlocks(), ...), which runs once at init. Changing [allowedBlocks] later only filters the slash menu via EditorStore — extensions don't re-add/remove. Document or rebuild on change.
  • editor.utils.ts uses raw console.error for upload failures — prefer DotHttpErrorManagerService or a logger.
  • .eslintrc.json disables @angular-eslint/prefer-standalone even though every component in the lib is standalone — remove the override or document why.
  • Link.configure({ HTMLAttributes: { target: '_self' } }) — check legacy docs with no target round-trip cleanly.
  • DotContentletService fakes totalRecords = offset + count + 1 when the API omits resultsSize — paginator will show a phantom next page. Worth a TODO.

Recommendation

Don't merge as-is (also marked WIP). Address blockers 1–3, publish a TipTap v3 round-trip plan against existing stored content, and add at minimum smoke tests over the CVA contract, the upload pipeline, and the slash menu before this becomes the default editor.


🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Block Editor v2

2 participants