fix: various improvements

apps/remix/app/components/general/envelope-editor/canvas/use-envelope-canvas-contents-layer.ts

@@ -350,9 +350,7 @@ export const useEnvelopeCanvasContentsLayer = ({
 
     applyPageItemsVisibility();
 
-    // Contents may have changed in a way which affects the transformer, e.g.
+    selection.refreshTransformer();
-    // an image attached to the selected content locks its ratio.
-    selection.refreshTransformerConfig();
 
     layer.batchDraw();
   }, [

apps/remix/app/components/general/envelope-editor/canvas/use-envelope-canvas-fields-layer.ts

@@ -388,7 +388,7 @@ export const useEnvelopeCanvasFieldsLayer = ({
 
     applyPageItemsVisibility();
 
-    selection.refreshTransformerConfig();
+    selection.refreshTransformer();
 
     layer.batchDraw();
   }, [

apps/remix/app/components/general/envelope-editor/canvas/use-envelope-canvas-selection.ts

@@ -68,11 +68,16 @@ export const useEnvelopeCanvasSelection = ({ getTransformerConfig, onChange }: U
   const selectionRef = useLatestRef(selection);
 
   /**
-   * Re-resolve the transformer configuration for the current selection, for
+   * Re-sync the transformer after the items change without the selection
-   * when the selected items change in a way which affects it (e.g. an image
+   * changing: re-resolve its configuration (e.g. an image attached to the
-   * being attached to a selected content) without the selection changing.
+   * selected content locks its ratio) and raise it back above the items.
+   *
+   * The handles straddle the selected item's edges, so any item stacked above
+   * the transformer hides part of them and takes their clicks. Rendering
+   * appends new items on top and the contents stacking raises every content,
+   * so each reconcile calls this afterwards.
    */
-  const refreshTransformerConfig = useCallback(() => {
+  const refreshTransformer = useCallback(() => {
     const transformer = transformerRef.current;
 
     if (!transformer) {
@@ -80,6 +85,7 @@ export const useEnvelopeCanvasSelection = ({ getTransformerConfig, onChange }: U
     }
 
     applyTransformerConfig(transformer, selectionRef.current);
+    transformer.moveToTop();
     transformer.forceUpdate();
   }, [applyTransformerConfig]);
 
@@ -97,6 +103,9 @@ export const useEnvelopeCanvasSelection = ({ getTransformerConfig, onChange }: U
       nextSelection.groups[0].moveToTop();
     }
 
+    // Above the item just raised, so its handles stay visible and clickable.
+    transformer?.moveToTop();
+
     setSelectionState(nextSelection);
     onChangeRef.current(nextSelection);
   }, []);
@@ -186,7 +195,6 @@ export const useEnvelopeCanvasSelection = ({ getTransformerConfig, onChange }: U
       keepRatio: false,
       borderStroke: selectionColor,
       anchorStroke: selectionColor,
-      shouldOverdrawWholeArea: true,
       ignoreStroke: true,
       flipEnabled: false,
       anchorStyleFunc: (anchor) => {
@@ -243,7 +251,7 @@ export const useEnvelopeCanvasSelection = ({ getTransformerConfig, onChange }: U
     toggle,
     clear,
     isSelected,
-    refreshTransformerConfig,
+    refreshTransformer,
   };
 };
 

packages/lib/server-only/envelope-content/set-envelope-contents.ts

@@ -10,7 +10,7 @@ import { generateDatabaseId } from '../../universal/id';
 import type { CreateDocumentAuditLogDataResponse } from '../../utils/document-audit-logs';
 import { createDocumentAuditLogData, diffContentChanges } from '../../utils/document-audit-logs';
 import { canContentBeChanged, type EnvelopeIdOptions } from '../../utils/envelope';
-import { assertEnvelopeContentSaveWithinLimits, getDataContentIds } from '../../utils/envelope-content';
+import { assertEnvelopeContentSaveWithinLimits } from '../../utils/envelope-content';
 import { getEnvelopeWhereInput } from '../envelope/get-envelope-by-id';
 
 export type SetEnvelopeContentsOptions = {
@@ -38,8 +38,15 @@ export type SetEnvelopeContentsOptions = {
     contentMeta: TEnvelopeContentMetaInput;
 
     /**
-     * The ID of an uploaded data content (e.g. an image) to attach to the
+     * The ID of a data content (e.g. an image) to attach to the content, or
-     * content, or null for none.
+     * null for none.
+     *
+     * This ID must already be attached to the content, or be null to remove an image.
+     *
+     * This ID parameter is only here to allow users to duplicate data content without
+     * reuploading it.
+     *
+     * To attach a new image you must go through trpc.envelope.content.uploadImage
      */
     dataContentId: string | null;
   }[];
@@ -51,9 +58,6 @@ export type SetEnvelopeContentsOptions = {
  *
  * Existing contents not present in the list are deleted, contents with a
  * matching ID are updated, and the rest are created.
- *
- * Data contents referenced by the list are attached. They are shared rather
- * than copied, and never deleted once no longer referenced.
  */
 export const setEnvelopeContents = async ({
   userId,
@@ -77,7 +81,16 @@ export const setEnvelopeContents = async ({
           id: true,
         },
       },
-      contents: true,
+      contents: {
+        include: {
+          dataContent: {
+            select: {
+              id: true,
+              metadata: true,
+            },
+          },
+        },
+      },
       team: {
         select: {
           organisation: {
@@ -109,7 +122,14 @@ export const setEnvelopeContents = async ({
     });
   }
 
-  const existingContents = envelope.contents;
+  const existingContents = envelope.contents.map(({ dataContent: _dataContent, ...content }) => content);
+
+  // Create a map of all the existing data contents on this envelope.
+  const attachedDataContents = new Map(
+    envelope.contents.flatMap((content) =>
+      content.dataContent ? [[content.dataContent.id, content.dataContent]] : [],
+    ),
+  );
 
   // The organisation's plan caps how much content an envelope may hold.
   assertEnvelopeContentSaveWithinLimits({
@@ -122,25 +142,6 @@ export const setEnvelopeContents = async ({
     (existingContent) => !contents.some((content) => content.id === existingContent.id),
   );
 
-  const requestedDataContentIds = getDataContentIds(contents);
-
-  // A data content is an immutable blob which any number of contents may point
-  // at, so it is shared rather than copied. Knowing its ID is all that is
-  // needed to attach it: the IDs are random and unguessable, and attaching one
-  // only lets its bytes be displayed, so there is no ownership check.
-  let requestedDataContents: DataContent[] = [];
-
-  // Verify that the data contents exist.
-  if (requestedDataContentIds.length > 0) {
-    requestedDataContents = await prisma.dataContent.findMany({
-      where: {
-        id: {
-          in: requestedDataContentIds,
-        },
-      },
-    });
-  }
-
   const linkedContents = contents.map((content) => {
     const foundEnvelopeItem = envelope.envelopeItems.find((envelopeItem) => envelopeItem.id === content.envelopeItemId);
 
@@ -169,7 +170,7 @@ export const setEnvelopeContents = async ({
     return {
       ...content,
       contentMeta,
-      dataContentId: resolveDataContentId(content.dataContentId, contentMeta.type, requestedDataContents),
+      dataContentId: resolveDataContentId(content.dataContentId, contentMeta.type, attachedDataContents),
       _persisted: persisted,
     };
   });
@@ -331,18 +332,22 @@ export const setEnvelopeContents = async ({
 
 /**
  * The data content to attach to a content, or null for none. Throws if the
- * data content does not exist or is not the type the content type can hold.
+ * data content is not already attached to the envelope or is not the type the
+ * content type can hold.
+ *
+ * Deliberately reports an unattached data content as not found rather than
+ * forbidden, so the response does not reveal whether the ID exists.
  */
 const resolveDataContentId = (
   dataContentId: string | null | undefined,
   contentType: EnvelopeContentType,
-  dataContents: Pick<DataContent, 'id' | 'metadata'>[],
+  attachedDataContents: Map<string, Pick<DataContent, 'id' | 'metadata'>>,
 ) => {
   if (!dataContentId) {
     return null;
   }
 
-  const dataContent = dataContents.find((item) => item.id === dataContentId);
+  const dataContent = attachedDataContents.get(dataContentId);
 
   if (!dataContent) {
     throw new AppError(AppErrorCode.NOT_FOUND, {

packages/lib/universal/content-renderer/content-generic-items.ts

@@ -27,6 +27,10 @@ type ContentGroupPosition = {
  * Draws a box content type's own nodes inside the group, after the shared
  * frame (group, position, hover outline) has been set up.
  *
+ * Called on every render, and again on every tick of a resize with the box
+ * at its current size, so it must lay everything out from `geometry` rather
+ * than assume the size is settled.
+ *
  * `meta` is any box content; the renderer narrows it to its own type via
  * `assertContentMetaType`.
  */
@@ -67,13 +71,61 @@ export const renderBoxContent = (
     rotation: getContentRotation(meta),
   });
 
-  renderChildren({ contentGroup, meta, geometry });
+  /**
+   * Lay the content out at a box: the type's own nodes, then the shared hover
+   * outline around them.
+   */
+  const layoutBox = (box: Rect) => {
+    renderChildren({ contentGroup, meta, geometry: box });
 
-  createContentHoverInteraction(contentGroup, content, options, geometry.width, geometry.height);
+    createContentHoverInteraction(contentGroup, content, options, box.width, box.height);
+  };
+
+  layoutBox(geometry);
+
+  bindBoxContentResize(contentGroup, layoutBox);
 
   return contentGroup;
 };
 
+/**
+ * Keep a box content's strokes and text unscaled while it is being resized.
+ *
+ * The transformer resizes by scaling the group, which scales everything in
+ * it: strokes thicken unevenly and dash patterns stretch until the gesture
+ * ends and the box is re-rendered from its written back size. So on every
+ * transform tick the group scale is folded into the box size, the content is
+ * laid out again at that size and the scale is reset, as the multi item
+ * fields do.
+ *
+ * The write back reads the bounds size multiplied by the group scale, which
+ * folding the scale in early leaves unchanged.
+ */
+const bindBoxContentResize = (contentGroup: Konva.Group, layoutBox: (box: Rect) => void) => {
+  contentGroup.off('transform.contentBox');
+
+  contentGroup.on('transform.contentBox', () => {
+    const boundsNode = contentGroup.findOne(`.${CONTENT_BOUNDS_NODE_NAME}`);
+
+    if (!boundsNode) {
+      return;
+    }
+
+    const box: Rect = {
+      x: contentGroup.x(),
+      y: contentGroup.y(),
+      width: boundsNode.width() * contentGroup.scaleX(),
+      height: boundsNode.height() * contentGroup.scaleY(),
+    };
+
+    contentGroup.scale({ x: 1, y: 1 });
+
+    layoutBox(box);
+
+    contentGroup.getLayer()?.batchDraw();
+  });
+};
+
 /**
  * Upsert the Konva group of a content at the given position.
  *

packages/lib/universal/content-renderer/render-text-content.ts

@@ -79,7 +79,7 @@ export const renderTextContentElement = (content: ContentToRender, options: Rend
       fill: color,
     } satisfies Partial<Konva.TextConfig>);
 
-    const hitRect = upsertContentHitRect(contentGroup, content, geometry.width, geometry.height);
+    upsertContentHitRect(contentGroup, content, geometry.width, geometry.height);
 
     // A persistent dashed outline marks the text box bounds while editing,
     // matching the image placeholder treatment, since the text itself
@@ -109,74 +109,5 @@ export const renderTextContentElement = (content: ContentToRender, options: Rend
 
     // Keep the text above the outline so the dashes never cross the glyphs.
     contentText.moveToTop();
-
-    contentGroup.off('transform.contentText');
-    contentGroup.off('transformend.contentText');
-
-    // The transformer resizes by scaling the group. Counter-scale the text so it
-    // keeps its real size while the box visibly resizes, laying it out against
-    // the scaled bounds in unscaled units. Mirrors the generic text field.
-    contentGroup.on('transform.contentText', () => {
-      const groupScaleX = contentGroup.scaleX();
-      const groupScaleY = contentGroup.scaleY();
-
-      contentText.scaleX(1 / groupScaleX);
-      contentText.scaleY(1 / groupScaleY);
-
-      const scaledWidth = hitRect.width() * groupScaleX;
-      const scaledHeight = hitRect.height() * groupScaleY;
-
-      // During the active transform use the crop bounds (content box only).
-      contentText.x(0);
-      contentText.y(0);
-      contentText.width(scaledWidth);
-      contentText.height(scaledHeight);
-      contentText.wrap('word');
-
-      contentGroup.getLayer()?.batchDraw();
-    });
-
-    // Once the gesture settles, recalculate the overflow layout against the new
-    // box dimensions. The page renderer subsequently re-renders from the
-    // persisted geometry, which resets the group scale.
-    contentGroup.on('transformend.contentText', () => {
-      const groupScaleX = contentGroup.scaleX();
-      const groupScaleY = contentGroup.scaleY();
-
-      contentText.scaleX(1);
-      contentText.scaleY(1);
-
-      const newWidth = hitRect.width() * groupScaleX;
-      const newHeight = hitRect.height() * groupScaleY;
-
-      const newOverflowLayout = calculateOverflowLayout({
-        overflowMode: CONTENT_TEXT_OVERFLOW_MODE,
-        isLabel: false,
-        textToRender,
-        fontSize,
-        fontFamily: konvaTextFontFamily,
-        lineHeight,
-        letterSpacing,
-        textAlign,
-        verticalAlign,
-        baseX: 0,
-        baseY: 0,
-        baseWidth: newWidth,
-        baseHeight: newHeight,
-        groupX: contentGroup.x(),
-        groupY: contentGroup.y(),
-        pageWidth,
-        pageHeight,
-      });
-
-      contentText.x(newOverflowLayout.x);
-      contentText.y(newOverflowLayout.y);
-      contentText.width(newOverflowLayout.width);
-      contentText.height(newOverflowLayout.height);
-      contentText.wrap(newOverflowLayout.wrap);
-      contentText.verticalAlign(newOverflowLayout.verticalAlign);
-
-      contentGroup.getLayer()?.batchDraw();
-    });
   });
 };

packages/trpc/server/envelope-router/set-envelope-contents.types.ts

@@ -17,7 +17,9 @@ export const ZSetEnvelopeContentsRequestSchema = z.object({
       dataContentId: z
         .string()
         .nullable()
-        .describe('The id of an uploaded data content (e.g. an image) to attach, or null for none.'),
+        .describe(
+          'The id of a data content (e.g. an image) already attached to a content on this envelope, or null for none. New images are attached via the upload image route.',
+        ),
     }),
   ),
 });