Skip to content

Refactor: Block Editor#35257

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

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

Conversation

@rjvelazco
Copy link
Copy Markdown
Member

@rjvelazco rjvelazco commented Apr 8, 2026

WIP

Videos

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 18s —— View job


PR Review: Refactor Block Editor

  • Read key files and analyze the PR

Issues Found

🔴 HIGH — JWT token committed to git history

core-web/libs/new-block-editor/src/lib/editor/services/dot-cms.config.ts:6-7

A full JWT (expires 2029) is hardcoded and consumed by all three services (DotCmsUploadService, DotCmsContentletService, DotCmsContentTypeService) as Authorization: Bearer headers. The comment calls it "local demo," but git history is permanent — the token is now publicly disclosed. The token needs to be revoked regardless of whether this is merged. Future pattern should inject the token via an InjectionToken configured by the host app.


🔴 HIGH — allowedBlocks input is never read when extensions are created

core-web/libs/new-block-editor/src/lib/editor/editor.component.ts:164

readonly editor: Editor = new Editor({
    extensions: createEditorExtensions(this.menuService, this.allowedBlocks()),
    //                                                   ^^^^^^^^^^^^^^^^^^^
    // Field initializer runs during construction — Angular signal inputs
    // have not been set by the parent yet; this always returns undefined.

The class field initializer runs before Angular assigns signal inputs from the parent. this.allowedBlocks() is always undefined here, so heading, bulletList, orderedList, blockquote, codeBlock, and horizontalRule are always registered regardless of what allowedBlocks the host passes. The effect() correctly syncs the slash menu filter later, but that only hides items from the UI — users can still trigger excluded block types via keyboard shortcuts. The extensions themselves never respect allowedBlocks.


🟡 MEDIUM — Commented-out state in openSubmenu() and setItems() leaves the menu broken

core-web/libs/new-block-editor/src/lib/editor/slash-menu/slash-menu.service.ts:164-166, 178-179

openSubmenu(): void {
    // this.items.set([]);        // stale items stay visible while loading
    // this.activeIndex.set(0);
    // this.isLoading.set(true);  // spinner never shown
}

setItems(items: BlockItem[], commandFn): void {
    this.items.set(items);
    // this.activeIndex.set(0);   // Enter can pick out-of-bounds if new list is shorter
    // this.isLoading.set(false);
}

Three concrete bugs:

  • Stale items from the previous menu remain visible during the async content-type fetch.
  • isLoading is never set to true (only set to false in close()), so the loading spinner is dead UI.
  • activeIndex is not reset when setItems fires — if the previous activeIndex is 3 and the new list has 2 items, Enter calls this.items()[3] which is undefined, then this.commandFn?.(undefined) which silently no-ops.

🟡 MEDIUM — toPromise() is deprecated/removed

core-web/libs/new-block-editor/src/lib/editor/services/dot-cms-upload.service.ts:30,34

const tempId = await this.uploadToTemp(file).pipe(take(1)).toPromise();
const url    = await this.publishAsset(tempId).pipe(take(1)).toPromise();

toPromise() was deprecated in RxJS 7 and dropped in RxJS 8. Replace with firstValueFrom():

import { firstValueFrom } from 'rxjs';
const tempId = await firstValueFrom(this.uploadToTemp(file));

🟡 MEDIUM — Removing shared code styles may break other consumers

core-web/libs/dotcms-scss/angular/styles.scss (8 lines deleted)

The code element styles (color, background, padding, font-family, line-break) were removed from a shared stylesheet imported across the entire admin UI, not just the block editor. Any app that was relying on this global style for inline code rendering will silently regress. Needs a grep across consuming apps before deletion.


🟡 MEDIUM — No content I/O contract on EditorComponent

core-web/libs/new-block-editor/src/lib/editor/editor.component.ts:165

EditorComponent initializes with hardcoded EDITOR_DEMO_CONTENT and has no content = input(), no contentChange = output(), and no ControlValueAccessor. There is no way for the host app (dotcms-block-editor) to pass in or read back the document content. This is presumably a WIP gap, but it means the library cannot replace the existing block editor in production until this is added.


🟠 LOW — Scaffolding files not cleaned up; app.spec.ts will fail

core-web/libs/new-block-editor/src/lib/app.spec.ts:22

expect(compiled.querySelector('h1')?.textContent).toContain('Hello, block-editor');

The App component doesn't render this. This test fails if run. app.config.ts and app.routes.ts are also not imported anywhere — leftover Nx generator output.


🟠 LOW — hostFolder: '' in asset publish

core-web/libs/new-block-editor/src/lib/editor/services/dot-cms-upload.service.ts:70

Empty hostFolder when publishing a dotAsset defaults to whatever the API resolves as the default host. In multi-site instances this is unpredictable and may silently publish to the wrong site.


Not an issue (addressed by Cursor Bugbot but context matters)

The floating dialog duplication (floatX/floatY/positioning logic in table/image/video dialogs) is already tracked in CLAUDE.md under "Deferred Refactors" with a documented trigger condition. Leave it.

@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.

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.

*/
export const DOT_CMS_BASE_URL = 'http://localhost:8080';
export const DOT_CMS_AUTH_TOKEN =
'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJhcGljNjI1Yjg1NC0zYzc2LTRjMjItYTc0Yy00MWI1M2NkYmYwMzkiLCJ4bW9kIjoxNzc1NzY3MDM0MDAwLCJuYmYiOjE3NzU3NjcwMzQsImlzcyI6ImRvdGNtcy1wcm9kdWN0aW9uIiwibGFiZWwiOiJkZXYiLCJleHAiOjE4NzA0MDE2MDAsImlhdCI6MTc3NTc2NzAzNCwianRpIjoiOGI1M2VmNmYtNzA4OS00NThmLThjMjQtNDMzN2Y1MmNiMGRmIn0.4Y4SMqhMDG0vJ4xbMTZ2AtSAIeyB5NEgZ7yIUMWkASg';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hardcoded JWT auth token committed to repository

High Severity

A full JWT token is hardcoded in DOT_CMS_AUTH_TOKEN and will be committed to the repository. Even though the comment says "local demo," this token is used by multiple services (DotCmsUploadService, DotCmsContentletService, DotCmsContentTypeService) for Authorization: Bearer headers against a dotCMS instance. This token has an expiry date in 2029 and is now exposed in version control history permanently.

Fix in Cursor Fix in Web

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

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

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.

Refactor: Establish Block Editor Angular Standards and Documentation

1 participant