Skip to content

fix(block-editor): server-side search for content-type & contentlet pickers - #36488

Merged
rjvelazco merged 11 commits into
mainfrom
issue-36487-block-editor-content-type-picker-40-limit
Jul 10, 2026
Merged

fix(block-editor): server-side search for content-type & contentlet pickers#36488
rjvelazco merged 11 commits into
mainfrom
issue-36487-block-editor-content-type-picker-40-limit

Conversation

@rjvelazco

@rjvelazco rjvelazco commented Jul 9, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Fixes #36487 — the new Block Editor's content-type picker only showed the first 40 content types (alphabetical) and couldn't find the rest by scroll or search; the contentlet drill-down had the same limitation.

Root cause: the slash-menu submenu fetched once with an empty query (perPage: 40) and filtered client-side. The legacy editor re-queried the server on each keystroke, so any record was findable. This restores that behavior.

What changed:

  • SlashMenuService — new openAsyncSubmenu(search, commandFn): rows come from a debounced (250ms) server search that re-runs on every keystroke. Includes a stale-response token guard, consecutive-query dedupe, and timer/level-switch/close teardown. filterItems routes to the server search when in async mode. Removed the now-dead openSubmenu() / setItems() (the one-shot-fetch + client-filter path).
  • slash-menu-catalog — both picker levels use openAsyncSubmenu. buildContentletByTypeQuery now accepts the search term and builds a Lucene-escaped, tokenized query (ported from the legacy getContentlets, incl. UUID handling), so contentlet search is server-side too.
  • Sub-menu re-anchor fix (follow-up commit) — update() now refreshes the caret anchor (clientRectFn) in sub-menu mode too. Previously it was discarded there, so once the query text changed while searching, the overlay stayed pinned to the root menu's now-detached suggestion decoration node (getBoundingClientRect() → ~0,0) and the menu jumped to the top-left of the editor. A pre-existing bug that the new search box surfaced (multi-word queries in the sub-menu).

No fixed page-size cap → scales to instances with 2000+ content types. Empty/error/loading states, keyboard nav, and node insertion are preserved.

Checklist

  • nx run new-block-editor:lint passes (type-checked)
  • Server-side debounced search wired for both picker levels (content types + contentlets)
  • Sub-menu overlay re-anchors to the caret while searching (no top-left jump)
  • Manual verification pending — needs a running instance with >40 content types:
    • /Content → confirm the full type list searches server-side (types past the old 40-cap, e.g. MM… / RTL…, are findable)
    • pick a type → confirm contentlet search works server-side
    • type a multi-word query → confirm the menu stays anchored under the caret (positioning fix)
    • verify rapid typing (debounce / stale-response) behaves
    • (Could not runtime-verify in the authoring env: no local nx serve, no seeded backend.)

Additional info

🤖 Generated with Claude Code

This PR fixes: #36487

…ntentlet pickers (#36487)

The slash-menu content-type picker fetched only the first 40 types (perPage:40,
name ASC) and filtered that set client-side, hiding every content type past the
alphabetical cap ("list ends after Image Asset") — unreachable by scroll or search.
The contentlet drill-down had the same one-shot-fetch + client-filter limitation.

Restore the legacy editor's behavior: re-query the server (debounced) on each
keystroke, so results aren't capped by a page size and scale to 2000+ content types.

- SlashMenuService: add openAsyncSubmenu() — 250ms debounce, stale-response token
  guard, query dedupe, timer/level-switch teardown; filterItems routes to the server
  search in async mode. Remove the now-dead openSubmenu()/setItems() (one-shot path).
- slash-menu-catalog: both picker levels use openAsyncSubmenu. buildContentletByTypeQuery
  now takes the search term and builds a Lucene-escaped, tokenized query (ported from
  the legacy getContentlets, including UUID handling).

Reported via Freshdesk #38277.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rjvelazco's task in 13m 8s —— View job


Code Review

Reviewed the full diff against origin/main — service state machine, catalog query builder, the sub-menu re-anchor fix, and both new specs. The core design (debounced server search, token guard, dedupe, teardown on close()) is sound and the query builder faithfully mirrors the legacy getContentlets Lucene logic. Kevin's brace-style note is addressed (commit 45cc25a6).

One non-blocking gap below.

New Issues

  • 🟡 Medium: slash-menu.service.ts:172 / 291-305Pending debounce timer + in-flight search are not torn down when the editor is destroyed. close() clears submenuSearchTimer and bumps the token, but the service has no OnDestroy and detachEditor() (called from the slash extension's onDestroy) doesn't touch the timer. If the editor is destroyed while a sub-menu search is debouncing (user typing in the picker, then the field/editor unmounts), the setTimeout still fires ~250ms later → runSubmenuSearch calls search(query) (an HTTP request) and the stale guard passes because isInSubmenu is still true and the token still matches — so it sets signals on a service belonging to a destroyed component. Orphaned request + post-destroy state write. Low frequency, but the fix is small: clear the timer (and bump the token) in detachEditor(), or implement OnDestroy and call the same teardown as close(). Fix this →

Notes (non-blocking)

  • slash-menu.service.ts:299scheduleSubmenuSearch flips isLoading to true immediately, so the spinner shows for the full 250ms debounce while the previous rows stay visible. Acceptable UX; flagging only so it's intentional.
  • slash-menu-catalog.ts:47-49escapeLucene/UUID_LIKE/LUCENE_SPECIAL_CHARS are duplicated verbatim from the legacy suggestions.service.ts. The PR description already acknowledges this is deliberate pending a shared @dotcms/data-access util — no action needed now, but worth a tracking ticket so the two copies don't drift.
  • Test coverage is good: the query-builder spec pins the exact Lucene strings (UUID, multi-word, hyphen, escaping, trim) and the service spec exercises the debounce, dedupe, and the out-of-order stale-response guard. The one path not covered is the destroy/teardown case in the finding above.

Everything else — the token/isInSubmenu stale guard, the empty/error row handling, keywords[0] variable round-trip, and the update() caret re-anchor — checks out.

issue-36487-block-editor-content-type-picker-40-limit

@rjvelazco rjvelazco changed the title fix(block-editor): server-side search for content-type & contentlet pickers (#36487) fix(block-editor): server-side search for content-type & contentlet pickers Jul 9, 2026
The content-type / contentlet sub-menu discarded the fresh caret rect on each
keystroke — update() set items but not clientRectFn in sub-menu mode — so the
overlay kept the ROOT menu's original suggestion decoration node. Once the query
text changes during a search, that node detaches and getBoundingClientRect()
returns ~(0,0), stranding the menu at the top-left of the editor.

Refresh clientRectFn in the sub-menu branch too (still preserving the sub-menu's
own commandFn) so the overlay follows the caret exactly like the root menu.

Pre-existing bug surfaced by the new server-side search, which turns the sub-menu
into a real search box where longer queries are typed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rjvelazco

Copy link
Copy Markdown
Member Author

@claude can you review this code, pretty please

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review in progress

  • Gather context (PR diff, files, description)
  • Read slash-menu.service.ts changes
  • Read slash-menu-catalog.ts changes
  • Review for bugs, security, performance, maintainability
  • Post review feedback

View job run

Addresses the PR review's one actionable finding (missing coverage for the parts
most prone to silent regression). First specs in new-block-editor.

- slash-menu-catalog.spec.ts: buildContentletByTypeQuery — empty, single/multi-word,
  hyphenated, UUID, Lucene-escaping, and trimming cases (mirrors the legacy
  suggestions.service spec). Exported the builder for direct testing.
- slash-menu.service.spec.ts: openAsyncSubmenu — initial search, 250ms debounce
  coalescing, consecutive-query dedupe, stale-response token guard (out-of-order
  resolution), and close() dropping a late response.

12 tests, all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rjvelazco

Copy link
Copy Markdown
Member Author

Thanks for the review. Addressed the one actionable finding — added the first specs in new-block-editor (commit adds slash-menu-catalog.spec.ts + slash-menu.service.spec.ts, 12 tests, all green):

  • buildContentletByTypeQuery — asserts the exact Lucene strings for empty, single-word, multi-word, hyphenated, UUID, special-char escaping, and trimming (mirrors the legacy suggestions.service spec). Exported the builder for direct testing.
  • SlashMenuService.openAsyncSubmenujest.useFakeTimers() tests for the initial search, 250 ms debounce coalescing, consecutive-query dedupe, the stale-response token guard (out-of-order resolution — the exact off-by-one risk you called out), and close() dropping a late response.

Re your notes: agreed on all — query-builder parity, escaping, and the async state machine were the design intent, and the escapeLucene/UUID_LIKE duplication is deliberate (no legacy-lib dependency) pending a shared @dotcms/data-access util. Manual runtime verification (positioning + >40-type search) is still on the checklist for a reviewer with a seeded instance.

…spec

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rjvelazco
rjvelazco enabled auto-merge July 10, 2026 00:05
@rjvelazco
rjvelazco added this pull request to the merge queue Jul 10, 2026
Per review feedback — use braces on if statements even when the body is one line.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mergify

mergify Bot commented Jul 10, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@rjvelazco
rjvelazco removed this pull request from the merge queue due to a manual request Jul 10, 2026
@rjvelazco
rjvelazco enabled auto-merge July 10, 2026 00:12
…esolution)

The slash-menu service spec imported from @ngneat/spectator/jest, which is
no longer a declared dependency — the workspace migrated to the @openng/spectator
fork. It passed locally only because a stale @ngneat package lingered in
node_modules; CI's clean install has only @OpenNg, so the suite failed to run
with 'Cannot find module @ngneat/spectator/jest'.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rjvelazco
rjvelazco added this pull request to the merge queue Jul 10, 2026
Merged via the queue into main with commit 57a25b2 Jul 10, 2026
42 checks passed
@rjvelazco
rjvelazco deleted the issue-36487-block-editor-content-type-picker-40-limit branch July 10, 2026 21:35
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: content-type picker only shows the first 40 types; search can't find the rest

2 participants