fix(page-templates): tree marker (#38), embed chrome (#39), embed refresh (#40) #45

Merged
Ghost merged 12 commits from fix/page-template-demo-issues into develop 2026-06-21 01:51:54 +03:00

Исправления багов/дизайна фичи page templates / page embed, найденных при ручной проверке после мержа PR #17. Одна ветка от develop, по коммиту на баг. Closes #38, #39, #40.

Что сделано

#38 — маркер шаблона в дереве (859223db)

Страницы-шаблоны были неотличимы в сайдбаре. Добавлен значок IconTemplate рядом с заголовком строки, когда node.isTemplate === true, в Tooltip(label="Template") + aria-label/role="img". Значок — потомок строки-ссылки, поэтому клик навигирует как обычно; pointer-events оставлены включёнными (иначе тултип Mantine не срабатывает). Добавлен i18n-ключ Template (en-US, ru-RU).

#39 — чистка хрома узла page-embed (c9eb4956)

  • Двойная рамка: квадратный голубой outline от .ProseMirror-selectednode накладывался на скруглённую рамку .includeWrap. Узел pageEmbed добавлен в уже существующее правило outline:none (рядом с transclusion-узлами) — остаётся одна скруглённая рамка.
  • Дублирующие кнопки: кнопка external-link в плавающей панели дублировала ссылку-заголовок бейджа. Убрана (остались Refresh + ⋯-меню); заголовок-ссылка — единственный способ открыть источник. Фолбэк-иконка бейджа IconArrowsMaximize (читалась как «развернуть») заменена на нейтральную IconFileText.
  • По замечанию ревью: бейдж теперь рендерится при наличии sourceHref (ссылка не исчезает при пустых title+icon); ссылке добавлены title/aria-label + ключ Open source page (en-US, ru-RU).

#40 — Refresh эмбеда реально перерисовывает контент (b8655ae5)

Корень: read-only под-рендер монтирует Tiptap EditorProvider, а тот читает content только при первом маунте — после Refresh свежий контент не доходил до под-редактора, эмбед «не обновлялся». Фикс: key={result.sourceUpdatedAt} на PageEmbedContent — при изменении исходной страницы (её updatedAt бампится при каждом сохранении) компонент перемонтируется и применяет свежий контент.
Ограничение: серверная свежесть относительно живых collab-правок ограничена debounce-персистом 10с (collaboration.gateway.ts) — это отдельная задача, остаётся задокументированной в #40, вне scope этого фикса.

Как рассуждал

  • #40 изначально выглядел как серверная проблема (lag персиста). Эмпирически подтвердил, что сервер отдаёт свежий pages.content сразу после сохранения (REST), значит «не обновляется даже после Refresh» — это клиент. Нашёл, что EditorProvider не реагирует на смену content-пропа → ремаунт по sourceUpdatedAt.
  • Для #38/#39 использовал уже существующие в коде паттерны (правило outline:none для transclusion-узлов; иконки Tabler из соседних файлов), чтобы изменения были минимальны и консистентны.

Косяки, найденные на ревью (и исправленные)

  • #38: pointerEvents:"none" полностью отключал Tooltip (Mantine вешает hover-хендлеры на сам SVG) → убрал; клики и так проваливаются на строку-ссылку. Отсутствовал i18n-ключ Template → добавил (en-US, ru-RU). Дублирование aria-label+title+Tooltip → оставил aria-label+role="img"+Tooltip, убрал нативный title.
  • #39: при пустых title+icon бейдж не рендерился → терялся единственный способ открыть источник → рендерю бейдж при sourceHref; добавил подпись ссылке + ключ Open source page.
  • #40: ревью — APPROVE; единственная находка (теоретический ложноотрицательный ремаунт при двух сохранениях в одну миллисекунду) признана пограничной, оставлена как есть.

Проверка

pnpm --filter @docmost/editor-ext build && pnpm --filter ./apps/client exec tsc --noEmitexit 0 после каждого фикса.

Issue #40 содержит матрицу из 12 веб-сценариев для дальнейшего ручного тестинга (вложенность, циклы, права, шара, конкурентные правки) — её стоит прогнать отдельно.

🤖 Generated with Claude Code

Исправления багов/дизайна фичи **page templates / page embed**, найденных при ручной проверке после мержа PR #17. Одна ветка от `develop`, по коммиту на баг. Closes #38, #39, #40. ## Что сделано ### #38 — маркер шаблона в дереве (`859223db`) Страницы-шаблоны были неотличимы в сайдбаре. Добавлен значок `IconTemplate` рядом с заголовком строки, когда `node.isTemplate === true`, в `Tooltip(label="Template")` + `aria-label`/`role="img"`. Значок — потомок строки-ссылки, поэтому клик навигирует как обычно; `pointer-events` оставлены включёнными (иначе тултип Mantine не срабатывает). Добавлен i18n-ключ `Template` (en-US, ru-RU). ### #39 — чистка хрома узла page-embed (`c9eb4956`) - **Двойная рамка:** квадратный голубой `outline` от `.ProseMirror-selectednode` накладывался на скруглённую рамку `.includeWrap`. Узел `pageEmbed` добавлен в уже существующее правило `outline:none` (рядом с transclusion-узлами) — остаётся одна скруглённая рамка. - **Дублирующие кнопки:** кнопка external-link в плавающей панели дублировала ссылку-заголовок бейджа. Убрана (остались Refresh + ⋯-меню); заголовок-ссылка — единственный способ открыть источник. Фолбэк-иконка бейджа `IconArrowsMaximize` (читалась как «развернуть») заменена на нейтральную `IconFileText`. - По замечанию ревью: бейдж теперь рендерится при наличии `sourceHref` (ссылка не исчезает при пустых title+icon); ссылке добавлены `title`/`aria-label` + ключ `Open source page` (en-US, ru-RU). ### #40 — Refresh эмбеда реально перерисовывает контент (`b8655ae5`) Корень: read-only под-рендер монтирует Tiptap `EditorProvider`, а тот читает `content` только при первом маунте — после Refresh свежий контент не доходил до под-редактора, эмбед «не обновлялся». Фикс: `key={result.sourceUpdatedAt}` на `PageEmbedContent` — при изменении исходной страницы (её `updatedAt` бампится при каждом сохранении) компонент перемонтируется и применяет свежий контент. Ограничение: серверная свежесть относительно живых collab-правок ограничена debounce-персистом 10с (`collaboration.gateway.ts`) — это отдельная задача, остаётся задокументированной в #40, вне scope этого фикса. ## Как рассуждал - #40 изначально выглядел как серверная проблема (lag персиста). Эмпирически подтвердил, что сервер отдаёт свежий `pages.content` сразу после сохранения (REST), значит «не обновляется даже после Refresh» — это клиент. Нашёл, что `EditorProvider` не реагирует на смену `content`-пропа → ремаунт по `sourceUpdatedAt`. - Для #38/#39 использовал уже существующие в коде паттерны (правило `outline:none` для transclusion-узлов; иконки Tabler из соседних файлов), чтобы изменения были минимальны и консистентны. ## Косяки, найденные на ревью (и исправленные) - **#38:** `pointerEvents:"none"` полностью отключал Tooltip (Mantine вешает hover-хендлеры на сам SVG) → убрал; клики и так проваливаются на строку-ссылку. Отсутствовал i18n-ключ `Template` → добавил (en-US, ru-RU). Дублирование `aria-label`+`title`+`Tooltip` → оставил `aria-label`+`role="img"`+`Tooltip`, убрал нативный `title`. - **#39:** при пустых title+icon бейдж не рендерился → терялся единственный способ открыть источник → рендерю бейдж при `sourceHref`; добавил подпись ссылке + ключ `Open source page`. - **#40:** ревью — APPROVE; единственная находка (теоретический ложноотрицательный ремаунт при двух сохранениях в одну миллисекунду) признана пограничной, оставлена как есть. ## Проверка `pnpm --filter @docmost/editor-ext build && pnpm --filter ./apps/client exec tsc --noEmit` — **exit 0** после каждого фикса. > Issue #40 содержит матрицу из 12 веб-сценариев для дальнейшего ручного тестинга (вложенность, циклы, права, шара, конкурентные правки) — её стоит прогнать отдельно. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 3 commits 2026-06-20 21:27:41 +03:00
Template pages were toggleable but indistinguishable in the sidebar tree.
Render an IconTemplate next to the title when node.isTemplate is true, wrapped
in a Tooltip(label='Template') with an aria-label + role='img' for AT. The
icon is a child of the row Link so clicks navigate as normal; pointer events
stay enabled so the tooltip's hover handlers fire. Adds the 'Template' i18n
key to en-US and ru-RU (other locales fall back to en-US).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two design problems on the whole-page embed (pageEmbed) node:

- Double selection frame: the generic square cyan .ProseMirror-selectednode
  outline stacked on top of the rounded .includeWrap border. Add node-pageEmbed
  to the existing outline:none rule (already covering the transclusion nodes) so
  only the single rounded border remains.
- Redundant 'open source' controls: the floating toolbar's external-link button
  duplicated the header badge title link. Remove the toolbar button; the badge
  title is now the single way to open the source (kept Refresh + ... menu).
  Also swap the badge fallback icon IconArrowsMaximize (read as 'expand') for a
  neutral IconFileText.

Follow-ups from review: render the badge whenever the source resolves (so the
only open-source link can't vanish when title+icon are empty), and label the
link (title/aria-label) + add the 'Open source page' i18n key (en-US, ru-RU).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The read-only embed renderer mounts a Tiptap EditorProvider with the looked-up
content, but Tiptap consumes the `content` option only at initial mount. After
Refresh busted the lookup cache and re-fetched fresh content, the new content
prop never reached the sub-editor, so the embed appeared not to update at all.

Key PageEmbedContent on result.sourceUpdatedAt (the source page's updatedAt,
already returned by the lookup and bumped on every persisted content change) so
the component and its EditorProvider remount and apply the refreshed content
when the source changes.

Note: server-side freshness vs. live collab edits is bounded by the 10s persist
debounce (collaboration.gateway.ts) — that separate limitation stays documented
in #40 and is out of scope here; this commit fixes the client never re-rendering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ghost added 4 commits 2026-06-20 21:43:27 +03:00
In the page-embed lookup flush(), the success branch cleared inFlightRef and
resolved waiters only for ids present in the response items. A short/partial
server response would leave a requested id stuck in inFlightRef forever (the
subscribe/refresh path is guarded by !inFlightRef.has(id)) and its refresh()
promise would never resolve. After processing returned items, also clear +
resolve any requested id that wasn't returned, mirroring the catch branch.
Cannot trigger under today's exact-mapping server contract; this is hardening.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace bare //@ts-ignore (no space, no reason) with // @ts-expect-error plus a
reason on the pageEmbed sourcePageId reassignment, matching the codebase style.
ProseMirror Attrs is read-only typed, so the reassignment genuinely errors —
@ts-expect-error is valid here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'used in N pages' reverse-navigation method had zero callers in the merged
PR #17 — unreachable, untested code. Remove it. The reverse-navigation feature
can be (re)added with the method if/when it's actually built.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consistency hardening from #17 review (not currently exploitable):
- toggleTemplate now explicitly rejects a page outside the caller's workspace
  (page.workspaceId !== user.workspaceId -> NotFound, avoiding existence leak)
  instead of relying solely on the space-membership model.
- PageTemplateReferencesRepo.deleteByReferenceAndSources is now workspace-scoped
  (adds a workspaceId filter + param), matching the 'scope by workspaceId
  everywhere' invariant; the sole caller threads its workspaceId.
The PAGE_TEMPLATE_THROTTLER limit is intentionally left as-is (the issue's
throttle item was 'consider only'; no change without usage data).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ghost added 4 commits 2026-06-20 22:38:14 +03:00
The transclusion specs predated two added constructor params, so they failed to
compile (TS2554: expected 11 args, got 10) and the suites couldn't run. Add the
missing mock args: workspaceRepo (param 11) in the lookup/access specs, and
pageTemplateReferencesRepo (param 4, which had shifted pageRepo into the wrong
slot) in the unsync-html-embed spec. All three suites now compile and pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cycle/self-embed/depth guard (PAGE_EMBED_MAX_DEPTH=5) lives only on the
client and is the sole protection against runaway nested rendering — and was
untested. Extract the inline predicates into pure, behavior-identical exported
helpers (isPageEmbedCycle, isPageEmbedTooDeep in the ancestry context;
filterPageEmbedOptions in the picker) so they're unit-testable without mounting
the heavy Tiptap NodeView, and add vitest coverage (20 tests): ancestry chain/
host accumulation, cycle (ancestor-in-chain + top-level self-embed), too-deep at
the cap, and picker host-exclusion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the per-node pageEmbed remap decision into a shared pure helper
(remapPageEmbedSourceId) and use it BOTH in PageService.duplicatePage and the
JSON walker, so the test guards the real production path (not a mirror that
could drift). Behavior is identical: source in the copied set -> new copy id;
otherwise keep the original. Add jest coverage (16 tests): the remap helper
(in-set/out-of-set/null/nested), syncPageTemplateReferences toDelete (stale refs
removed with the right workspaceId), and insertTemplateReferencesForPages
multi-workspace grouping/filtering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The security-relevant catch->not_found branch in lookupTemplate (returns
  not_found instead of raw content when comment-mark stripping throws) is now
  tested by forcing the strip to throw with a malformed text node, asserting no
  content/marks leak.
- not_found for a soft-deleted source resolved through the REAL
  filterViewerAccessiblePageIds (deletedAt-excluded), not the stubbed filter.
- Rename the misleading 'honours <=50 cap' test to reflect it only exercises
  dedup (the cap lives in the DTO, never engaged in the service unit).
- Cover the onlyTemplates search filter (restricts to is_template=true).
Also fix two pre-existing failing 'should be defined' specs (search service +
controller) that couldn't resolve the @InjectKysely token via createTestingModule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vvzvlad added 1 commit 2026-06-21 01:51:21 +03:00
Merge develop into fix/page-template-demo-issues (#45)
Some checks failed
Test / test (pull_request) Has been cancelled
d7681b4fb6
Resolve conflicts from the parallel page-embed refactor that landed in develop
via #49:
- page-embed-view.tsx: keep develop's canonical decideEmbedState for the
  cycle/depth/availability guard; keep #45's #39 chrome cleanup (single source
  link, IconFileText fallback) and #40 refresh remount key. Drop #45's now-unused
  isPageEmbedCycle/isPageEmbedTooDeep wiring.
- page-embed-picker.tsx: use develop's excludeHost util; drop #45's duplicate
  filterPageEmbedOptions and its test.
- page-embed-ancestry-context.test.tsx: keep #45's superset suite.
- page-template-access.spec.ts: keep develop's constructor args; update the two
  deleteByReferenceAndSources assertions to the new 4-arg workspace-scoped
  signature introduced by #45 (#36 defense-in-depth).

Full suite green: server 624, client 219, editor-ext 56, mcp 247.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ghost merged commit 0944e0f455 into develop 2026-06-21 01:51:54 +03:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#45