fix: separator div instead of gap

app/components/Actions.ts

@@ -15,10 +15,10 @@ export const Action = styled(Flex)`
   }
 `;
 
-export const Separator = styled.div`
+export const Separator = styled.div<{ height?: number }>`
   flex-shrink: 0;
   width: 1px;
-  height: 28px;
+  height: ${(props) => props.height || 28}px;
   background: ${s("divider")};
 `;
 

app/editor/components/SelectionToolbar.tsx

@@ -1,7 +1,10 @@
 import { Selection, NodeSelection, TextSelection } from "prosemirror-state";
 import * as React from "react";
 import filterExcessSeparators from "@shared/editor/lib/filterExcessSeparators";
-import { getMarkRange } from "@shared/editor/queries/getMarkRange";
+import {
+  getMarkRange,
+  getMarkRangeNodeSelection,
+} from "@shared/editor/queries/getMarkRange";
 import { isInCode } from "@shared/editor/queries/isInCode";
 import { isInNotice } from "@shared/editor/queries/isInNotice";
 import { isNodeActive } from "@shared/editor/queries/isNodeActive";
@@ -117,7 +120,6 @@ export function SelectionToolbar(props: Props) {
   const isDividerSelection = isNodeActive(state.schema.nodes.hr)(state);
   const colIndex = getColumnIndex(state);
   const rowIndex = getRowIndex(state);
-  const link = getMarkRange(selection.$from, state.schema.marks.link);
   const isImageSelection =
     selection instanceof NodeSelection && selection.node.type.name === "image";
   const isAttachmentSelection =
@@ -127,6 +129,10 @@ export function SelectionToolbar(props: Props) {
     selection instanceof NodeSelection && selection.node.type.name === "embed";
   const isCodeSelection = isInCode(state, { onlyBlock: true });
   const isNoticeSelection = isInNotice(state);
+  const link =
+    selection instanceof NodeSelection
+      ? getMarkRangeNodeSelection(selection, state.schema.marks.link)
+      : getMarkRange(selection.$from, state.schema.marks.link);
 
   let items: MenuItem[] = [];
   let align: "center" | "start" | "end" = "center";

app/editor/menus/image.tsx

@@ -7,6 +7,7 @@ import {
   AlignImageCenterIcon,
   AlignFullWidthIcon,
   CommentIcon,
+  LinkIcon,
 } from "outline-icons";
 import { EditorState } from "prosemirror-state";
 import { isNodeActive } from "@shared/editor/queries/isNodeActive";
@@ -102,6 +103,12 @@ export default function imageMenuItems(
     {
       name: "separator",
     },
+    {
+      name: "linkOnImage",
+      tooltip: dictionary.createLink,
+      shortcut: `${metaDisplay}+K`,
+      icon: <LinkIcon />,
+    },
     {
       name: "commentOnImage",
       tooltip: dictionary.comment,

shared/editor/commands/link.ts

@@ -0,0 +1,233 @@
+import { chainCommands, toggleMark } from "prosemirror-commands";
+import { Attrs } from "prosemirror-model";
+import {
+  Command,
+  NodeSelection,
+  Selection,
+  TextSelection,
+} from "prosemirror-state";
+import { getMarkRange } from "../queries/getMarkRange";
+import { toast } from "sonner";
+import { sanitizeUrl } from "@shared/utils/urls";
+import { addMark } from "./addMark";
+import { getMarkRangeNodeSelection } from "../queries/getMarkRange";
+
+const addLinkTextSelection =
+  (attrs: Attrs): Command =>
+  (state, dispatch) => {
+    if (!(state.selection instanceof TextSelection)) {
+      return false;
+    }
+
+    return addMark(state.schema.marks.link, attrs)(state, dispatch);
+  };
+
+const addLinkNodeSelection =
+  (attrs: Attrs): Command =>
+  (state, dispatch) => {
+    if (!(state.selection instanceof NodeSelection)) {
+      return false;
+    }
+    const { selection } = state;
+    const existingMarks = selection.node.attrs.marks ?? [];
+    const newMark = {
+      type: "link",
+      attrs,
+    };
+    const updatedMarks = [...existingMarks, newMark];
+    dispatch?.(
+      state.tr.setNodeAttribute(selection.from, "marks", updatedMarks)
+    );
+    return true;
+  };
+
+const openLinkTextSelection =
+  (attrs: Attrs): Command =>
+  (state) => {
+    if (!(state.selection instanceof TextSelection)) {
+      return false;
+    }
+
+    const range = getMarkRange(state.selection.$from, state.schema.marks.link);
+    if (range && range.mark && attrs.onClickLink) {
+      try {
+        const event = new KeyboardEvent("keydown", { metaKey: false });
+        attrs.onClickLink(sanitizeUrl(range.mark.attrs.href), event);
+      } catch (_err) {
+        toast.error(attrs.dictionary.openLinkError);
+      }
+      return true;
+    }
+    return false;
+  };
+
+const openLinkNodeSelection =
+  (attrs: Attrs): Command =>
+  (state) => {
+    if (!(state.selection instanceof NodeSelection)) {
+      return false;
+    }
+
+    if (!attrs.onClickLink) {
+      return false;
+    }
+
+    const marks = state.selection.node.attrs.marks ?? [];
+    const linkMark = marks.find((mark: any) => mark.type === "link");
+    if (!linkMark) {
+      return false;
+    }
+
+    try {
+      const event = new KeyboardEvent("keydown", { metaKey: false });
+      attrs.onClickLink(sanitizeUrl(linkMark.attrs.href), event);
+    } catch (_err) {
+      toast.error(attrs.dictionary.openLinkError);
+    }
+    return true;
+  };
+
+const updateLinkTextSelection =
+  (attrs: Attrs): Command =>
+  (state, dispatch) => {
+    if (!(state.selection instanceof TextSelection)) {
+      return false;
+    }
+
+    const range = getMarkRange(state.selection.$from, state.schema.marks.link);
+
+    if (range && range.mark) {
+      const nextSelection =
+        Selection.findFrom(state.doc.resolve(range.to), 1, true) ??
+        TextSelection.create(state.tr.doc, 0);
+      dispatch?.(
+        state.tr
+          .setSelection(nextSelection)
+          .removeMark(range.from, range.to, state.schema.marks.link)
+          .addMark(range.from, range.to, state.schema.marks.link.create(attrs))
+      );
+      return true;
+    }
+    return false;
+  };
+
+const updateLinkNodeSelection =
+  (attrs: Attrs): Command =>
+  (state, dispatch) => {
+    if (!(state.selection instanceof NodeSelection)) {
+      return false;
+    }
+
+    const markRange = getMarkRangeNodeSelection(
+      state.selection,
+      state.schema.marks.link
+    );
+    if (!markRange) {
+      return false;
+    }
+
+    const existingMarks = state.selection.node.attrs.marks ?? [];
+    const updatedMarks = existingMarks.map((mark: any) =>
+      mark.type === "link"
+        ? { ...mark, attrs: { ...mark.attrs, ...attrs } }
+        : mark
+    );
+    const nextValidSelection =
+      Selection.findFrom(state.doc.resolve(markRange.to), 1, true) ??
+      TextSelection.create(state.tr.doc, 0);
+    dispatch?.(
+      state.tr
+        .setSelection(nextValidSelection)
+        .setNodeAttribute(state.selection.from, "marks", updatedMarks)
+    );
+    return true;
+  };
+
+const removeLinkTextSelection = (): Command => (state, dispatch) => {
+  if (!(state.selection instanceof TextSelection)) {
+    return false;
+  }
+  const range = getMarkRange(state.selection.$from, state.schema.marks.link);
+  if (range && range.mark) {
+    const nextSelection =
+      Selection.findFrom(state.doc.resolve(range.to), 1, true) ??
+      TextSelection.create(state.tr.doc, 0);
+    dispatch?.(
+      state.tr
+        .setSelection(nextSelection)
+        .removeMark(range.from, range.to, range.mark)
+    );
+    return true;
+  }
+  return false;
+};
+
+const removeLinkNodeSelection = (): Command => (state, dispatch) => {
+  if (!(state.selection instanceof NodeSelection)) {
+    return false;
+  }
+
+  const markRange = getMarkRangeNodeSelection(
+    state.selection,
+    state.schema.marks.link
+  );
+  if (!markRange) {
+    return false;
+  }
+
+  const existingMarks = state.selection.node.attrs.marks ?? [];
+  const updatedMarks = existingMarks.filter(
+    (mark: any) => mark.type !== "link"
+  );
+
+  const nextValidSelection =
+    Selection.findFrom(state.doc.resolve(markRange.to), 1, true) ??
+    TextSelection.create(state.tr.doc, 0);
+  dispatch?.(
+    state.tr
+      .setSelection(nextValidSelection)
+      .setNodeAttribute(state.selection.from, "marks", updatedMarks)
+  );
+  return true;
+};
+
+const toggleLinkTextSelection =
+  (attrs: Attrs): Command =>
+  (state, dispatch) => {
+    if (!(state.selection instanceof TextSelection)) {
+      return false;
+    }
+
+    return toggleMark(state.schema.marks.link, attrs)(state, dispatch);
+  };
+
+const toggleLinkNodeSelection =
+  (attrs: Attrs): Command =>
+  (state, dispatch) => {
+    if (!(state.selection instanceof NodeSelection)) {
+      return false;
+    }
+
+    const existingMarks = state.selection.node.attrs.marks ?? [];
+    const linkMark = existingMarks.find((mark: any) => mark.type === "link");
+    if (linkMark) {
+      return removeLinkNodeSelection()(state, dispatch);
+    } else {
+      return addLinkNodeSelection(attrs)(state, dispatch);
+    }
+  };
+
+export const toggleLink = (attrs: Attrs): Command =>
+  chainCommands(toggleLinkTextSelection(attrs), toggleLinkNodeSelection(attrs));
+
+export const addLink = (attrs: Attrs): Command =>
+  chainCommands(addLinkTextSelection(attrs), addLinkNodeSelection(attrs));
+
+export const openLink = (attrs: Attrs): Command =>
+  chainCommands(openLinkTextSelection(attrs), openLinkNodeSelection(attrs));
+
+export const updateLink = (attrs: Attrs): Command =>
+  chainCommands(updateLinkTextSelection(attrs), updateLinkNodeSelection(attrs));
+
+export const removeLink = (): Command =>
+  chainCommands(removeLinkTextSelection(), removeLinkNodeSelection());

shared/editor/components/Image.tsx

@@ -1,4 +1,4 @@
-import { CrossIcon, DownloadIcon, GlobeIcon } from "outline-icons";
+import { CrossIcon, DownloadIcon, GlobeIcon, ZoomInIcon } from "outline-icons";
 import type { EditorView } from "prosemirror-view";
 import * as React from "react";
 import styled from "styled-components";
@@ -10,12 +10,16 @@ import { ComponentProps } from "../types";
 import { ResizeLeft, ResizeRight } from "./ResizeHandle";
 import useDragResize from "./hooks/useDragResize";
 import { useTranslation } from "react-i18next";
+import find from "lodash/find";
+import { Separator } from "~/components/Actions";
 
 type Props = ComponentProps & {
   /** Callback triggered when the image is clicked */
   onClick: () => void;
   /** Callback triggered when the download button is clicked */
   onDownload?: (event: React.MouseEvent<HTMLButtonElement>) => void;
+  /** Callback triggered when the zoom in button is clicked */
+  onZoomIn?: (event: React.MouseEvent<HTMLButtonElement>) => void;
   /** Callback triggered when the image is resized */
   onChangeSize?: (props: { width: number; height?: number }) => void;
   /** The editor view */
@@ -67,7 +71,11 @@ const Image = (props: Props) => {
   }, [node.attrs.width]);
 
   const sanitizedSrc = sanitizeUrl(src);
-
+  const imgLink =
+    find(node.attrs.marks ?? [], (mark) => mark.type === "link")?.attrs.href ||
+    // Coalescing to `undefined` to avoid empty string in href because empty string
+    // in href still shows pointer on hover and click navigates to nowhere
+    undefined;
   const handleOpen = React.useCallback(() => {
     window.open(sanitizedSrc, "_blank");
   }, [sanitizedSrc]);
@@ -109,11 +117,29 @@ const Image = (props: Props) => {
         {!dragging && width > 60 && isDownloadable && (
           <Actions>
             {isExternalUrl(src) && (
-              <Button onClick={handleOpen} aria-label={t("Open")}>
-                <GlobeIcon />
-              </Button>
+              <>
+                <Button onClick={handleOpen} aria-label={t("Open")}>
+                  <GlobeIcon />
+                </Button>
+                <Separator height={24} />
+              </>
+            )}
+            {imgLink && !isSelected && (
+              <>
+                <Button
+                  // `mousedown` on ancestor `div.ProseMirror` was preventing the `onClick` handler from firing
+                  onMouseDown={(e) => e.stopPropagation()}
+                  onClick={props.onZoomIn}
+                  aria-label={t("Zoom In")}
+                >
+                  <ZoomInIcon />
+                </Button>
+                <Separator height={24} />
+              </>
             )}
             <Button
+              // `mousedown` on ancestor `div.ProseMirror` was preventing the `onClick` handler from firing
+              onMouseDown={(e) => e.stopPropagation()}
               onClick={props.onDownload}
               aria-label={t("Download")}
               disabled={isDownloading}
@@ -127,16 +153,18 @@ const Image = (props: Props) => {
             <CrossIcon size={16} /> Image failed to load
           </Error>
         ) : (
-          <>
+          <a
+            href={imgLink}
+            // Do not show hover preview when the image is selected
+            className={!isSelected ? "use-hover-preview" : ""}
+            target="_blank"
+            rel="noopener noreferrer nofollow"
+          >
             <img
               className={EditorStyleHelper.imageHandle}
               style={{
                 ...widthStyle,
                 display: loaded ? "block" : "none",
-                pointerEvents:
-                  dragging || (!props.isSelected && props.isEditable)
-                    ? "none"
-                    : "all",
               }}
               src={sanitizedSrc}
               alt={node.attrs.alt || ""}
@@ -164,18 +192,18 @@ const Image = (props: Props) => {
               onClick={handleImageClick}
               onTouchStart={handleImageTouchStart}
             />
-            {!loaded && width && height && (
-              <img
-                style={{
-                  ...widthStyle,
-                  display: "block",
-                }}
-                src={`data:image/svg+xml;charset=UTF-8,${encodeURIComponent(
-                  getPlaceholder(width, height)
-                )}`}
-              />
-            )}
-          </>
+          </a>
+        )}
+        {!loaded && width && height && (
+          <img
+            style={{
+              ...widthStyle,
+              display: "block",
+            }}
+            src={`data:image/svg+xml;charset=UTF-8,${encodeURIComponent(
+              getPlaceholder(width, height)
+            )}`}
+          />
         )}
         {isEditable && !isFullWidth && isResizable && (
           <>
@@ -220,7 +248,6 @@ const Actions = styled.div`
   display: flex;
   align-items: center;
   position: absolute;
-  gap: 1px;
   top: 8px;
   right: 8px;
   opacity: 0;

shared/editor/components/Styles.ts

@@ -785,9 +785,6 @@ img.ProseMirror-separator {
 .component-image + img.ProseMirror-separator + br.ProseMirror-trailingBreak {
   display: none;
 }
-.component-image img {
-  cursor: zoom-in;
-}
 
 .${EditorStyleHelper.imageCaption} {
   border: 0;
@@ -895,6 +892,41 @@ h6:not(.placeholder)::before {
   }
 }
 
+.ProseMirror[contenteditable="true"] {
+  & .image-wrapper.ProseMirror-selectednode > a {
+    /* force zoom-in cursor if image node is selected */
+    cursor: zoom-in !important;
+  }
+  &.ProseMirror-focused {
+    .image-wrapper:not(.ProseMirror-selectednode) > a {
+      /* prevents cursor from turning to pointer on pointer down */
+      pointer-events: none;
+    }
+  }
+  &:not(.ProseMirror-focused) {
+    .image-wrapper  {
+      & > a[href] {
+        cursor: pointer;
+      }
+      & > a:not([href]) {
+        /* prevents cursor from turning to pointer on pointer down */
+        pointer-events: none;
+      }
+    }
+  }
+}
+
+.ProseMirror[contenteditable="false"] {
+  .image-wrapper  {
+    & > a[href] {
+      cursor: pointer;
+    }
+    & > a:not([href]) {
+      cursor: zoom-in;
+    }
+  }
+}
+
 .with-emoji {
   margin-${props.rtl ? "right" : "left"}: -1em;
 }

shared/editor/marks/Link.tsx

@@ -1,29 +1,26 @@
 import { Token } from "markdown-it";
-import { toggleMark } from "prosemirror-commands";
 import { InputRule } from "prosemirror-inputrules";
 import { MarkdownSerializerState } from "prosemirror-markdown";
 import {
-  Attrs,
   MarkSpec,
   MarkType,
   Node,
   Mark as ProsemirrorMark,
 } from "prosemirror-model";
-import {
-  Command,
-  EditorState,
-  Plugin,
-  Selection,
-  TextSelection,
-} from "prosemirror-state";
+import { Command, EditorState, Plugin, TextSelection } from "prosemirror-state";
 import { EditorView } from "prosemirror-view";
 import { toast } from "sonner";
 import { isUrl, sanitizeUrl } from "../../utils/urls";
 import { getMarkRange } from "../queries/getMarkRange";
-import { isMarkActive } from "../queries/isMarkActive";
 import Mark from "./Mark";
 import { isInCode } from "../queries/isInCode";
-import { addMark } from "../commands/addMark";
+import {
+  addLink,
+  openLink,
+  removeLink,
+  toggleLink,
+  updateLink,
+} from "../commands/link";
 
 const LINK_INPUT_REGEX = /\[([^[]+)]\((\S+)\)$/;
 
@@ -113,102 +110,32 @@ export default class Link extends Mark {
     ];
   }
 
-  keys({ type }: { type: MarkType }): Record<string, Command> {
+  keys(): Record<string, Command> {
     return {
       "Mod-k": (state, dispatch) => {
         if (state.selection.empty) {
           return false;
         }
 
-        return toggleMark(type, { href: "" })(state, dispatch);
-      },
-      "Mod-Enter": (state) => {
-        if (isMarkActive(type)(state)) {
-          const range = getMarkRange(
-            state.selection.$from,
-            state.schema.marks.link
-          );
-          if (range && range.mark && this.options.onClickLink) {
-            try {
-              const event = new KeyboardEvent("keydown", { metaKey: false });
-              this.options.onClickLink(
-                sanitizeUrl(range.mark.attrs.href),
-                event
-              );
-            } catch (_err) {
-              toast.error(this.options.dictionary.openLinkError);
-            }
-            return true;
-          }
-        }
-        return false;
+        return toggleLink({ href: "" })(state, dispatch);
       },
+      "Mod-Enter": openLink({
+        onClickLink: this.options.onClickLink,
+        dictionary: this.options.dictionary,
+      }),
     };
   }
 
-  commands({ type }: { type: MarkType }) {
+  commands() {
     return {
-      addLink: (attrs: Attrs): Command => addMark(type, attrs),
-      updateLink:
-        (attrs: Attrs): Command =>
-        (state, dispatch) => {
-          const range = getMarkRange(
-            state.selection.$from,
-            state.schema.marks.link
-          );
-
-          if (range && range.mark) {
-            const nextSelection =
-              Selection.findFrom(state.doc.resolve(range.to), 1, true) ??
-              TextSelection.create(state.tr.doc, 0);
-            dispatch?.(
-              state.tr
-                .setSelection(nextSelection)
-                .removeMark(range.from, range.to, state.schema.marks.link)
-                .addMark(
-                  range.from,
-                  range.to,
-                  state.schema.marks.link.create(attrs)
-                )
-            );
-            return true;
-          }
-          return false;
-        },
-      openLink: (): Command => (state) => {
-        const range = getMarkRange(
-          state.selection.$from,
-          state.schema.marks.link
-        );
-        if (range && range.mark && this.options.onClickLink) {
-          try {
-            const event = new KeyboardEvent("keydown", { metaKey: false });
-            this.options.onClickLink(sanitizeUrl(range.mark.attrs.href), event);
-          } catch (_err) {
-            toast.error(this.options.dictionary.openLinkError);
-          }
-          return true;
-        }
-        return false;
-      },
-      removeLink: (): Command => (state, dispatch) => {
-        const range = getMarkRange(
-          state.selection.$from,
-          state.schema.marks.link
-        );
-        if (range && range.mark) {
-          const nextSelection =
-            Selection.findFrom(state.doc.resolve(range.to), 1, true) ??
-            TextSelection.create(state.tr.doc, 0);
-          dispatch?.(
-            state.tr
-              .setSelection(nextSelection)
-              .removeMark(range.from, range.to, range.mark)
-          );
-          return true;
-        }
-        return false;
-      },
+      addLink,
+      updateLink,
+      openLink: (): Command =>
+        openLink({
+          onClickLink: this.options.onClickLink,
+          dictionary: this.options.dictionary,
+        }),
+      removeLink,
     };
   }
 
@@ -268,6 +195,19 @@ export default class Link extends Mark {
               return false;
             }
 
+            // If an image is selected in write mode, disallow navigation to its href
+            const selectedDOMNode = view.nodeDOM(view.state.selection.from);
+            if (
+              view.editable &&
+              selectedDOMNode &&
+              selectedDOMNode instanceof HTMLSpanElement &&
+              selectedDOMNode.classList.contains("component-image") &&
+              event.target instanceof HTMLImageElement &&
+              selectedDOMNode.contains(event.target)
+            ) {
+              return false;
+            }
+
             // clicking a link while editing should show the link toolbar,
             // clicking in read-only will navigate
             if (!view.editable || (view.editable && !view.hasFocus())) {
@@ -278,7 +218,7 @@ export default class Link extends Mark {
                   : "");
 
               try {
-                if (this.options.onClickLink) {
+                if (this.options.onClickLink && href) {
                   event.stopPropagation();
                   event.preventDefault();
                   this.options.onClickLink(sanitizeUrl(href), event);

shared/editor/nodes/Image.tsx

@@ -17,6 +17,7 @@ import { ComponentProps } from "../types";
 import SimpleImage from "./SimpleImage";
 import { LightboxImageFactory } from "../lib/Lightbox";
 import { addComment } from "../commands/comment";
+import { addLink } from "../commands/link";
 
 const imageSizeRegex = /\s=(\d+)?x(\d+)?$/;
 
@@ -326,6 +327,14 @@ export default class Image extends SimpleImage {
       view.dispatch(transaction);
     };
 
+  handleZoomIn =
+    ({ getPos, view }: ComponentProps) =>
+    () => {
+      this.editor.updateActiveLightboxImage(
+        LightboxImageFactory.createLightboxImage(view, getPos())
+      );
+    };
+
   handleClick =
     ({ getPos, view }: ComponentProps) =>
     () => {
@@ -358,6 +367,7 @@ export default class Image extends SimpleImage {
         isDownloading={isDownloading}
         onClick={this.handleClick(props)}
         onDownload={handleDownload}
+        onZoomIn={this.handleZoomIn(props)}
         onChangeSize={this.handleChangeSize(props)}
       >
         <Caption
@@ -515,6 +525,7 @@ export default class Image extends SimpleImage {
         },
       commentOnImage: (): Command =>
         addComment({ userId: this.options.userId }),
+      linkOnImage: (): Command => addLink({ href: "" }),
     };
   }
 

shared/editor/nodes/SimpleImage.tsx

@@ -17,7 +17,9 @@ import Node from "./Node";
 import { LightboxImageFactory } from "../lib/Lightbox";
 
 export default class SimpleImage extends Node {
-  options: Options & { userId?: string };
+  options: Options & {
+    userId?: string;
+  };
 
   get name() {
     return "image";

shared/editor/queries/getMarkRange.ts

@@ -1,4 +1,5 @@
 import { ResolvedPos, MarkType } from "prosemirror-model";
+import { NodeSelection } from "prosemirror-state";
 
 export function getMarkRange($pos?: ResolvedPos, type?: MarkType) {
   if (!$pos || !type) {
@@ -38,3 +39,18 @@ export function getMarkRange($pos?: ResolvedPos, type?: MarkType) {
 
   return { from: startPos, to: endPos, mark };
 }
+
+export function getMarkRangeNodeSelection(
+  selection: NodeSelection,
+  type: MarkType
+) {
+  const mark = (selection.node.attrs.marks ?? []).find(
+    (mark: any) => mark.type === type.name
+  );
+
+  if (!mark) {
+    return false;
+  }
+
+  return { from: selection.from, to: selection.to, mark };
+}

shared/i18n/locales/en_US/translation.json

@@ -1368,5 +1368,6 @@
   "{{ user }} created {{ timeAgo }}": "{{ user }} created {{ timeAgo }}",
   "Caption": "Caption",
   "Open": "Open",
+  "Zoom In": "Zoom In",
   "Error loading data": "Error loading data"
 }