Minor improvements to Diff UI

docs/project_versions_design.md

@@ -43,7 +43,10 @@ objects *first*, then reads every `versionId.json` (including incomplete ones),
 * All objects are local to a project.  (ie, even if a module is identical across projects there is 
  a copy per project)
 * Modules are compared by ID, so a renamed module is still compared with itself
-* Only the outermost added or removed block is marked
+* Every added or removed block is outlined, but a group of them (a block with the blocks inside
+  and after it that were added or removed too) is one change for next/previous change
+* A block moved out of a removed block into an added block (like a component's port when the
+  component is moved into a mechanism) is part of both groups, not changed
 * Moved top-level stacks count as a change. (but not in the next/previous change)
 
 # Diffs

frontend/index.css

@@ -65,44 +65,72 @@ code {
   scroll-behavior: smooth !important;
 }
 /* Blocks that differ between a version and the current project (see VersionsModal.tsx). The
- * colors are used for the legend too. Only the block's own outline is styled, not its children. */
+ * colors are used for the legend too. Changed and moved blocks have their own outline styled, not
+ * their children's. Added and removed blocks have one outline around each group of them (made by
+ * ReadOnlyBlocklyPreview.tsx), which is masked so only the outer half of its stroke shows. */
 :root {
   --mrc-diff-added: #2e9d4c;
   --mrc-diff-removed: #d9363e;
   --mrc-diff-changed: #e8a317;
   --mrc-diff-moved: var(--mrc-diff-changed);
 }
 
-.mrcDiffAdded > .blocklyPath,
-.mrcDiffRemoved > .blocklyPath,
-.mrcDiffMoved > .blocklyPath,
-.mrcDiffChanged > .blocklyPath {
-  stroke-width: 4px;
+.mrcDiffGroupOutlineStrokes > path {
+  fill: none;
+  /* 2px shows outside the group: half of the stroke, less the 1px that the mask's stroke covers. */
+  stroke-width: 6px;
+  stroke-linejoin: round;
 }
 
-.mrcDiffAdded > .blocklyPath {
+.mrcDiffAdded > .mrcDiffGroupOutline > .mrcDiffGroupOutlineStrokes > path {
   stroke: var(--mrc-diff-added);
 }
 
-.mrcDiffRemoved > .blocklyPath {
+.mrcDiffRemoved > .mrcDiffGroupOutline > .mrcDiffGroupOutlineStrokes > path {
   stroke: var(--mrc-diff-removed);
 }
 
-/* Moved blocks are dotted: zero-length dashes with round caps are drawn as dots. This is before
+/* The paths of the outline's mask are filled, so that the mask hides the stroke inside the group.
+ * They are stroked too, to close the gaps that antialiasing leaves where two blocks meet. */
+.mrcDiffGroupOutline mask path {
+  fill: #000;
+  stroke: #000;
+  stroke-width: 2px;
+}
+
+/* The FIRST blocks style draws a light line inside the top edge of each block, which would cover
+ * part of the outline. */
+.mrcDiffMoved > .firstBlocksStyleTopHighlight,
+.mrcDiffChanged > .firstBlocksStyleTopHighlight {
+  display: none;
+}
+
+/* The block styles set the outline width with selectors like
+ * `.<renderer>-renderer.<theme>-theme .blocklyPath`, so these selectors start with
+ * `.blocklySvg .blocklyBlockCanvas` to be more specific than those.
+ *
+ * Moved blocks are dotted: zero-length dashes with round caps are drawn as dots. This is before
  * changed, so a block that was moved and changed is outlined as changed. */
-.mrcDiffMoved > .blocklyPath {
+.blocklySvg .blocklyBlockCanvas .mrcDiffMoved > .blocklyPath {
   stroke: var(--mrc-diff-moved);
+  /* Wider than the other outlines, since the dots aren't solid. */
+  stroke-width: 4px;
   stroke-dasharray: 0 8;
   stroke-linecap: round;
 }
 
-.mrcDiffChanged > .blocklyPath {
+.blocklySvg .blocklyBlockCanvas .mrcDiffChanged > .blocklyPath {
   stroke: var(--mrc-diff-changed);
+  stroke-width: 2px;
   stroke-dasharray: none;
   stroke-linecap: butt;
 }
 
 /* The change that was last scrolled to with Previous Change or Next Change. */
-.mrcDiffCurrent > .blocklyPath {
-  stroke-width: 7px;
+.blocklySvg .blocklyBlockCanvas .mrcDiffCurrent.mrcDiffChanged > .blocklyPath {
+  stroke-width: 4px;
+}
+
+.mrcDiffCurrent > .mrcDiffGroupOutline > .mrcDiffGroupOutlineStrokes > path {
+  stroke-width: 10px;
 }

frontend/reactComponents/ReadOnlyBlocklyPreview.tsx

@@ -47,13 +47,22 @@ export interface BlockHighlights {
   moved?: Set<string>;
 }
 
-/** The CSS class (defined in index.css) for each kind of highlight. */
+/**
+ * The CSS class (defined in index.css) for each kind of highlight. Added and removed blocks are
+ * outlined in groups (see addGroupOutline), and changed and moved blocks one at a time.
+ */
 const HIGHLIGHT_CLASSES: [keyof BlockHighlights, string][] = [
   ['added', 'mrcDiffAdded'],
   ['removed', 'mrcDiffRemoved'],
   ['changed', 'mrcDiffChanged'],
   ['moved', 'mrcDiffMoved'],
 ];
+const GROUP_HIGHLIGHT_KINDS: (keyof BlockHighlights)[] = ['added', 'removed'];
+const GROUP_OUTLINE_CLASS = 'mrcDiffGroupOutline';
+const GROUP_OUTLINE_STROKES_CLASS = 'mrcDiffGroupOutlineStrokes';
+
+/** Used to give each group outline's mask a unique id. */
+let nextGroupOutlineMaskId = 1;
 
 const FULL_SIZE_STYLE: React.CSSProperties = {
   width: '100%',
@@ -238,23 +247,101 @@ export default function ReadOnlyBlocklyPreview(props: ReadOnlyBlocklyPreviewProp
 }
 
 /**
- * Adds the highlight CSS classes to the given blocks. Returns the top-most highlighted block, or
- * null if no blocks were highlighted.
+ * Highlights the given blocks: adds the highlight CSS classes to them, and outlines each group of
+ * added or removed blocks. Returns the top-most highlighted block, or null if no blocks were
+ * highlighted.
  */
 function highlightBlocks(
     workspace: Blockly.WorkspaceSvg, highlights?: BlockHighlights): Blockly.BlockSvg | null {
+  // The outlines are made from the blocks' rendered shapes.
+  Blockly.renderManagement.triggerQueuedRenders(workspace);
   let first: Blockly.BlockSvg | null = null;
   for (const [kind, className] of HIGHLIGHT_CLASSES) {
-    for (const id of highlights?.[kind] ?? []) {
+    const ids = highlights?.[kind] ?? new Set<string>();
+    for (const id of ids) {
       const block = workspace.getBlockById(id);
       if (!block) {
         continue;
       }
       block.addClass(className);
+      const parent = block.getParent();
+      const isFirstInGroup = !parent || !ids.has(parent.id);
+      if (GROUP_HIGHLIGHT_KINDS.includes(kind) && isFirstInGroup) {
+        addGroupOutline(block, ids);
+      }
       if (!first || block.getRelativeToSurfaceXY().y < first.getRelativeToSurfaceXY().y) {
         first = block;
       }
     }
   }
   return first;
 }
+
+/**
+ * Outlines the outside of the group of blocks that starts with the given block: the block, and the
+ * blocks inside it and after it that are in the given ids too. Each block's outer edge is
+ * stroked, and a mask of all of the shapes hides the strokes inside the group, so only the outside
+ * edge is left. The outline is added to the first block's SVG group, after the blocks in it, so that it
+ * moves with the block and is drawn over the blocks around it.
+ */
+function addGroupOutline(firstBlock: Blockly.BlockSvg, ids: Set<string>): void {
+  const groupBlocks: Blockly.BlockSvg[] = [];
+  const addToGroup = (block: Blockly.BlockSvg): void => {
+    groupBlocks.push(block);
+    for (const child of block.getChildren(false)) {
+      if (ids.has(child.id)) {
+        addToGroup(child);
+      }
+    }
+  };
+  addToGroup(firstBlock);
+
+  const svg = Blockly.utils.Svg;
+  const dom = Blockly.utils.dom;
+  const outline = dom.createSvgElement(svg.G, {
+    'class': GROUP_OUTLINE_CLASS,
+    'pointer-events': 'none',
+  });
+  const maskId = `mrcDiffGroupOutlineMask${nextGroupOutlineMaskId++}`;
+  const mask = dom.createSvgElement<SVGMaskElement>('mask', {
+    'id': maskId,
+    'maskUnits': 'userSpaceOnUse',
+    'x': -1e6, 'y': -1e6, 'width': 2e6, 'height': 2e6,
+  }, dom.createSvgElement(svg.DEFS, {}, outline));
+  dom.createSvgElement(svg.RECT, {
+    'x': -1e6, 'y': -1e6, 'width': 2e6, 'height': 2e6, 'fill': '#fff',
+  }, mask);
+  const strokes = dom.createSvgElement(svg.G, {
+    'class': GROUP_OUTLINE_STROKES_CLASS,
+    'mask': `url(#${maskId})`,
+  }, outline);
+
+  const origin = firstBlock.getRelativeToSurfaceXY();
+  for (const block of groupBlocks) {
+    const d = getOuterPath(block.pathObject.svgPath.getAttribute('d') ?? '');
+    if (!d) {
+      continue;
+    }
+    const xy = block.getRelativeToSurfaceXY();
+    const transform = `translate(${xy.x - origin.x}, ${xy.y - origin.y})`;
+    dom.createSvgElement(svg.PATH, {'d': d, 'transform': transform}, mask);
+    dom.createSvgElement(svg.PATH, {'d': d, 'transform': transform}, strokes);
+  }
+  firstBlock.getSvgRoot().appendChild(outline);
+}
+
+/**
+ * Returns the outer edge of a block from its path: everything before the first move that comes
+ * after something is drawn. The renderers draw the sockets of a block's inline inputs (the ones its
+ * fields and blocks are in) after the outer edge, each starting with a move. The outer edge itself
+ * can start with more than one move, such as ' m 0,0  m 0,8 a ...' for a statement block.
+ */
+function getOuterPath(d: string): string {
+  // Command letters other than moves (and the e of an exponent in a number).
+  const firstDraw = d.search(/[a-df-ln-zA-DF-LN-Z]/);
+  if (firstDraw === -1) {
+    return '';
+  }
+  const nextMove = d.slice(firstDraw).search(/[mM]/);
+  return nextMove === -1 ? d : d.slice(0, firstDraw + nextMove);
+}

frontend/reactComponents/VersionsModal.tsx

@@ -568,7 +568,7 @@ export default function VersionsModal(props: VersionsModalProps): React.JSX.Elem
   // An unchanged module is shown in one preview, since both would show the same blocks.
   const isUnchanged = selectedDiff?.change === versionDiff.ModuleChange.UNCHANGED;
   const changeCount = selectedDiff
-      ? selectedDiff.blockDiff.added.size + selectedDiff.blockDiff.removed.size +
+      ? selectedDiff.blockDiff.addedGroups.size + selectedDiff.blockDiff.removedGroups.size +
           selectedDiff.blockDiff.changed.size
       : 0;
 
@@ -585,10 +585,10 @@ export default function VersionsModal(props: VersionsModalProps): React.JSX.Elem
     }
     const blockDiff = selectedDiff.blockDiff;
     const changes: BlockChange[] = [
-      ...[...blockDiff.removed].map(blockId => ({ blockId, panes: ['left'] as Pane[] })),
+      ...[...blockDiff.removedGroups].map(blockId => ({ blockId, panes: ['left'] as Pane[] })),
       ...[...blockDiff.changed].map(
           blockId => ({ blockId, panes: ['right', 'left'] as Pane[] })),
-      ...[...blockDiff.added].map(blockId => ({ blockId, panes: ['right'] as Pane[] })),
+      ...[...blockDiff.addedGroups].map(blockId => ({ blockId, panes: ['right'] as Pane[] })),
     ];
     const getPosition = (change: BlockChange): Blockly.utils.Coordinate =>
         workspacesRef.current[change.panes[0]]?.getBlockById(change.blockId)
@@ -598,13 +598,13 @@ export default function VersionsModal(props: VersionsModalProps): React.JSX.Elem
         (positions.get(a)!.y - positions.get(b)!.y) || (positions.get(a)!.x - positions.get(b)!.x));
   };
 
-  /** Scrolls the panes to the change at the given index, wrapping around at either end. */
+  /** Scrolls the panes to the change at the given index. */
   const goToChange = (index: number): void => {
     const changes = getChanges();
     if (!changes.length) {
       return;
     }
-    const i = (index + changes.length) % changes.length;
+    const i = Math.min(Math.max(index, 0), changes.length - 1);
     const change = changes[i];
     currentChangeBlocksRef.current.forEach(block => block.removeClass(CURRENT_CHANGE_CLASS));
     currentChangeBlocksRef.current = [];
@@ -683,15 +683,15 @@ export default function VersionsModal(props: VersionsModalProps): React.JSX.Elem
       <Antd.Button
         size="small"
         icon={<ArrowUpOutlined />}
-        disabled={changeCount === 0}
+        disabled={changeCount === 0 || changeIndex <= 0}
         onClick={() => goToChange(changeIndex - 1)}
       >
         {t('VERSIONS.PREVIOUS_CHANGE')}
       </Antd.Button>
       <Antd.Button
         size="small"
         icon={<ArrowDownOutlined />}
-        disabled={changeCount === 0}
+        disabled={changeCount === 0 || changeIndex >= changeCount - 1}
         onClick={() => goToChange(changeIndex + 1)}
       >
         {t('VERSIONS.NEXT_CHANGE')}

frontend/storage/module_content.ts

@@ -56,9 +56,19 @@ function startingBlocksToModuleContentText(
   const privateComponents: Component[] = [];
   const events: Event[] = [];
   const methods: Method[] = [];
+  // The starting blocks don't have ids, so give each block one. Otherwise the blocks would only
+  // get ids when the module is loaded into the editor, and a version created before the module
+  // is saved would have blocks without ids, which can't be matched with the blocks in other
+  // versions. Copy the starting blocks first so the imported JSON isn't changed.
+  const blocks = JSON.parse(JSON.stringify(startingBlocks));
+  visitAllBlockJson(blocks, blockJson => {
+    if (!blockJson.id) {
+      blockJson.id = Blockly.utils.idGenerator.genUid();
+    }
+  });
   return makeModuleContentText(
       module,
-      startingBlocks,
+      blocks,
       mechanisms,
       components,
       privateComponents,