feat(comment): hover tooltip with comment text over comment marks (#268) #271
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#271
Reference in New Issue
Block a user
Delete Branch "feat/268-comment-hover"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Всплывающая подсказка с текстом комментария при наведении на жёлтую подсветку
.comment-mark. closes #268apps/client/src/features/comment/components/comment-hover-preview.tsx— смонтирован вpage-editor.tsxрядом с<EditorContent>. По наведению на.comment-mark[data-comment-id]показывает маленькую плавающую карточку (createPortalв body,position:fixed,pointer-events:none— никогда не перехватывает клик марки → боковая панель открывается как раньше) с plain-text родительского комментария.useCommentsQuery({pageId})(общий кеш["comments", pageId]с панелью → без лишнего запроса), индексMap<id, IComment>через ref (свежие данные без переподключения слушателей).data-resolvedИЛИresolvedAt/resolvedById), пустой текст.mouseover/mouseoutна контейнере +scroll(capture)/resize/mousedownна закрытие; задержка ~120мс против мерцания; сброс при сменеpageId. Полная очистка слушателей и таймера на unmount.comment-content-to-text.ts(commentContentToText) — плоско разворачивает ProseMirror-JSON (строка ИЛИ объект) в plain-text, сохраняетhardBreakкак\n, не бросает на malformed/empty.editor-extне трогал.How verified
На стенде (apps/client):
vitest run comment-hover-preview.test.tsx— 16/16 (6 на хелпер: flatten, списки, объект, empty/malformed, raw-fallback, hardBreak; 10 на компонент: показ после задержки, скрытие по mouseout/scroll/mousedown/смене страницы, анти-фликер при relatedTarget внутри спана, пропуск resolved×2/unknown/empty).tsc --noEmit— 0;eslint(3 новых файла) — 0.Внутренний ревью (отдельный субагент: lifecycle слушателей, ref-гонка, click-through, positioning, robustness хелпера, качество тестов) — APPROVE WITH SUGGESTIONS, критичных/WARNING нет. Все 5 рекомендаций внёс: hardBreak→
\n, проверку active-span до парсинга, закрытие поmousedown, усилил слабые ассерты и добавил тесты на новые ветки.Checklist
Ревью
a848003db— раунд 1, Full tier (9 аспектов вкл. COHERENCE). Фича #268: всплывающая подсказка с текстом комментария при наведении на.comment-mark.Вердикт: CHANGES (один пункт — low, но он покрывает центральный инвариант фичи). Сама реализация корректна, аккуратна и проверена по коду многими аспектами. Отвечай по id.
Что проверено и ЧИСТО:
position:fixed+pointer-events:noneрендерится вdocument.bodyвне спана; реальный клик марки висит в расширении (packages/editor-ext/.../comment.ts:198, прямойaddEventListener→ACTIVE_COMMENT_EVENT, слушается вpage-editor.tsx), а новыйmousedown→hideтолько чистит таймер/стейт (безpreventDefault/stopPropagation, событие независимое) → клик и боковая панель работают как раньше.menuContainerRef— верный корень делегирования (оборачивает<EditorContent>).data-resolvedреально эмитится расширением (comment.ts:58-68) — скип резолвнутых корректен (атрибут + модельresolvedAt/resolvedById). Общий кеш["comments", pageId]с панелью → дублирующего запроса нет.hide(); эффект слушателей[containerRef](стабильный реф) — без двойного навешивания;commentMapRef-зеркало свежо к моменту hover (в худшем случае суб-тик без тултипа, не неверный тултип); 120ms-таймер гардactiveSpanRef===span && span.isConnectedкорректно дропает заменённый ProseMirror'ом узел; быстрый A→B→A таймер не течёт;position:fixed+getBoundingClientRectконсистентны, scroll→hide убирает устаревший rect.{hover.text}), нетdangerouslySetInnerHTML/innerHTML-стока;commentContentToTextотдаёт только plain-string; тот же авторизованный запрос, что и панель, резолвнутые скрыты — переэкспозиции нет..comment-mark, ничего не отменяют; scroll-capture-слушатель — лишьhide()(no-op без карточки), не конфликтует со scroll-restore (#267)/bubble-menu/drag-handle; компонент возвращает null до hover и порталит в body — лэйаут-сдвига нет; второй потребительuseCommentsQueryдедупится React-Query.commentContentToTextПРЯМО покрыт (test-coverage): отдельныйdescribe(6 кейсов: concat+перевод строк, вложенный bulletList/listItem, parsed-object, empty/malformed→"", non-JSON→raw, hardBreak→\n). Компонент-тест не-вакуозен (реальный компонент, fake timers, замоканныйuseCommentsQuery, настоящие mouseover/out/down/scroll), покрывает show-after-delay/resolved/unknown/mousedown-hide/scroll-hide/intra-span-ignore/empty-text/pageId-change.Tooltip/HoverCardоборачивают React-таргет с рефом — не подходят для hover по ProseMirror-спанам),generateTextпотянул бы всю схему редактора ради тултипа; все комментарии-утверждения сверены с кодом (включаяdata-resolved).Что сделать
F1 [test coverage] Заассертить
pointer-events:none— центральный инвариант фичи (карточка не должна перехватывать клик марки) —apps/client/src/features/comment/components/comment-hover-preview.test.tsx:172Главное требование #268 — тултип НЕ мешает клику по
.comment-mark(открывающему панель). Гарантирует это единственное свойствоpointerEvents:'none'на карточке, и оно НЕ покрыто ни одним ассертом. Карточка порталится вdocument.bodyВНЕ контейнера, поэтому тестmousedown→hideего не защищает: еслиpointerEvents:'none'уберут (одна строка в большом style-объекте), клик по задержавшейся карточке перехватится на ней, до контейнера/марки не дойдёт, панель не откроется — и НИ ОДИН тест не упадёт. Это покрытие самого рискового пути изменения (его собственного главного требования). (Severity low, но фикс тривиален и стережёт несущий инвариант.)Fix: в тест «shows after delay» добавить
expect(screen.getByTestId("comment-hover-preview").style.pointerEvents).toBe("none").⛔ DROP — кодеру НЕ делать (калибровка)
[below-threshold]low/high[test-coverage] flip-позиционирование (placeAbove/left-clamp) +resize→hideне покрыты —comment-hover-preview.tsx:168-179,142: косметика, в jsdomgetBoundingClientRectнулевой (трудно осмысленно тестировать), resize зеркалит покрытый scroll.[below-threshold]low/med[conventions] ручной PM-doc→text walker вместоgenerateText—comment-content-to-text.ts:9: simplification отдельно подтвердил, что walker — верный выбор (generateText связал бы тултип со всей схемой редактора); общего клиентского json→text util нет.[style/linter]low/low[conventions] крупный inline-styleблок vs «Mantine + CSS-modules» —comment-hover-preview.tsx:188-206: сиблинги тоже используют inline-стили, динамика (left/top/bottom) всё равно обязана быть inline; чисто организационно.[superseded]info/low[conventions] ручной портал vs Mantine floating — сам аспект пометил «acceptable»; simplification закрыл.[speculative]low/low[stability] mixed-content ветка walker'а могла бы разбить абзац по строкам —comment-content-to-text.ts:51: недостижимо при inline-схеме ProseMirror этого приложения (inline-дети — text/hardBreak/leaf-атомы без вложенногоcontent), не падает/не зацикливается.Объективные проверки: vitest сам прогнать не могу (нет node_modules в окружении ревью); тесты НЕЗАВИСИМО верифицированы не-вакуозными против реального компонента и util (
commentContentToTextпокрыт прямо). После F1 прогониpnpm --filter client vitest run comment-hover-preview.Маркер
reviewed_headобновлён наa848003db. После правки верниreview/needs.F1: fixed — заассертил центральный инвариант в тесте «shows after delay»:
expect(card.style.pointerEvents).toBe("none"). Это единственное свойство, гарантирующее, что задержавшаяся карточка не перехватит клик по.comment-mark(открывающий панель); раньше оно было непокрыто — регрессия, убравшаяpointerEvents:'none'из style-объекта, не уронила бы ни один тест, теперь — уронит этот.vitest— 16/16,eslint— 0. Возвращаюreview/needs.Ревью
d58fe967a— раунд 2 (правка F1), спотлайт на добавленном ассерте.Вердикт: PASS. Единственный пункт (F1 — непокрытый центральный инвариант) закрыт и сверен по коду. Готово к мержу.
expect(card.style.pointerEvents).toBe("none")— ровно там, где тест уже проверяет, что карточка показалась с верным текстом. Ассерт читает реальный отрендеренный портал-узел; если из style-объекта компонента уберутpointerEvents:'none'(то самое свойство, что гарантирует непрехват клика по.comment-mark→ панель открывается),card.style.pointerEventsстанет""и тест упадёт. Несущий инвариант фичи теперь застрахован.Дельта — только тест (+4 строки), логика не менялась, регрессий нет. Остальная часть PR верифицирована чистой в раунде 1 (9 аспектов: клик не крадётся, data-resolved учтён, общий кеш, без гонок/утечек, security чисто, util покрыт прямо).
Объективные проверки: vitest сам прогнать не могу (нет node_modules в окружении ревью); базис PASS — ассерт НЕЗАВИСИМО верифицирован не-вакуозным против реального компонента (узел из
createPortal, компонент ставитpointerEvents:'none'в style); кодер отчитался оvitest 16/16, eslint 0.Маркер
reviewed_headобновлён наd58fe967a.Per maintainer feedback: show the comment author and the whole thread (parent + replies), but as simple "Author: text" lines — no avatars, timestamps, or thread chrome ("it's already clear they're comments on one entry, one after another"). Also lengthen the open delay so the card doesn't pop up on a passing glance. - Render each comment in the thread as a plain line: bold "Name:" + text, parent first then replies (createdAt asc). Empty-text comments are skipped. - OPEN_DELAY_MS 120 -> 350. - Drop the avatar/relative-time/divider UI (and the CustomAvatar/timeAgo imports). buildThread (root + direct replies) is unchanged — the comment model is flat, so direct children of the root are the full thread. Tests updated to the "Author: text" shape (textContent-based, incl. ordering). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Переделал по твоему замечанию:
OPEN_DELAY_MS120 → 350мс (не выскакивает на пролёте курсора).buildThread (родитель + прямые ответы) оставил — модель комментов плоская (все ответы привязаны к корню), так что прямые дети корня = весь тред.
vitest— 18/18 (обновил ассерты под формат «Автор: текст», включая порядок родитель→ответы);tsc— 0;eslint— 0. Возвращаюreview/needs.Ревью
64a18298e— раунд 3 (доработка по запросу vvzvlad: автор + всё дерево комментариев), Full tier, 9 аспектов вкл. COHERENCE.Вердикт: CHANGES (два пункта, оба low — полировка новой ветки). Доработка сделана верно и цель достигнута; проверено по коду многими аспектами. Отвечай по id.
Что проверено и ЧИСТО:
useCommentsQuery.data.items— плоский список ПО ВСЕМ страницам, ВКЛЮЧАЯ ответы (доказано боковой панелью: родители =parentCommentId===null, ответы фильтруются поparentIdиз ТОГО ЖЕ плоского списка —comment-list-with-tabs.tsx:62,315).buildThreadсканитparentCommentId===root.id→ реально находит ответы. Марка всегда несётdata-comment-idРОДИТЕЛЯ (.setComment(createdComment.id)только для top-level; ответы марок не создают), так чтоbuildThreadвсегда получает корень и всех сиблингов — reply-как-root невозможен. Цель vvzvlad (автор + дерево) выполнена.Author: textчерез Mantine<Text span fw={600}>— как в сиблингеcomment-list-item.tsx;creator?.name— тот же путь, что у панели. Тест не-вакуозен: корень + 2 ответа поданы в ПЕРЕПУТАННОМ порядке (Carol раньше Bob) и ассертит createdAt-asc порядок + все имена + формат — сломанныйsortупал бы. Delay 350 совпадает сadvanceTimersByTime(350).pointerEvents:'none'на месте (:239), и раунд-2 ассертcard.style.pointerEvents === "none"пережил рефактор теста (:195). Стейт-машина (activeSpanRef/openTimer/isConnected), intra-span early-return ДОbuildThread(тяжёлый build не пересчитывается на mousemove), все 5 слушателей и cleanup — не тронуты.HoverState.text→thread— все читатели обновлены,hover.textнигде не осталось.<Text>, нет innerHTML; тот же авторизованный запрос, что и панель; резолвнутые/unknown скипаются ДО buildThread. Comments/simplification/architecture — LGTM (все 5 комментариев сверены с кодом; двойной проход по text — частично неотделим от сигнала «есть ответы»; структурных развилок нет).Что сделать
F1 [stability/coherence] Не показывать ПУСТУЮ карточку, когда ни у корня, ни у ответов нет текста —
apps/client/src/features/comment/components/comment-hover-preview.tsx:145-146hasContent = thread.length > 1 || thread.some(row => row.text.length > 0)открывает карточку по РАЗМЕРУ треда (length>1), а рендер (:243-246) фильтрует поrow.text.length > 0. Достижимый кейс: корень без текста (image-only) И его единственный ответ тоже без текста →length>1истинно → карточка открывается → оба ряда отфильтрованы → рендерится ПУСТОЙ<Paper>(пустая рамка с тенью). Пустой тултип хуже отсутствия тултипа и не отвечает цели «показать автора+текст». Два аспекта (stability+coherence) указали независимо. Дизъюнктlength>1— единственная причина и в остальном избыточен.Fix: гейтить по рядам-с-текстом:
const hasContent = thread.some(row => row.text.length > 0);(кейс «пустой корень, но у ответа есть текст» по-прежнему проходит —some()истинно из-за текста ответа, существующий тест на delta:87-110 не ломается). Опционально: вычислитьconst rows = thread.filter(r => r.text.length>0)один раз и рендеритьrows, убрав дублирующий предикат.F2 [stability] Согласовать оценку высоты для flip с реальным max-height —
apps/client/src/features/comment/components/comment-hover-preview.tsx:19,168-170,233placeAboveрезервируетESTIMATED_CARD_HEIGHT=200, но фактическийmaxHeight=CARD_MAX_HEIGHT=300. Длинный тред (200–300px), размещённый ВНИЗУ под спаном у нижней кромки, проходит проверку «влезает снизу» (зарезервировано только 200), но рендерится до 300px → низ на 0–100px уходит за вьюпорт и молча режетсяoverflow:hidden(аpointer-events:none— не проскроллить). Обрезка длинных тредов сама по себе принята дизайном, но РЕШЕНИЕ о flip опирается на устаревшую оценку. (Severity low, фикс — одна константа.)Fix:
ESTIMATED_CARD_HEIGHT = 300(=CARD_MAX_HEIGHT), чтобы flip срабатывал до того, как макс-высотная карточка переполнит вьюпорт.Объективные проверки: vitest сам прогнать не могу (нет node_modules в окружении ревью); тесты НЕЗАВИСИМО верифицированы не-вакуозными против реального компонента (тред-логика, порядок, автор, pointer-events, delay-совпадение), кодер прошлый раунд отчитывался о vitest 16/16. F1/F2 — точечная полировка новой ветки.
Маркер
reviewed_headобновлён на64a18298e. После правок верниreview/needs.F1: fixed — гейт карточки теперь по рядам-с-текстом:
const hasContent = thread.some(row => row.text.length > 0)(убрал дизъюнктthread.length > 1). Пустой корень + единственный пустой ответ больше не открывают пустой<Paper>(рендер и так фильтрует пустые ряды). Добавил тест-замок: родитель+ответ оба без текста → карточки нет. Кейс «пустой корень, но у ответа есть текст» по-прежнему проходит.F2: fixed —
ESTIMATED_CARD_HEIGHT200 → 300 (=CARD_MAX_HEIGHT), чтобы решение о flip-вверх учитывало реальный max, и высокий тред у нижней кромки переворачивался наверх, а не уезжал за вьюпорт.vitest— 19/19 (18 + новый),tsc— 0,eslint— 0. Возвращаюreview/needs.Ревью
ad9cc78f0— раунд 4 (правки F1/F2), спотлайт на двух фиксах.Вердикт: PASS. Оба пункта закрыты и сверены по коду. Готово к мержу.
hasContent = thread.some(row => row.text.length > 0)— дизъюнктthread.length > 1убран; пустой корень + единственный пустой ответ больше не открывают пустой<Paper>(рендер и так фильтрует пустые ряды). Кейс «пустой корень, но у ответа есть текст» проходит (some()истинно). Тест-замок добавлен: два empty-content комментария (корень+ответ) → после 350msqueryByTestId("comment-hover-preview")=== null; откат фикса (назад кlength>1) открыл бы пустую карточку и уронил бы этот тест.ESTIMATED_CARD_HEIGHT200→300 (=CARD_MAX_HEIGHT) с поясняющим комментарием — решение о flip-вверх теперь резервирует реальный worst-case, высокий тред у нижней кромки переворачивается наверх, а не уезжает за вьюпорт.Дельта — точечная (13 строк компонента: два фикса + комментарии; 27 строк теста: замок F1). Логика треда/автора/инвариант pointer-events не тронуты (верифицированы чистыми в раунде 3). Регрессий нет.
Объективные проверки: vitest сам прогнать не могу (нет node_modules в окружении); базис PASS — новый тест НЕЗАВИСИМО верифицирован не-вакуозным против реального компонента, кодер отчитался
vitest 19/19, tsc 0, eslint 0.Маркер
reviewed_headобновлён наad9cc78f0.