feat(editor): float image with text wrap (#145) #157

Ghost · 2026-06-24T06:36:05+03:00

Ghost commented

2026-06-24 06:36:05 +03:00

Закрывает #145.

Что

Обтекание картинки текстом: floatLeft/floatRight — текст обтекает изображение сбоку, в дополнение к блочным left/center/right. Порт из Forkmost PR #7 / апстрим Docmost PR #1132 (fuscodev), адаптирован под нашу императивную ноду-вью картинки (у апстрима — React styled-компонент, у нас стилизуется контейнер node-view через applyAlignment).

Изменения

editor-ext/image.ts: setImageAlign принимает floatLeft/floatRight; applyAlignment сначала сбрасывает float/padding, затем для float-режима ставит float:left|right + боковой padding на shrink-to-fit контейнер (внутренний <img> уже с max-width:100%). Резолвнутый align зеркалится в data-image-align на контейнере для адаптивного правила. data-align уже round-trip'ится через parse/renderHTML → float переживает сериализацию/коллаборацию/историю без смены схемы.
image-menu.tsx: кнопки Float-left / Float-right в bubble-меню (IconFloatLeft/Right) с active-состоянием.
image-resize.module.css: на узких экранах (≤600px) плавающая картинка схлопывается в 100% ширины и сбрасывает float (!important, по data-image-align) — это follow-up-коммит «100% width on small screen» из Forkmost.
i18n: en-US + ru-RU.

Проверка

pnpm --filter @docmost/editor-ext build + client tsc --noEmit — чисто. Логика и сериализация подтверждены типами/сборкой; визуальное обтекание лучше глянуть в браузере.

🤖 Generated with Claude Code

Закрывает #145. ## Что Обтекание картинки текстом: `floatLeft`/`floatRight` — текст обтекает изображение сбоку, в дополнение к блочным left/center/right. Порт из Forkmost PR #7 / апстрим Docmost PR #1132 (fuscodev), адаптирован под нашу **императивную** ноду-вью картинки (у апстрима — React styled-компонент, у нас стилизуется контейнер node-view через `applyAlignment`). ## Изменения - `editor-ext/image.ts`: `setImageAlign` принимает `floatLeft`/`floatRight`; `applyAlignment` сначала сбрасывает float/padding, затем для float-режима ставит `float:left|right` + боковой padding на shrink-to-fit контейнер (внутренний `<img>` уже с `max-width:100%`). Резолвнутый align зеркалится в `data-image-align` на контейнере для адаптивного правила. `data-align` уже round-trip'ится через parse/renderHTML → float переживает сериализацию/коллаборацию/историю без смены схемы. - `image-menu.tsx`: кнопки Float-left / Float-right в bubble-меню (`IconFloatLeft/Right`) с active-состоянием. - `image-resize.module.css`: на узких экранах (≤600px) плавающая картинка схлопывается в 100% ширины и сбрасывает float (`!important`, по `data-image-align`) — это follow-up-коммит «100% width on small screen» из Forkmost. - i18n: en-US + ru-RU. ## Проверка `pnpm --filter @docmost/editor-ext build` + client `tsc --noEmit` — чисто. Логика и сериализация подтверждены типами/сборкой; визуальное обтекание лучше глянуть в браузере. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-06-24 06:36:05 +03:00

feat(editor): float image with text wrap (#145 , port from Forkmost) 8ef66ba712

Adds floatLeft / floatRight image alignment so text wraps beside the image,
beyond the existing block left/center/right. Ported from Forkmost PR #7 /
upstream Docmost PR #1132 (fuscodev), adapted to gitmost's imperative image
node-view (the upstream uses a React styled component; ours styles the node-view
container directly via applyAlignment).

- editor-ext image.ts: `setImageAlign` accepts `floatLeft`/`floatRight`;
  `applyAlignment` resets float/padding then, for a float mode, sets
  `float:left|right` + side padding on the (shrink-to-fit) container so text
  flows beside it (the inner <img> already has max-width:100%). The resolved
  align is mirrored onto the container as `data-image-align` for the responsive
  rule. `data-align` already round-trips the value through parse/renderHTML, so
  float survives serialization / collab / history with no schema change.
- image-menu.tsx: Float-left / Float-right bubble-menu buttons (IconFloatLeft/
  Right) with active state.
- image-resize.module.css: on narrow screens (<=600px) a floated image collapses
  to full width and drops the float (`!important`, keyed on data-image-align) —
  the upstream "100% width on small screen" follow-up.
- i18n: en-US + ru-RU strings.

editor-ext build + client tsc --noEmit clean. Visual wrap behavior is best
confirmed in-browser (logic/serialization verified by build + types).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Ghost referenced this issue from a commit

2026-06-24 12:49:47 +03:00

fix(editor): load the float responsive rule + test applyAlignment (#145 review)

Ghost added 1 commit 2026-06-24 12:49:47 +03:00

fix(editor): load the float responsive rule + test applyAlignment (#145 review) 99359fa0fa

Review of #157 (Request changes) caught two blockers:

1. DEAD responsive CSS: the `@media (max-width:600px)` float-reset was added to
   `image-resize.module.css`, which is imported NOWHERE — the image container's
   classes come from `common/node-resize.module.css` (via buildResizeClasses).
   So on mobile a floated image kept its px width + float and crushed the text,
   exactly the failure the rule promised to prevent. Moved the rule to
   `common/node-resize.module.css` (the module actually imported by the resize
   node views); its `:global([data-image-align=...])` selectors are data-attr
   based, so they work unchanged. Reverted the dead addition from the (pre-existing,
   orphaned) image-resize.module.css.

2. `applyAlignment` was untested. Exported it and added `image.spec.ts` (vitest/
   jsdom) covering all five align values, the data-image-align mirror, and the
   floatLeft -> left reset-then-apply (the guard against a leaked float).
   Switched the float writes to the canonical CSSOM `cssFloat` property (portable:
   browsers + jsdom; behavior identical to the `.float` alias).

editor-ext build + client tsc clean; 6 image.spec tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

vvzvlad commented

2026-06-24 13:05:24 +03:00

Code review — float image with text wrap

Вердикт: Approve with comments. Прогнаны все 8 аспектов (Full‑tier, >100 строк). Критичных проблем и регрессий нет, ядро новой логики (applyAlignment) покрыто проходящими тестами (6/6, jsdom). Замечания ниже — только улучшения, мердж не блокируют.

Critical

Нет.

Warnings

Нет. Security, documentation/a11y/i18n, simplification — LGTM. Инъекций нет: align используется только в сравнениях ===, значения стилей — литералы, data-image-align пишется через DOM dataset (экранируется браузером). i18n‑политика (только en‑US + ru‑RU, fallbackLng:"en-US") задокументирована в i18n.ts. У обеих кнопок есть aria-label={t(...)}.

Suggestions

[conventions] Незаскоупленный :global([data-image-align=…]) — node-resize.module.css:84-90
Единственный другой :global в файле локально заскоуплен (.container:global(.ProseMirror-selectednode), стр. 47), а новое правило полностью глобально в *.module.css. Работает только потому, что data-image-align есть лишь на image‑контейнере.
Fix: .container:global([data-image-align="floatLeft"]), .container:global([data-image-align="floatRight"]) — это тот же элемент, на который applyAlignment пишет dataset.imageAlign.
[stability/regressions] Фолбэк ImageView не знает про float — image-view.tsx:13-18
alignClass мапит только left/right/center; floatLeft/floatRight молча уходят в alignCenter. Не живая регрессия: ImageView обслуживает только плейсхолдер/загрузку (нет src) или конфиг с выключенным resize; картинка с src рендерится через float‑aware ResizableNodeView. Fix (по желанию): добавить в memo if (align === "floatLeft") return "alignLeft"; if (align === "floatRight") return "alignRight";.
[stability] У плавающей картинки нет clearing — node-resize.module.css:76-90
Классика CSS‑float: картинка выше следующего короткого абзаца может перекрыть последующие блоки (картинку, код, таблицу). Присуще самой фиче, не дефект кода — стоит глазами проверить кейс «высокая картинка → короткий абзац → блок» и решить, нужен ли clear.

Test coverage

Ядро покрыто осмысленно: image.spec.ts гоняет все 5 веток applyAlignment (cssFloat/padding/dataset.imageAlign/justifyContent) и пинит reset‑guard на floatLeft→left. Тест проходит (6/6, jsdom). Один пробел:

[test-coverage] Ветка else/default тестируется только валидным "center" — image.spec.ts:49-56
applyAlignment типизирована как align: string и зовётся с node.attrs.align из parseHTML(getAttribute("data-align")) — устаревшее/неизвестное значение реально попадёт в else. Контракт «любой неизвестный align безопасно деградирует в center» не запинен. Fix: добавить кейс applyAlignment(el, "justify") с проверкой justifyContent==="center", cssFloat==="", padding==="", dataset.imageAlign==="justify".

Node‑view (вызовы на init/onUpdate) и кнопки меню тестами не покрыты — но это соответствует планке проекта (у существующих кнопок left/center/right тестов тоже нет), не отмечаю.

Architecture & design (forward-looking, non-blocking)

1. Дублирование applyAlignment по image/video/drawio/excalidraw. Ветка left/center/right побайтово идентична в четырёх расширениях; PR расширяет до 5 веток только image.ts, увеличивая расхождение. Дублирование было до PR.

A — оставить как есть, float остаётся image‑only (small). + scope PR не раздут, ноль риска для остальных, нет спекулятивной обобщённости. − расхождение сохраняется.
B — вынести общий applyMediaAlignment(container, align) (medium). + единый источник, новые режимы один раз. − правка четырёх расширений в PR про image‑float, второго потребителя float пока нет.
Рекомендация: A. Триггер для B — момент, когда float понадобится второму типу ноды.

2. Раздвоение источника истины: data-align (renderHTML) vs data-image-align + inline‑стили (node‑view). Подтверждено: ни в клиентском CSS, ни в серверном generateHTML нет правила data-align → выравнивание. Выравнивание (и существующее left/center/right, и float) применяется только JS‑нодвью. Следствие: в серверно‑сериализованном HTML (экспорт HTML/DOCX/PDF, статика публичного шаринга) float не рендерится — как и текущее выравнивание. В read‑only редакторе внутри приложения (те же расширения, resize включён) float работает. Это не регрессия — то же ограничение у left/center/right.

A — принять границу, задокументировать, что выравнивание медиа только в редакторе (small). + честно, ноль риска. − float тихо деградирует в экспорте.
B — единый prose‑стиль (редактор + read‑only + экспорт) с data-align как единственным источником истины, убрать data-image-align (large). + консистентность везде, уходит смелл. − масштаб больше фичи, риск регрессии выравнивания.
Рекомендация: A для этого PR + запись ограничения; B — отдельный follow‑up (кросс‑каттинг рефактор выравнивания).

Итог: чистая, протестированная фича; логика выравнивания корректна на всех прослеженных путях, сериализация floatLeft/floatRight через data-align безопасна (потребители трактуют значение как непрозрачную строку), старый клиент деградирует в центрированный блок без потери данных. К мерджу после (опционального) учёта замечаний — в первую очередь тест‑кейс на неизвестный align и скоуп CSS‑правила.

_{🤖 Multi-aspect review (security · stability · conventions · documentation · regressions · test-coverage · simplification · architecture)}

## Code review — float image with text wrap **Вердикт: Approve with comments.** Прогнаны все 8 аспектов (Full‑tier, >100 строк). Критичных проблем и регрессий нет, ядро новой логики (`applyAlignment`) покрыто проходящими тестами (6/6, jsdom). Замечания ниже — только улучшения, мердж не блокируют. ### Critical Нет. ### Warnings Нет. Security, documentation/a11y/i18n, simplification — LGTM. Инъекций нет: `align` используется только в сравнениях `===`, значения стилей — литералы, `data-image-align` пишется через DOM `dataset` (экранируется браузером). i18n‑политика (только en‑US + ru‑RU, `fallbackLng:"en-US"`) задокументирована в `i18n.ts`. У обеих кнопок есть `aria-label={t(...)}`. ### Suggestions - **[conventions] Незаскоупленный `:global([data-image-align=…])`** — `node-resize.module.css:84-90` Единственный другой `:global` в файле локально заскоуплен (`.container:global(.ProseMirror-selectednode)`, стр. 47), а новое правило полностью глобально в `*.module.css`. Работает только потому, что `data-image-align` есть лишь на image‑контейнере. Fix: `.container:global([data-image-align="floatLeft"]), .container:global([data-image-align="floatRight"])` — это тот же элемент, на который `applyAlignment` пишет `dataset.imageAlign`. - **[stability/regressions] Фолбэк `ImageView` не знает про float** — `image-view.tsx:13-18` `alignClass` мапит только left/right/center; `floatLeft`/`floatRight` молча уходят в `alignCenter`. **Не живая регрессия**: `ImageView` обслуживает только плейсхолдер/загрузку (нет `src`) или конфиг с выключенным resize; картинка с `src` рендерится через float‑aware `ResizableNodeView`. Fix (по желанию): добавить в memo `if (align === "floatLeft") return "alignLeft"; if (align === "floatRight") return "alignRight";`. - **[stability] У плавающей картинки нет clearing** — `node-resize.module.css:76-90` Классика CSS‑float: картинка выше следующего короткого абзаца может перекрыть последующие блоки (картинку, код, таблицу). Присуще самой фиче, не дефект кода — стоит глазами проверить кейс «высокая картинка → короткий абзац → блок» и решить, нужен ли `clear`. ### Test coverage Ядро покрыто осмысленно: `image.spec.ts` гоняет все 5 веток `applyAlignment` (cssFloat/padding/dataset.imageAlign/justifyContent) и пинит reset‑guard на floatLeft→left. Тест проходит (6/6, jsdom). Один пробел: - **[test-coverage] Ветка `else`/default тестируется только валидным `"center"`** — `image.spec.ts:49-56` `applyAlignment` типизирована как `align: string` и зовётся с `node.attrs.align` из `parseHTML(getAttribute("data-align"))` — устаревшее/неизвестное значение реально попадёт в `else`. Контракт «любой неизвестный align безопасно деградирует в center» не запинен. Fix: добавить кейс `applyAlignment(el, "justify")` с проверкой `justifyContent==="center"`, `cssFloat===""`, `padding===""`, `dataset.imageAlign==="justify"`. Node‑view (вызовы на init/onUpdate) и кнопки меню тестами не покрыты — но это соответствует планке проекта (у существующих кнопок left/center/right тестов тоже нет), не отмечаю. ### Architecture & design (forward-looking, non-blocking) **1. Дублирование `applyAlignment` по image/video/drawio/excalidraw.** Ветка left/center/right побайтово идентична в четырёх расширениях; PR расширяет до 5 веток только `image.ts`, увеличивая расхождение. Дублирование было до PR. - *A — оставить как есть, float остаётся image‑only* (small). + scope PR не раздут, ноль риска для остальных, нет спекулятивной обобщённости. − расхождение сохраняется. - *B — вынести общий `applyMediaAlignment(container, align)`* (medium). + единый источник, новые режимы один раз. − правка четырёх расширений в PR про image‑float, второго потребителя float пока нет. - **Рекомендация: A.** Триггер для B — момент, когда float понадобится второму типу ноды. **2. Раздвоение источника истины: `data-align` (renderHTML) vs `data-image-align` + inline‑стили (node‑view).** Подтверждено: ни в клиентском CSS, ни в серверном `generateHTML` нет правила `data-align → выравнивание`. Выравнивание (и существующее left/center/right, и float) применяется **только** JS‑нодвью. Следствие: в серверно‑сериализованном HTML (экспорт HTML/DOCX/PDF, статика публичного шаринга) float **не рендерится** — как и текущее выравнивание. В read‑only редакторе внутри приложения (те же расширения, resize включён) float работает. Это **не регрессия** — то же ограничение у left/center/right. - *A — принять границу, задокументировать, что выравнивание медиа только в редакторе* (small). + честно, ноль риска. − float тихо деградирует в экспорте. - *B — единый prose‑стиль (редактор + read‑only + экспорт) с `data-align` как единственным источником истины, убрать `data-image-align`* (large). + консистентность везде, уходит смелл. − масштаб больше фичи, риск регрессии выравнивания. - **Рекомендация: A** для этого PR + запись ограничения; B — отдельный follow‑up (кросс‑каттинг рефактор выравнивания). --- Итог: чистая, протестированная фича; логика выравнивания корректна на всех прослеженных путях, сериализация `floatLeft`/`floatRight` через `data-align` безопасна (потребители трактуют значение как непрозрачную строку), старый клиент деградирует в центрированный блок без потери данных. К мерджу после (опционального) учёта замечаний — в первую очередь тест‑кейс на неизвестный `align` и скоуп CSS‑правила. <sub>🤖 Multi-aspect review (security · stability · conventions · documentation · regressions · test-coverage · simplification · architecture)</sub>

vvzvlad commented

2026-06-24 13:06:46 +03:00

после исправления можно мержить

Ghost added 1 commit 2026-06-24 13:08:56 +03:00

style(editor): scope the float responsive :global to .container (#145 review) 43cf1913e0

Per review: the file's other :global is locally scoped (.container:global(...)),
but the new float-reset media rule was fully global in a *.module.css. Scope it to
.container — the image node-view container carries BOTH the .container class and
the data-image-align attribute (same element), so behavior is unchanged while the
selector no longer leaks globally.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

vvzvlad merged commit e97024343a into develop

2026-06-24 14:04:04 +03:00

vvzvlad referenced this issue from a commit

2026-06-24 14:04:05 +03:00

Merge pull request 'feat(editor): float image with text wrap (#145)' (#157) from feat/float-image into develop

vvzvlad deleted branch feat/float-image

2026-06-24 14:04:11 +03:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#157