fix(block-editor): apply links to a selected image instead of replacing it (#36368)

rjvelazco · claude · web-flow · commit 58cd429ce4cc · 2026-06-30T19:25:39.000Z
## Proposed Changes Fixes #36361 — in the new block editor, selecting an image and clicking the toolbar **Link** button deleted the image and replaced it with a text link. The `dotImage` node already supports `href`/`target` end-to-end (it parses a wrapping `<a>` and re-wraps the `<img>` on render); the break was purely in the UI flow, which always routed through the text-link *insert* path. ### What changed - **Toolbar Link button is image-aware** — when a `dotImage` node is selected, the link popover opens in *image-link mode* (URL + "Open in new tab" only; Text/Advanced fields hidden) and applies the link via `updateAttributes('dotImage', { href, target })`, so the image is preserved instead of replaced. - **Remove link** — a trash-can action appears when editing an image that already has a link, clearing `href`/`target`. - **Click behavior** — clicking a linked image no longer opens the text-link editor; it selects the image node (the link is edited from the toolbar) and prevents the anchor from navigating. - **Layout fixes** — eliminated the vertical jump/extra space when a link wraps the image: zero the inline-block baseline descender gap, and zero the Tailwind Typography `img` margin that reappears once the image is no longer a direct `<figure>` child. - **Active state** — the toolbar Link button now highlights for linked images (`isImageLinked`), mirroring `isLink` for text links. The text-link flow is unchanged. No `dotImage` node name or stored-content shape changed — links round-trip through the existing `href`/`target` attributes, so no data migration is required. ### Files - `link-popover.component.{ts,html}` — image-link mode, Remove action - `toolbar.component.{ts,html}` + `editor-toolbar.store.ts` — image detection, active state - `editor-chrome-click.ts` — don't open the dialog on linked-image clicks - `editor.component.css` — layout/margin fixes - `editor-popover.service.ts` — `isImageLink` payload flag - `Language.properties` — `dot.block.editor.dialog.link.remove` ## Testing - `pnpm nx lint new-block-editor` ✓ - `tsc --noEmit` on the lib ✓ - Manual: insert image → select → Link button opens URL-only popover → Save keeps the image wrapped in `<a>` (not replaced); re-select shows active Link button + trash to remove; no layout jump toggling the link; text-link flow unchanged. ## Video https://github.com/user-attachments/assets/43a03e69-d132-496a-a360-2f238c3129e8 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/core-web/libs/new-block-editor/src/lib/editor/components/link-popover/link-popover.component.html b/core-web/libs/new-block-editor/src/lib/editor/components/link-popover/link-popover.component.html
@@ -75,22 +75,24 @@
                 </p-autoComplete>
             </div>
 
-            <div class="flex flex-col gap-1">
-                <label for="link-text" class="text-xs font-medium text-gray-700">
-                    {{ 'dot.block.editor.dialog.link.field.text.label' | dm }}
-                    <span class="font-normal text-gray-400">
-                        {{ 'dot.block.editor.dialog.link.field.text.optional' | dm }}
-                    </span>
-                </label>
-                <input
-                    pInputText
-                    id="link-text"
-                    type="text"
-                    [formControl]="form.controls.displayText"
-                    [placeholder]="'dot.block.editor.dialog.link.field.text.placeholder' | dm"
-                    pSize="small"
-                    class="w-full rounded-sm border border-gray-300 px-3 py-1.5 text-sm focus:border-indigo-500 focus:outline-none" />
-            </div>
+            @if (!isImageLink()) {
+                <div class="flex flex-col gap-1">
+                    <label for="link-text" class="text-xs font-medium text-gray-700">
+                        {{ 'dot.block.editor.dialog.link.field.text.label' | dm }}
+                        <span class="font-normal text-gray-400">
+                            {{ 'dot.block.editor.dialog.link.field.text.optional' | dm }}
+                        </span>
+                    </label>
+                    <input
+                        pInputText
+                        id="link-text"
+                        type="text"
+                        [formControl]="form.controls.displayText"
+                        [placeholder]="'dot.block.editor.dialog.link.field.text.placeholder' | dm"
+                        pSize="small"
+                        class="w-full rounded-sm border border-gray-300 px-3 py-1.5 text-sm focus:border-indigo-500 focus:outline-none" />
+                </div>
+            }
 
             <label class="flex cursor-pointer items-center gap-2" for="open-in-new-tab">
                 <input
@@ -103,20 +105,22 @@
                 </span>
             </label>
 
-            <button
-                type="button"
-                [attr.aria-expanded]="advancedOpen()"
-                aria-controls="link-advanced-section"
-                data-testid="link-advanced-toggle"
-                (mousedown)="$event.preventDefault(); toggleAdvanced()"
-                class="flex items-center gap-1 self-start rounded-sm text-xs font-medium text-gray-600 hover:text-indigo-700 focus:ring-2 focus:ring-indigo-300 focus:outline-none">
-                <span>{{ 'dot.block.editor.dialog.link.advanced' | dm }}</span>
-                <span aria-hidden="true" class="material-symbols-outlined text-base">
-                    {{ advancedOpen() ? 'expand_less' : 'expand_more' }}
-                </span>
-            </button>
+            @if (!isImageLink()) {
+                <button
+                    type="button"
+                    [attr.aria-expanded]="advancedOpen()"
+                    aria-controls="link-advanced-section"
+                    data-testid="link-advanced-toggle"
+                    (mousedown)="$event.preventDefault(); toggleAdvanced()"
+                    class="flex items-center gap-1 self-start rounded-sm text-xs font-medium text-gray-600 hover:text-indigo-700 focus:ring-2 focus:ring-indigo-300 focus:outline-none">
+                    <span>{{ 'dot.block.editor.dialog.link.advanced' | dm }}</span>
+                    <span aria-hidden="true" class="material-symbols-outlined text-base">
+                        {{ advancedOpen() ? 'expand_less' : 'expand_more' }}
+                    </span>
+                </button>
+            }
 
-            @if (advancedOpen()) {
+            @if (!isImageLink() && advancedOpen()) {
                 <div id="link-advanced-section" class="flex flex-col gap-3">
                     <div class="flex flex-col gap-1">
                         <label for="link-title" class="text-xs font-medium text-gray-700">
@@ -174,6 +178,20 @@
             }
 
             <div class="flex justify-end gap-2 pt-1">
+                @if (canRemoveLink()) {
+                    <button
+                        type="button"
+                        data-testid="link-remove"
+                        [attr.aria-label]="'dot.block.editor.dialog.link.remove' | dm"
+                        [attr.title]="'dot.block.editor.dialog.link.remove' | dm"
+                        (mousedown)="$event.preventDefault()"
+                        (click)="onRemoveLink()"
+                        class="mr-auto flex cursor-pointer items-center rounded-sm p-1.5 text-red-600 hover:bg-red-50 focus:ring-2 focus:ring-red-300 focus:outline-none">
+                        <span aria-hidden="true" class="material-symbols-outlined text-xl">
+                            delete
+                        </span>
+                    </button>
+                }
                 <button
                     type="button"
                     (mousedown)="$event.preventDefault(); manager.close()"
diff --git a/core-web/libs/new-block-editor/src/lib/editor/components/link-popover/link-popover.component.ts b/core-web/libs/new-block-editor/src/lib/editor/components/link-popover/link-popover.component.ts
@@ -33,6 +33,7 @@ import { DotCMSContentlet } from '@dotcms/dotcms-models';
 import { DotMessagePipe } from '@dotcms/ui';
 
 import { FULLSCREEN_AWARE_OVERLAY_OPTIONS } from '../../config.utils';
+import { DOT_IMAGE_NODE_NAME } from '../../extensions/nodes/image.extension';
 import { LINK_SELECTION_KEY } from '../../extensions/selection-preserve.extension';
 import { EditorPopoverService } from '../../services/editor-popover.service';
 import { EditorStore } from '../../store/editor.store';
@@ -139,6 +140,26 @@ export class LinkPopoverComponent {
         () => this.manager.linkPayload()?.initialValues != null
     );
 
+    /**
+     * True when the popover targets a selected `dotImage` node. Hides the text-only fields (Text,
+     * Advanced) and routes {@link onInsert} to update the image node's `href`/`target` attributes
+     * instead of inserting a text node with a link mark.
+     */
+    protected readonly isImageLink = computed(
+        () => this.manager.linkPayload()?.isImageLink === true
+    );
+
+    /**
+     * True when there's an existing link to remove — drives the trash (Remove link) button.
+     * For images: the node already has an `href`. For text: the popover was opened on a real
+     * anchor (`linkEl` set), not while creating a new link over plain selected text.
+     */
+    protected readonly canRemoveLink = computed(() => {
+        const payload = this.manager.linkPayload();
+        if (!payload) return false;
+        return payload.isImageLink ? !!payload.initialValues?.href : !!payload.linkEl;
+    });
+
     /**
      * Tracks whether the Advanced (Title / Aria Label / Rel) section is visible.
      * Auto-expands when an existing link's payload carries any of those values, so the
@@ -274,6 +295,8 @@ export class LinkPopoverComponent {
         effect((onCleanup) => {
             if (!this.manager.isOpen('link')) return;
             if (this.manager.linkPayload()?.linkEl) return;
+            // Image-link mode is a NodeSelection — there is no text range to paint.
+            if (this.manager.linkPayload()?.isImageLink) return;
             const view = this.editor().view;
             view.dispatch(view.state.tr.setMeta(LINK_SELECTION_KEY, { active: true }));
             onCleanup(() =>
@@ -364,6 +387,21 @@ export class LinkPopoverComponent {
             rel: (rel ?? '').trim() || null
         };
 
+        if (payload?.isImageLink) {
+            // Image-link mode — set the link on the selected image node's attributes. The node's
+            // renderHTML/nodeView wrap the <img> in <a href target>, so the image is preserved.
+            editor
+                .chain()
+                .focus()
+                .updateAttributes(DOT_IMAGE_NODE_NAME, {
+                    href,
+                    target: openInNewTab ? '_blank' : null
+                })
+                .run();
+            this.manager.close();
+            return;
+        }
+
         if (payload?.linkEl) {
             // Edit mode — update the link in place using the pre-computed anchor position.
             const linkEl = payload.linkEl;
@@ -401,4 +439,42 @@ export class LinkPopoverComponent {
 
         this.manager.close();
     }
+
+    /**
+     * Removes the existing link. For an image, clears the node's `href`/`target`; for text,
+     * unsets the `link` mark over its full range (anchored at the captured edit position).
+     */
+    protected onRemoveLink(): void {
+        const payload = this.manager.linkPayload();
+        const editor = this.editor();
+
+        if (payload?.isImageLink) {
+            editor
+                .chain()
+                .focus()
+                .updateAttributes(DOT_IMAGE_NODE_NAME, { href: null, target: null })
+                .run();
+            this.manager.close();
+            return;
+        }
+
+        const linkEl = payload?.linkEl;
+        const anchorPos =
+            payload?.anchorPos ??
+            (() => {
+                try {
+                    return linkEl ? editor.view.posAtDOM(linkEl, 0) : editor.state.selection.from;
+                } catch {
+                    return editor.state.selection.from;
+                }
+            })();
+        editor
+            .chain()
+            .focus()
+            .setTextSelection(anchorPos)
+            .extendMarkRange('link')
+            .unsetMark('link')
+            .run();
+        this.manager.close();
+    }
 }
diff --git a/core-web/libs/new-block-editor/src/lib/editor/components/toolbar/editor-toolbar.store.ts b/core-web/libs/new-block-editor/src/lib/editor/components/toolbar/editor-toolbar.store.ts
@@ -24,6 +24,9 @@ export class EditorToolbarStore {
     readonly canIndent = signal(false);
     readonly canOutdent = signal(false);
     readonly isImageSelected = signal(false);
+    /** True when the selected `dotImage` node carries a link (`href`) — drives the toolbar
+     *  Link button's active state for linked images, mirroring `isLink` for text links. */
+    readonly isImageLinked = signal(false);
     readonly imageTextWrap = signal<string | null>(null);
     readonly imageTextAlign = signal<string | null>(null);
     readonly textAlign = signal<'left' | 'center' | 'right' | 'justify'>('left');
@@ -47,6 +50,9 @@ export class EditorToolbarStore {
                 this.isCodeBlock.set(editor.isActive('codeBlock'));
                 this.isLink.set(editor.isActive('link'));
                 this.isImageSelected.set(editor.isActive('dotImage'));
+                this.isImageLinked.set(
+                    editor.isActive('dotImage') && !!editor.getAttributes('dotImage').href
+                );
                 this.imageTextWrap.set(
                     editor.isActive('dotImage')
                         ? (editor.getAttributes('dotImage').textWrap ?? null)
diff --git a/core-web/libs/new-block-editor/src/lib/editor/components/toolbar/toolbar.component.html b/core-web/libs/new-block-editor/src/lib/editor/components/toolbar/toolbar.component.html
@@ -369,7 +369,7 @@
             [tooltipPosition]="TOOLTIP_POSITION"
             [tooltipOptions]="overlayTooltipOptions()"
             [showDelay]="TOOLTIP_SHOW_DELAY"
-            [class]="btnClass(state.isLink() || popovers.isOpen('link'))"
+            [class]="btnClass(state.isLink() || state.isImageLinked() || popovers.isOpen('link'))"
             (mousedown)="openLinkDialog($event)">
             <span aria-hidden="true" class="material-symbols-outlined">link</span>
         </button>
diff --git a/core-web/libs/new-block-editor/src/lib/editor/components/toolbar/toolbar.component.ts b/core-web/libs/new-block-editor/src/lib/editor/components/toolbar/toolbar.component.ts
@@ -26,6 +26,7 @@ import {
     FULLSCREEN_AWARE_OVERLAY_OPTIONS,
     OVERLAY_ABOVE_FULLSCREEN_Z_INDEX
 } from '../../config.utils';
+import { DOT_IMAGE_NODE_NAME } from '../../extensions/nodes/image.extension';
 import { BLOCK_TARGET_KEY } from '../../extensions/selection-preserve.extension';
 import { ContentletEditUrlService } from '../../services/contentlet-edit-url.service';
 import { EditorModalService } from '../../services/editor-modal.service';
@@ -412,6 +413,19 @@ export class ToolbarComponent implements OnDestroy {
         const { from, to, empty } = editor.state.selection;
         const btn = event.currentTarget as HTMLElement;
 
+        // Image selected → apply the link to the image node's href/target instead of inserting
+        // text. Without this branch the insert-text path below would delete the image (#36361).
+        if (editor.isActive(DOT_IMAGE_NODE_NAME)) {
+            const attrs = editor.getAttributes(DOT_IMAGE_NODE_NAME);
+            this.popovers.openLink(() => btn.getBoundingClientRect(), {
+                isImageLink: true,
+                initialValues: attrs['href']
+                    ? { href: attrs['href'], target: attrs['target'] ?? null }
+                    : undefined
+            });
+            return;
+        }
+
         // Check if cursor/selection is inside an existing link
         const linkMark = editor.state.doc
             .resolve(from)
diff --git a/core-web/libs/new-block-editor/src/lib/editor/editor-chrome-click.ts b/core-web/libs/new-block-editor/src/lib/editor/editor-chrome-click.ts
@@ -14,6 +14,18 @@ export function handleEditorProseMirrorClick(
     const anchor = (event.target as HTMLElement).closest('a[href]');
     if (!anchor) return;
 
+    // A linked image renders as <a href><img></a>. Clicking the image must NOT open the link
+    // editor — the click selects the `dotImage` node (surfacing the image toolbar group), and the
+    // link is edited from the toolbar Link button. Just stop the anchor from navigating. Opening
+    // the text-link editor here would show the wrong fields and, on save, replace the image
+    // (#36361). Gate on the *clicked* element being the image (not merely any descendant img) so
+    // a text click in a mixed anchor still routes to the text-link editor.
+    const clickedImage = (event.target as HTMLElement).closest('img');
+    if (clickedImage && anchor.contains(clickedImage)) {
+        event.preventDefault();
+        return;
+    }
+
     const href = anchor.getAttribute('href') ?? '';
     const displayText = anchor.textContent?.trim() ?? '';
     const target = anchor.getAttribute('target');
diff --git a/core-web/libs/new-block-editor/src/lib/editor/editor.component.css b/core-web/libs/new-block-editor/src/lib/editor/editor.component.css
@@ -225,6 +225,26 @@
     display: inline-block;
     max-width: 100%;
     height: auto;
+    /* Align to the top of the line box so the inline-block image carries no baseline
+       descender gap. Without this, wrapping the image in an inline <a> (when a link is
+       added) introduces extra space below it and the layout visibly jumps (#36361). */
+    vertical-align: top;
+    /* Tailwind Typography's `.prose :where(img)` adds `margin: 2em 0`, normally cancelled by
+       its `figure > *` reset. Once the image is wrapped in <a> (linked), it's no longer a direct
+       figure child so that reset stops applying and the 2em margin returns. Zero it explicitly —
+       this selector out-specifies prose's zero-specificity `:where()` rules (#36361). */
+    margin: 0;
+}
+
+/* A linked image is rendered as <figure><a href><img></a></figure>. Keep the anchor a
+   zero-line-height inline-block so it hugs the image and linking doesn't shift layout.
+   `vertical-align: top` is essential: once the anchor wraps the image, the *anchor* (not the
+   img) is the inline-level box in the figure's formatting context, so it must align to the top
+   or the browser reserves font-descender space below it and the layout jumps (#36361). */
+:host ::ng-deep .ProseMirror figure a {
+    display: inline-block;
+    line-height: 0;
+    vertical-align: top;
 }
 
 /* ─── Selected node ring ────────────────────────────────── */
diff --git a/core-web/libs/new-block-editor/src/lib/editor/editor.utils.ts b/core-web/libs/new-block-editor/src/lib/editor/editor.utils.ts
@@ -10,7 +10,11 @@ import {
     replacePlaceholder,
     removePlaceholder
 } from './extensions/nodes/upload-placeholder.extension';
-import { type DotVideoData, DOT_VIDEO_NODE_NAME } from './extensions/nodes/video.extension';
+import {
+    type DotVideoData,
+    DOT_VIDEO_NODE_NAME,
+    videoMetaAttrsFromContentlet
+} from './extensions/nodes/video.extension';
 import { type UploadedImage, type UploadedVideo } from './services/dot-upload.service';
 
 export function handleMediaDrop(
@@ -81,11 +85,11 @@ export function handleMediaDrop(
 
         if (uploadVideo) {
             uploadVideo(file)
-                .then(({ src, data }) => {
+                .then(({ src, data, mimeType, width, height, orientation }) => {
                     const title = file.name.replace(/\.[^.]+$/, '');
                     replacePlaceholder(editor, id, {
                         type: DOT_VIDEO_NODE_NAME,
-                        attrs: { src, title, data }
+                        attrs: { src, title, data, mimeType, width, height, orientation }
                     });
                 })
                 .catch((err) => {
@@ -154,7 +158,8 @@ export function insertDotVideoFromContentlet(editor: Editor, contentlet: DotCMSC
             attrs: {
                 src: data.asset,
                 title: data.title || null,
-                data
+                data,
+                ...videoMetaAttrsFromContentlet(contentlet)
             }
         })
         .run();
diff --git a/core-web/libs/new-block-editor/src/lib/editor/extensions/nodes/image.extension.ts b/core-web/libs/new-block-editor/src/lib/editor/extensions/nodes/image.extension.ts
@@ -140,15 +140,17 @@ export const DotImage = Image.extend({
         else if (textAlign) figAttrs['class'] = `image-align-${textAlign}`;
 
         const img = ['img', mergeAttributes(this.options.HTMLAttributes, imgAttrs)];
-        const anchorAttrs: Record<string, string> = { href };
+        // Build conditionally so a null/absent href never serializes as href="null".
+        const anchorAttrs: Record<string, string> = {};
+        if (href) anchorAttrs['href'] = href;
         if (target) anchorAttrs['target'] = target;
         const inner = href ? ['a', anchorAttrs, img] : img;
 
         return ['figure', figAttrs, inner];
     },
 
     addNodeView() {
-        return ({ node }) => {
+        return ({ node, getPos, editor }) => {
             const figure = document.createElement('figure');
             const img = document.createElement('img');
 
@@ -175,6 +177,17 @@ export const DotImage = Image.extend({
                 if (target) a.setAttribute('target', String(target));
                 a.appendChild(img);
                 figure.appendChild(a);
+
+                // The wrapping <a> makes the browser treat clicks as link interactions, so
+                // ProseMirror won't cleanly node-select the image (it took several clicks to
+                // activate the image + its toolbar options). Select the node ourselves on the
+                // first mousedown and suppress the anchor's default behaviour (#36361).
+                figure.addEventListener('mousedown', (event) => {
+                    event.preventDefault();
+                    if (typeof getPos === 'function') {
+                        editor.commands.setNodeSelection(getPos());
+                    }
+                });
             } else {
                 figure.appendChild(img);
             }
diff --git a/core-web/libs/new-block-editor/src/lib/editor/extensions/nodes/video.extension.ts b/core-web/libs/new-block-editor/src/lib/editor/extensions/nodes/video.extension.ts
diff --git a/core-web/libs/new-block-editor/src/lib/editor/services/dot-upload.service.ts b/core-web/libs/new-block-editor/src/lib/editor/services/dot-upload.service.ts
diff --git a/core-web/libs/new-block-editor/src/lib/editor/services/editor-popover.service.ts b/core-web/libs/new-block-editor/src/lib/editor/services/editor-popover.service.ts
diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties
