Skip to content

feat(dev-tools): persist last query + unify theming across dev-tool portlets (#36692) - #36781

Open
AP2300 wants to merge 1 commit into
mainfrom
issue-36692-dev-tool-consistency
Open

feat(dev-tools): persist last query + unify theming across dev-tool portlets (#36692)#36781
AP2300 wants to merge 1 commit into
mainfrom
issue-36692-dev-tool-consistency

Conversation

@AP2300

@AP2300 AP2300 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #36692. Two consistency gaps across the migrated dev-tool portlets (dot-query-tool, dot-es-search, dot-velocity-playground), addressed together.

  • Persist last query per portlet — new shared withPersistedQuery() SignalStore feature in libs/data-access/dot-localstorage/ (backed by pure readJson/writeJson/removeKey helpers, SSR-safe, error-swallowing). Storage key convention: dotcms.devtools.{portletKey}.lastQuery. Hydrates on init, debounces writes at 300 ms, exposes a clearPersistedQuery() action. Wired into all three stores.
  • Clear-query button in each editor's panel header (pi pi-eraser, tooltip via | dm, disabled-when-empty, no confirmation — conscious action per issue discussion). Sits inline next to Help so it doesn't fight PrimeNG's pTemplate="icons" slot on portlets that also have a Wrap-code checkbox.
  • Removed dark Monaco theme from dot-velocity-playground — kept the VTL syntax tokenizer, dropped the hardcoded dot-velocity-dark theme. Editor now inherits DOT_MONACO_BASE_OPTIONS' theme: 'vs', the same light theme used by the other two dev tools.
  • Reconciled velocity persistence — the implicit "on init, prime code from history[0]" was a duplicate persistence path for the query. Removed; the new persisted-query key is now the single source of truth. History remains for the multi-entry selector.
  • i18n: added queryTool.action.clearQuery, esSearch.action.clearQuery, velocityPlayground.action.clearQuery to Language.properties.

Acceptance criteria coverage

  • Dark theme removed from dot-velocity-playground (theme definition + defineTheme/setTheme calls + theme: bindings)
  • All three portlets render with the same default Monaco theme
  • Shared persistence helper in libs/data-access/ (not duplicated)
  • Restores last query on load
  • Saves on change (debounced 300 ms)
  • Per-portlet Clear action
  • Storage keys follow dotcms.devtools.{portlet}.lastQuery
  • Velocity's existing localStorage (history, splitter, wrap) reconciled — no duplicate storage of the query
  • Unit tests: load-on-init, debounced-write, clear, corrupt/missing storage

Test plan

  • Type a query in each portlet, reload the page, confirm the query is restored
  • Click Clear query, confirm the editor empties and reload leaves it empty (localStorage entry gone)
  • Rapid typing writes only the final value to localStorage after ~300 ms (Application → Local Storage in devtools)
  • Velocity editor renders with the shared light theme, VTL syntax still highlighted (directives, variables, strings)
  • Wrap-code checkbox on es-search / velocity headers renders on the right without wrapping
  • Manually corrupt the stored value ({not-json) and confirm the portlet loads with an empty editor rather than erroring

🤖 Generated with Claude Code

… portlets

- Add shared `withPersistedQuery` SignalStore feature + pure helpers in
  `libs/data-access/dot-localstorage/` — hydrate on init, debounced write
  (300ms), and `clearPersistedQuery()` method backed by
  `dotcms.devtools.{portletKey}.lastQuery`.
- Wire the feature into `dot-query-tool`, `dot-es-search`, and
  `dot-velocity-playground` stores; add a "Clear query" button to each
  editor's panel header (`pi-eraser` icon, disabled-when-empty).
- Remove the hardcoded dark Monaco theme from `dot-velocity-playground`;
  keep the VTL syntax tokenizer. Editor now inherits the shared light
  theme (`vs`) used by the other two dev tools.
- Drop the implicit "code = history[0] on init" in velocity — persisted
  query key is the single source of truth per AC.
- Add i18n keys `queryTool.action.clearQuery`, `esSearch.action.clearQuery`,
  `velocityPlayground.action.clearQuery`.
- Unit tests for utils, the feature (with fake-timer debounce), and each
  store's hydrate + clear integration.

Fixes #36692

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300
AP2300 force-pushed the issue-36692-dev-tool-consistency branch from c7124c7 to 547a1b6 Compare July 30, 2026 18:31
@AP2300 AP2300 self-assigned this Jul 30, 2026
* fallback in non-browser environments, when the key is missing, or when the
* payload can't be parsed.
*/
export const readJson = <T>(key: string, fallback: T): T => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These three helpers (readJson / writeJson / removeKey) already exist in libs/portlets/dot-velocity-playground/src/lib/dot-velocity-playground.utils.ts (lines 28-57) with identical implementations — same SSR guard, same try/catch, same JSON parse/stringify.

Since one AC of this PR is "Shared persistence helper in libs/data-access/ (not duplicated)", it would be worth finishing the consolidation: keep the copies here as the single source, drop them from the velocity utils, and have the velocity store import readJson / writeJson / removeKey from @dotcms/data-access for its HISTORY / SPLITTER / WRAP keys. Otherwise the same three functions live in two places inside the very portlet the AC targets.

* localStorage entry. The debounced-write effect will observe the
* empty value but the removeKey call has already cleared storage.
*/
clearPersistedQuery(): void {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Small race worth double-checking: the state change here (patchField(store, '')) re-fires the tracked effect below, which schedules a setTimeout(() => writeJson(storageKey, ''), 300). 300 ms later, writeJson writes "" (JSON-encoded empty string) back to storage — the entry removeKey just deleted gets re-created.

User-visible impact is small (on reload, stored.length > 0 is false → initial state kept), but it contradicts the docstring's "removeKey call has already cleared storage" and the unit test's getItem(persistedKey) === null assertion. The test passes only because it doesn't advanceTimersByTime(300) after clearPersistedQuery().

Two-line fix inside the effect works — skip the write when value === '' (and removeKey there too for safety) — plus a regression test that advances timers past the debounce window.

return;
}

const timer = setTimeout(() => writeJson(storageKey, value), debounceMs);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

try to avoid use setTimeout here

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

Labels

Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Dev tools consistency: remove dark mode from velocity-playground and persist last query across query-tool, es-search, and velocity-playground

2 participants