github-mirror/oxynote
1
0
Fork 0
mirror of https://github.com/oxynote/oxynote.git synced 2026-10-17 02:49:18 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

improve diff-modified status on metric, image, and other nested blocks

Browse Source
This commit is contained in:
swithek 2026-09-25 16:07:39 +02:00
parent b2226c79d5
commit 20d87cf8a3
23 changed files with 745 additions and 134 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
web/app/assets/css/main.css
View File

@ -68,7 +68,8 @@
--diff-added: oklch(53.282% 0.16399 143.531 / 0.3);
--diff-removed: oklch(57.909% 0.23191 28.689 / 0.3);
--diff-modified: oklch(70% 0.12 270 / 0.2);
--diff-added-foreground: oklch(42% 0.13 143.5);
--diff-removed-foreground: oklch(45% 0.18 28.7);
--diff-text-added: oklch(53.282% 0.16399 143.531 / 0.3);
--diff-text-removed: oklch(57.909% 0.23191 28.689 / 0.3);
--diff-field-added: oklch(53.282% 0.16399 143.531 / 0.2);
@ -238,7 +239,8 @@
--diff-added: oklch(64.522% 0.20475 143.722 / 0.25);
--diff-removed: oklch(49.991% 0.1933 29.256 / 0.25);
--diff-modified: oklch(70% 0.12 270 / 0.3);
--diff-added-foreground: oklch(82% 0.15 143.7);
--diff-removed-foreground: oklch(80% 0.1 29.3);
--diff-text-added: oklch(64.522% 0.20475 143.722 / 0.25);
--diff-text-removed: oklch(49.991% 0.1933 29.256 / 0.25);
--diff-field-added: oklch(64.522% 0.20475 143.722 / 0.2);
@ -350,7 +352,8 @@
--color-diff-added: var(--diff-added);
--color-diff-removed: var(--diff-removed);
--color-diff-modified: var(--diff-modified);
--color-diff-added-foreground: var(--diff-added-foreground);
--color-diff-removed-foreground: var(--diff-removed-foreground);
--color-diff-text-added: var(--diff-text-added);
--color-diff-text-removed: var(--diff-text-removed);
--color-diff-field-added: var(--diff-field-added);
@ -664,10 +667,6 @@
@apply bg-diff-removed;
}
.diff-modified {
@apply bg-diff-modified;
}
.diff-overlay-anchor {
@apply relative;
}
@ -684,8 +683,8 @@
@apply rounded-md bg-diff-removed;
}
.diff-overlay.diff-modified {
@apply rounded-md bg-diff-modified;
li.diff-checkbox-changed > label {
@apply rounded-sm bg-diff-added outline-[0.15em] outline-diff-added;
}
.diff-text-added {
@ -698,15 +697,13 @@
/* horizontal rules: pseudo-element overlay around the thin line */
hr.diff-added,
hr.diff-removed,
hr.diff-modified {
hr.diff-removed {
@apply relative overflow-visible;
background: none;
}
hr.diff-added::before,
hr.diff-removed::before,
hr.diff-modified::before {
hr.diff-removed::before {
@apply pointer-events-none absolute rounded-md;
content: "";
inset: -0.5em -0.2em -0.4em -0.2em;
@ -719,7 +716,3 @@ hr.diff-added::before {
hr.diff-removed::before {
@apply bg-diff-removed;
}
hr.diff-modified::before {
@apply bg-diff-modified;
}

4
web/app/components/editor/attribute-names.ts Normal file
View File

@ -0,0 +1,4 @@
// node attribute name constants to avoid circular import issues between
// editor modules
export const NODE_COMMENT_ID_ATTR = "nodeCommentId"

16
web/app/components/editor/blocks/figma/FigmaBlock.nuxt.test.ts
View File

@ -137,7 +137,6 @@ describe("<FigmaBlock>", { concurrent: false }, () => {
it.for([
{ status: "added", expected: "diff-added" },
{ status: "removed", expected: "diff-removed" },
{ status: "modified", expected: "diff-modified" },
])(
"marks a $status embed with its diff overlay",
async ({ status, expected }, { expect }) => {
@ -147,6 +146,21 @@ describe("<FigmaBlock>", { concurrent: false }, () => {
},
)
it("counts a modified embed's changes instead of tinting it", async ({
expect,
}) => {
const wrapper = await mountFigma({
src: FIGMA_URL,
diffStatus: "modified",
oldNode: { attrs: { uid: "figma-1", src: OTHER_FIGMA_URL } },
})
expect(wrapper.find(".diff-overlay").exists()).toBe(false)
expect(wrapper.text()).toContain(
t("editor.diff-change-marker.label", { removed: 1, added: 1 }),
)
})
it("shows no diff overlay on an unchanged embed", async ({ expect }) => {
const wrapper = await mountFigma({
src: FIGMA_URL,

8
web/app/components/editor/blocks/figma/FigmaBlock.vue
View File

@ -3,6 +3,7 @@ import { nodeViewProps, NodeViewWrapper } from "@tiptap/vue-3"
import { computePosition, flip, offset, shift } from "@floating-ui/dom"
import { cn } from "~/lib/utils"
import { DiffStatus } from "~/components/editor/diff/position-map"
import DiffChangeMarker from "~/components/editor/diff/DiffChangeMarker.vue"
import { isFigmaUrl, convertToEmbedUrl } from "./index"
const props = defineProps(nodeViewProps)
@ -35,8 +36,6 @@ const diffClass = computed(() => {
return "diff-added"
case DiffStatus.Removed:
return "diff-removed"
case DiffStatus.Modified:
return "diff-modified"
default:
return null
}
@ -263,6 +262,7 @@ onBeforeUnmount(() => {
:style="{ height: `${displayHeight}px` }"
allowfullscreen
/>
<DiffChangeMarker :node="props.node" class="absolute top-1.5 right-1.5" />
<div
v-if="!isEditingDisabled"
aria-hidden="true"
@ -292,6 +292,10 @@ onBeforeUnmount(() => {
@click="openPopover"
>
<Icon name="simple-icons:figma" class="size-4.5 text-foreground" />
<DiffChangeMarker
:node="props.node"
class="absolute top-1/2 right-2 -translate-y-1/2"
/>
<span class="mt-0.25 text-2sm text-muted-foreground">
{{
isEditingDisabled

17
web/app/components/editor/blocks/file/FileBlock.nuxt.test.ts
View File

@ -361,7 +361,6 @@ describe("<FileBlock>", { concurrent: false }, () => {
it.for([
{ status: "added", expected: "diff-added" },
{ status: "removed", expected: "diff-removed" },
{ status: "modified", expected: "diff-modified" },
])(
"marks a $status file with its diff overlay",
async ({ status, expected }, { expect }) => {
@ -372,6 +371,22 @@ describe("<FileBlock>", { concurrent: false }, () => {
},
)
it("counts a modified file's changes instead of tinting it", async ({
expect,
}) => {
const wrapper = await mountFile(
storedZip({
diffStatus: "modified",
oldNode: { attrs: storedZip({ uid: "file-1", size: 1 }) },
}),
)
expect(wrapper.find(".diff-overlay").exists()).toBe(false)
expect(wrapper.text()).toContain(
t("editor.diff-change-marker.label", { removed: 1, added: 1 }),
)
})
it("shows no diff overlay on an unchanged file", async ({ expect }) => {
const wrapper = await mountFile(storedZip({ diffStatus: "unchanged" }))

11
web/app/components/editor/blocks/file/FileBlock.vue
View File

@ -5,6 +5,7 @@ import { showProgressToast, showToastMessage } from "~/components/toast"
import type { ProgressToast } from "~/components/toast"
import { cn } from "~/lib/utils"
import { DiffStatus } from "~/components/editor/diff/position-map"
import DiffChangeMarker from "~/components/editor/diff/DiffChangeMarker.vue"
import {
fileKind,
fileKindStyle,
@ -57,8 +58,6 @@ const diffClass = computed(() => {
return "diff-added"
case DiffStatus.Removed:
return "diff-removed"
case DiffStatus.Modified:
return "diff-modified"
default:
return null
}
@ -234,6 +233,10 @@ async function downloadOnDesktop() {
{{ size }}
</span>
</span>
<DiffChangeMarker
:node="props.node"
class="absolute top-1/2 right-2 -translate-y-1/2"
/>
</component>
<button
v-else
@ -249,6 +252,10 @@ async function downloadOnDesktop() {
@click="openFilePicker"
>
<Icon name="mingcute:attachment-line" class="size-4 text-foreground" />
<DiffChangeMarker
:node="props.node"
class="absolute top-1/2 right-2 -translate-y-1/2"
/>
<div class="mt-0.25 text-2sm text-muted-foreground">
{{
uploading

16
web/app/components/editor/blocks/image/ImageBlock.nuxt.test.ts
View File

@ -230,7 +230,6 @@ describe("<ImageBlock>", { concurrent: false }, () => {
it.for([
{ status: "added", expected: "diff-added" },
{ status: "removed", expected: "diff-removed" },
{ status: "modified", expected: "diff-modified" },
])(
"marks a $status image with its diff overlay",
async ({ status, expected }, { expect }) => {
@ -243,6 +242,21 @@ describe("<ImageBlock>", { concurrent: false }, () => {
},
)
it("counts a modified image's changes instead of tinting it", async ({
expect,
}) => {
const wrapper = await mountImage({
src: "https://cdn.test/b.png",
diffStatus: "modified",
oldNode: { attrs: { uid: "image-1", src: "https://cdn.test/a.png" } },
})
expect(wrapper.find(".diff-overlay").exists()).toBe(false)
expect(wrapper.text()).toContain(
t("editor.diff-change-marker.label", { removed: 1, added: 1 }),
)
})
it("shows no diff overlay on an unchanged image", async ({ expect }) => {
const wrapper = await mountImage({
src: "https://cdn.test/a.png",

8
web/app/components/editor/blocks/image/ImageBlock.vue
View File

@ -4,6 +4,7 @@ import { nanoid } from "nanoid"
import { showToastMessage } from "~/components/toast"
import { cn } from "~/lib/utils"
import { DiffStatus } from "~/components/editor/diff/position-map"
import DiffChangeMarker from "~/components/editor/diff/DiffChangeMarker.vue"
const MIN_WIDTH = 128
@ -37,8 +38,6 @@ const diffClass = computed(() => {
return "diff-added"
case DiffStatus.Removed:
return "diff-removed"
case DiffStatus.Modified:
return "diff-modified"
default:
return null
}
@ -249,6 +248,7 @@ onBeforeUnmount(() => {
]"
draggable="false"
/>
<DiffChangeMarker :node="props.node" class="absolute top-1.5 right-1.5" />
<div
v-if="showResizeHandle"
aria-hidden="true"
@ -272,6 +272,10 @@ onBeforeUnmount(() => {
@click="openFilePicker"
>
<Icon name="lucide:image" class="size-4.5 text-foreground" />
<DiffChangeMarker
:node="props.node"
class="absolute top-1/2 right-2 -translate-y-1/2"
/>
<div class="mt-0.25 text-2sm text-muted-foreground">
{{
uploading

16
web/app/components/editor/blocks/mermaid/MainBlock.nuxt.test.ts
View File

@ -2,6 +2,7 @@ import type { VueWrapper } from "@vue/test-utils"
import { beforeEach, describe, it, vi } from "vitest"
import MainBlock from "./MainBlock.vue"
import MermaidPreview from "./MermaidPreview.vue"
import DiffChangeMarker from "../../diff/DiffChangeMarker.vue"
import {
makeEditor,
makeNode,
@ -260,7 +261,7 @@ describe("<MermaidMainBlock>", { concurrent: false }, () => {
},
)
it("marks a modified block only while its code is hidden", async ({
it("counts a modified block's changes instead of tinting it", async ({
expect,
}) => {
const wrapper = await mountMermaid({
@ -268,21 +269,10 @@ describe("<MermaidMainBlock>", { concurrent: false }, () => {
attrs: { diffStatus: DiffStatus.Modified },
})
expect(wrapper.get("[data-node-view-wrapper]").classes()).toContain(
"diff-modified",
)
})
it("leaves a modified block's colour to the inline diff while its code shows", async ({
expect,
}) => {
const wrapper = await mountMermaid({
attrs: { diffStatus: DiffStatus.Modified },
})
expect(wrapper.get("[data-node-view-wrapper]").classes()).not.toContain(
"diff-modified",
)
expect(wrapper.findComponent(DiffChangeMarker).exists()).toBe(true)
})
it("shows no diff colour on an unchanged block", async ({ expect }) => {

11
web/app/components/editor/blocks/mermaid/MainBlock.vue
View File

@ -3,6 +3,7 @@ import { NodeViewContent, nodeViewProps, NodeViewWrapper } from "@tiptap/vue-3"
import { cn } from "~/lib/utils"
import { explicitContentPlaceholder } from "../../placeholder"
import { DiffStatus } from "../../diff/position-map"
import DiffChangeMarker from "../../diff/DiffChangeMarker.vue"
import MermaidPreview from "./MermaidPreview.vue"
const props = defineProps(nodeViewProps)
@ -55,9 +56,6 @@ const diffStatus = computed(
() => props.node.attrs.diffStatus as DiffStatus | null,
)
// for added/removed, apply node-level diff class always.
// for modified, only apply when the code is hidden (otherwise
// text-level inline diffs are shown).
const diffClass = computed(() => {
if (diffStatus.value === DiffStatus.Added) {
return "diff-added"
@ -67,10 +65,6 @@ const diffClass = computed(() => {
return "diff-removed"
}
if (diffStatus.value === DiffStatus.Modified && !showCode.value) {
return "diff-modified"
}
return null
})
@ -124,7 +118,8 @@ watch(showCode, () => {
>
{{ lastOtherEditingUser?.name }}
</div>
<div class="absolute top-1 right-1.5 z-1">
<div class="absolute top-1 right-1.5 z-1 flex items-center gap-1">
<DiffChangeMarker :node="props.node" />
<ShadcnUiButton
variant="ghost-plain"
size="icon-sm"

19
web/app/components/editor/blocks/metrics/MainBlock.nuxt.test.ts
View File

@ -21,7 +21,7 @@ import {
disposeMockEndpoints,
makeXid,
} from "~/composables/api/test-helpers"
import { emitFrom, renderedIconNames } from "~/components/test-helpers"
import { emitFrom, renderedIconNames, t } from "~/components/test-helpers"
// echarts measures text on a canvas, which happy-dom does not implement
vi.mock("./visualizations/LineChart.vue", () => ({
@ -264,6 +264,23 @@ describe("<MetricMainBlock>", { concurrent: false }, () => {
).toEqual(expect.objectContaining({ title: "Before" }))
})
it("counts a modified block's changes", async ({ expect }) => {
const uid = nextUid()
const wrapper = await mountBlock({
uid: uid,
attrs: {
title: "After",
diffStatus: DiffStatus.Modified,
oldNode: { attrs: { uid: uid, title: "Before" } },
},
})
expect(wrapper.text()).toContain(
t("editor.diff-change-marker.label", { removed: 1, added: 1 }),
)
})
it("publishes no previous config for an added block", async ({ expect }) => {
const uid = nextUid()

2
web/app/components/editor/blocks/metrics/MainBlock.vue
View File

@ -11,6 +11,7 @@ import {
buildConfigFromNodeAttrs,
} from "./utils"
import { DiffStatus } from "~/components/editor/diff/position-map"
import DiffChangeMarker from "~/components/editor/diff/DiffChangeMarker.vue"
// legacy blobs may miss fields entirely and may still use the old "type"
// field name, so Partial reflects the actual runtime shape.
@ -370,6 +371,7 @@ function disableVisualizationRefreshTemporarily() {
<div
class="absolute top-1 right-1.5 z-1 flex items-center rounded-md bg-background pl-0.5"
>
<DiffChangeMarker :node="props.node" class="mr-1" />
<ShadcnUiTooltip v-if="simulationVisible">
<ShadcnUiTooltipTrigger as-child>
<Icon

2
web/app/components/editor/comments/node-comment-extension.ts
View File

@ -7,8 +7,6 @@ import { DIFF_COMMENT_TX_META } from "../diff/diff-content-lock"
import { DiffStatus, type PositionMap } from "../diff/position-map"
import type { DocumentComment } from "~/utils/api/comment"
export const NODE_COMMENT_ID_ATTR = "nodeCommentId"
export interface NodeCommentAttrs {
nodeCommentId: string
}

72
web/app/components/editor/diff/DiffChangeMarker.nuxt.test.ts Normal file
View File

@ -0,0 +1,72 @@
import { mountSuspended } from "@nuxt/test-utils/runtime"
import { describe, it } from "vitest"
import DiffChangeMarker from "./DiffChangeMarker.vue"
import { DiffStatus } from "./position-map"
import { makeNode } from "../test-helpers/node-view"
import { t } from "~/components/test-helpers"
describe("DiffChangeMarker", () => {
it("shows the removals and additions of a modified node", async ({
expect,
}) => {
const wrapper = await mountMarker({
src: "b.png",
alt: "a cat",
diffStatus: DiffStatus.Modified,
oldNode: { attrs: { src: "a.png" } },
})
expect(
wrapper.findAll("[aria-hidden='true']").map((s) => s.text()),
).toEqual([
t("editor.diff-change-marker.removed", { count: 1 }),
t("editor.diff-change-marker.added", { count: 2 }),
])
expect(wrapper.get(".sr-only").text()).toBe(
t("editor.diff-change-marker.label", { removed: 1, added: 2 }),
)
})
it("leaves out the side without changes", async ({ expect }) => {
const wrapper = await mountMarker({
alt: "a cat",
diffStatus: DiffStatus.Modified,
oldNode: { attrs: {} },
})
expect(
wrapper.findAll("[aria-hidden='true']").map((s) => s.text()),
).toEqual([t("editor.diff-change-marker.added", { count: 1 })])
})
it("renders nothing for a modified node whose counted attributes match", async ({
expect,
}) => {
const wrapper = await mountMarker({
src: "a.png",
diffStatus: DiffStatus.Modified,
oldNode: { attrs: { src: "a.png" } },
})
expect(wrapper.html()).toBe("<!--v-if-->")
})
it.for([DiffStatus.Added, DiffStatus.Removed, DiffStatus.Unchanged])(
"renders nothing for a %s node",
async (status, { expect }) => {
const wrapper = await mountMarker({
src: "b.png",
diffStatus: status,
oldNode: { attrs: { src: "a.png" } },
})
expect(wrapper.html()).toBe("<!--v-if-->")
},
)
})
function mountMarker(attrs: Record<string, unknown>) {
return mountSuspended(DiffChangeMarker, {
props: { node: makeNode(attrs) },
})
}

41
web/app/components/editor/diff/DiffChangeMarker.vue Normal file
View File

@ -0,0 +1,41 @@
<script lang="ts" setup>
import type { Node as PMNode } from "@tiptap/pm/model"
import { countNodeChanges } from "./change-count"
import { DiffStatus } from "./position-map"
const props = defineProps<{
node: PMNode
}>()
const count = computed(() =>
props.node.attrs.diffStatus === DiffStatus.Modified
? countNodeChanges(props.node)
: null,
)
</script>
<template>
<span
v-if="count && (count.removed || count.added)"
contenteditable="false"
class="pointer-events-none flex overflow-hidden rounded-full border border-border bg-background text-xs font-semibold select-none"
>
<span class="sr-only">
{{ $t("editor.diff-change-marker.label", { ...count }) }}
</span>
<span
v-if="count.removed"
aria-hidden="true"
class="bg-diff-removed px-2 py-0.5 text-diff-removed-foreground"
>
{{ $t("editor.diff-change-marker.removed", { count: count.removed }) }}
</span>
<span
v-if="count.added"
aria-hidden="true"
class="bg-diff-added px-2 py-0.5 text-diff-added-foreground"
>
{{ $t("editor.diff-change-marker.added", { count: count.added }) }}
</span>
</span>
</template>

315
web/app/components/editor/diff/change-count.test.ts Normal file
View File

@ -0,0 +1,315 @@
import { Schema, type Node as PMNode } from "@tiptap/pm/model"
import { describe, it } from "vitest"
import { countNodeChanges } from "./change-count"
import { DiffStatus } from "./position-map"
import {
COMMENT_MARK_NAME,
DIFF_TEXT_ADDED_MARK_NAME,
DIFF_TEXT_REMOVED_MARK_NAME,
} from "~/components/editor/mark-names"
import { NODE_COMMENT_ID_ATTR } from "~/components/editor/attribute-names"
import { SIMULATION_ACTIVE_ATTR } from "~/components/editor/blocks/metrics/simulation"
import {
FILE_BLOCK_NAME,
METRIC_BLOCK_NAME,
} from "~/components/editor/blocks/node-names"
const ADDED = DIFF_TEXT_ADDED_MARK_NAME
const REMOVED = DIFF_TEXT_REMOVED_MARK_NAME
// an atom stands in for attribute-driven blocks (image, metrics), a code
// textblock for mermaid. Attrs mirror what computeMergedDocument stamps.
const schema = new Schema({
nodes: {
doc: { content: "block*" },
atom: {
group: "block",
atom: true,
attrs: {
uid: { default: null },
src: { default: null },
alt: { default: null },
width: { default: null },
queries: { default: null },
[NODE_COMMENT_ID_ATTR]: { default: null },
[SIMULATION_ACTIVE_ATTR]: { default: null },
diffStatus: { default: null },
modifiedIndex: { default: null },
originalIndex: { default: null },
oldNode: { default: null },
},
},
[FILE_BLOCK_NAME]: {
group: "block",
atom: true,
attrs: {
uid: { default: null },
src: { default: null },
name: { default: null },
size: { default: null },
contentType: { default: null },
uploading: { default: false },
diffStatus: { default: null },
oldNode: { default: null },
},
},
[METRIC_BLOCK_NAME]: {
group: "block",
atom: true,
attrs: {
uid: { default: null },
title: { default: null },
queries: { default: null },
diffStatus: { default: null },
oldNode: { default: null },
},
},
code: {
group: "block",
content: "text*",
marks: "_",
attrs: {
uid: { default: null },
diffStatus: { default: null },
oldNode: { default: null },
modifiedTextContent: { default: null },
},
},
text: {},
},
marks: {
[COMMENT_MARK_NAME]: {},
[DIFF_TEXT_ADDED_MARK_NAME]: {},
[DIFF_TEXT_REMOVED_MARK_NAME]: {},
},
})
describe("countNodeChanges", () => {
it.for([
{
name: "counts a changed attribute as one removal and one addition",
input: { old: { src: "a.png" }, new: { src: "b.png" } },
expected: { removed: 1, added: 1 },
},
{
name: "counts a newly set attribute as one addition",
input: { old: { alt: null }, new: { alt: "a cat" } },
expected: { removed: 0, added: 1 },
},
{
name: "counts an attribute missing from the original as an addition",
input: { old: {}, new: { alt: "a cat" } },
expected: { removed: 0, added: 1 },
},
{
name: "counts a cleared attribute as one removal",
input: { old: { width: 300 }, new: { width: null } },
expected: { removed: 1, added: 0 },
},
{
name: "sums the changes across attributes",
input: {
old: { src: "a.png", alt: "a cat", width: null },
new: { src: "b.png", alt: null, width: 300 },
},
expected: { removed: 2, added: 2 },
},
{
name: "compares object attributes by content, not key order",
input: {
old: { queries: [{ expr: "up", legend: "x" }] },
new: { queries: [{ legend: "x", expr: "up" }] },
},
expected: { removed: 0, added: 0 },
},
{
name: "ignores the attributes the diff leaves out of its comparison",
input: {
old: { [NODE_COMMENT_ID_ATTR]: "c1", [SIMULATION_ACTIVE_ATTR]: false },
new: { [NODE_COMMENT_ID_ATTR]: "c2", [SIMULATION_ACTIVE_ATTR]: true },
},
expected: { removed: 0, added: 0 },
},
])("$name", ({ input, expected }, { expect }) => {
const node = schema.nodes.atom.create({
uid: "block-1",
...input.new,
diffStatus: DiffStatus.Modified,
modifiedIndex: 0,
originalIndex: 0,
oldNode: { type: "atom", attrs: { uid: "block-1", ...input.old } },
})
expect(countNodeChanges(node)).toEqual(expected)
})
it("counts no attribute changes on a node without its original", ({
expect,
}) => {
const node = schema.nodes.atom.create({
uid: "block-1",
src: "a.png",
diffStatus: DiffStatus.Modified,
})
expect(countNodeChanges(node)).toEqual({ removed: 0, added: 0 })
})
it.for([
{
name: "counts an uploaded file as one addition",
input: { old: {}, new: storedFile("a.zip") },
expected: { removed: 0, added: 1 },
},
{
name: "counts a replaced file as one removal and one addition",
input: { old: storedFile("a.zip"), new: storedFile("b.zip") },
expected: { removed: 1, added: 1 },
},
{
name: "counts a removed file as one removal",
input: { old: storedFile("a.zip"), new: {} },
expected: { removed: 1, added: 0 },
},
{
name: "counts the attributes outside the file group on their own",
input: {
old: storedFile("a.zip"),
new: { ...storedFile("a.zip"), uploading: true },
},
expected: { removed: 1, added: 1 },
},
])("$name", ({ input, expected }, { expect }) => {
const node = schema.nodes[FILE_BLOCK_NAME].create({
uid: "file-1",
...input.new,
diffStatus: DiffStatus.Modified,
oldNode: {
type: FILE_BLOCK_NAME,
attrs: { uid: "file-1", uploading: false, ...input.old },
},
})
expect(countNodeChanges(node)).toEqual(expected)
})
it.for([
{
name: "counts an added query by its query and legend",
input: {
old: [query("up", "")],
new: [query("up", ""), query("rate", "")],
},
expected: { removed: 0, added: 2 },
},
{
name: "counts every added query",
input: {
old: [query("up", "")],
new: [query("up", ""), query("rate", ""), query("sum", "{{job}}")],
},
expected: { removed: 0, added: 4 },
},
{
name: "counts a removed query by its query and legend",
input: {
old: [query("up", ""), query("rate", "")],
new: [query("up", "")],
},
expected: { removed: 2, added: 0 },
},
{
name: "counts an edited legend as one removal and one addition",
input: { old: [query("up", "")], new: [query("up", "{{job}}")] },
expected: { removed: 1, added: 1 },
},
{
name: "ignores the generated query names",
input: {
old: [{ ...query("up", ""), name: "Query 1" }],
new: [{ ...query("up", ""), name: "Query 2" }],
},
expected: { removed: 0, added: 0 },
},
])("$name", ({ input, expected }, { expect }) => {
const node = schema.nodes[METRIC_BLOCK_NAME].create({
uid: "metric-1",
queries: input.new,
diffStatus: DiffStatus.Modified,
oldNode: {
type: METRIC_BLOCK_NAME,
attrs: { uid: "metric-1", queries: input.old },
},
})
expect(countNodeChanges(node)).toEqual(expected)
})
it.for([
{
name: "counts a replaced run as one removal and one addition",
input: [
["graph TD\n"],
["A-->B", REMOVED],
["A-->C", ADDED],
["\nB-->D"],
],
expected: { removed: 1, added: 1 },
},
{
name: "counts every run on the same line",
input: [
["A-->"],
["B", REMOVED],
["C", ADDED],
[" & "],
["D", REMOVED],
[" & E"],
[" & F", ADDED],
],
expected: { removed: 2, added: 2 },
},
{
name: "counts a run spanning several lines once",
input: [["graph TD"], ["\nA-->B\nB-->C", REMOVED]],
expected: { removed: 1, added: 0 },
},
{
name: "counts a run split by another mark once",
input: [["graph TD\n"], ["A-->", ADDED], ["B", ADDED, COMMENT_MARK_NAME]],
expected: { removed: 0, added: 1 },
},
{
name: "counts no runs when the text is unchanged",
input: [["graph TD\nA-->B"]],
expected: { removed: 0, added: 0 },
},
])("$name", ({ input, expected }, { expect }) => {
const node = schema.nodes.code.create(
{ uid: "code-1", diffStatus: DiffStatus.Modified },
input.map(([text, ...marks]) => textNode(text ?? "", marks)),
)
expect(countNodeChanges(node)).toEqual(expected)
})
})
function query(expr: string, legendFormat: string): Record<string, unknown> {
return { name: "Query", query: expr, legendFormat: legendFormat }
}
function storedFile(name: string): Record<string, unknown> {
return {
src: `/files/${name}`,
name: name,
size: 1024,
contentType: "application/zip",
}
}
function textNode(text: string, marks: string[]): PMNode {
return schema.text(
text,
marks.map((mark) => schema.mark(mark)),
)
}

181
web/app/components/editor/diff/change-count.ts Normal file
View File

@ -0,0 +1,181 @@
import type { JSONContent } from "@tiptap/core"
import type { Node as PMNode } from "@tiptap/pm/model"
import {
DIFF_TEXT_ADDED_MARK_NAME,
DIFF_TEXT_REMOVED_MARK_NAME,
} from "~/components/editor/mark-names"
import { NODE_COMMENT_ID_ATTR } from "~/components/editor/attribute-names"
import { SIMULATION_ACTIVE_ATTR } from "~/components/editor/blocks/metrics/simulation"
import {
FILE_BLOCK_NAME,
METRIC_BLOCK_NAME,
} from "~/components/editor/blocks/node-names"
export interface ChangeCount {
removed: number
added: number
}
// the attributes the diff itself writes onto merged nodes
const DIFF_META_ATTRIBUTES = [
"diffStatus",
"modifiedIndex",
"originalIndex",
"oldNode",
"modifiedTextContent",
]
const IGNORED_ATTRIBUTES = new Set([
...DIFF_META_ATTRIBUTES,
NODE_COMMENT_ID_ATTR,
SIMULATION_ACTIVE_ATTR,
])
// attributes that change together and count as one change, per node
// type. Each attribute maps to the name of its group.
const ATTRIBUTE_GROUPS: Record<string, Record<string, string>> = {
[FILE_BLOCK_NAME]: {
src: "file",
name: "file",
size: "file",
contentType: "file",
},
}
// list attributes whose items count field by field, per node type. Only
// the listed fields of each item are counted.
const ITEM_FIELDS: Record<string, Record<string, string[]>> = {
[METRIC_BLOCK_NAME]: {
queries: ["query", "legendFormat"],
},
}
// count the changes of a modified node against its original. A changed
// attribute counts as one removal and one addition, a newly set one as an
// addition and a cleared one as a removal. A group of attributes counts
// like one attribute. Changed text counts by run.
export function countNodeChanges(node: PMNode): ChangeCount {
// an expanded textblock drops its oldNode, since its marks carry the
// original instead
const oldNode = node.attrs.oldNode as JSONContent | null
const attributes = oldNode
? countAttributeChanges(node.type.name, oldNode.attrs ?? {}, node.attrs)
: { removed: 0, added: 0 }
const text = countTextChanges(node)
return {
removed: attributes.removed + text.removed,
added: attributes.added + text.added,
}
}
function countAttributeChanges(
typeName: string,
oldAttrs: Record<string, unknown>,
newAttrs: Record<string, unknown>,
): ChangeCount {
const count: ChangeCount = { removed: 0, added: 0 }
const oldUnits = attributeUnits(typeName, oldAttrs)
const newUnits = attributeUnits(typeName, newAttrs)
const names = new Set([...oldUnits.keys(), ...newUnits.keys()])
for (const name of names) {
const oldUnit = oldUnits.get(name)
const newUnit = newUnits.get(name)
if (jsonStableStringify(oldUnit) === jsonStableStringify(newUnit)) {
continue
}
if (oldUnit) {
count.removed++
}
if (newUnit) {
count.added++
}
}
return count
}
// sort the set attributes into units, keyed by their group name, or by
// their own name when they have no group. A list attribute with item fields
// becomes one unit per field of each item instead. A unit whose attributes
// are all empty is left out.
function attributeUnits(
typeName: string,
attrs: Record<string, unknown>,
): Map<string, Record<string, unknown>> {
const groups = ATTRIBUTE_GROUPS[typeName] ?? {}
const itemFields = ITEM_FIELDS[typeName] ?? {}
const units = new Map<string, Record<string, unknown>>()
for (const [key, value] of Object.entries(attrs)) {
if (IGNORED_ATTRIBUTES.has(key) || value === null || value === undefined) {
continue
}
const fields = itemFields[key]
if (fields && Array.isArray(value)) {
addItemUnits(units, key, value as Record<string, unknown>[], fields)
continue
}
const name = groups[key] ?? key
units.set(name, { ...units.get(name), [key]: value })
}
return units
}
function addItemUnits(
units: Map<string, Record<string, unknown>>,
key: string,
items: Record<string, unknown>[],
fields: string[],
) {
for (const [index, item] of items.entries()) {
for (const field of fields) {
const value = item[field]
if (value !== null && value !== undefined) {
units.set(`${key}.${index}.${field}`, { [field]: value })
}
}
}
}
// a modified textblock holds both sides of its text, told apart by the
// added and removed marks. Each unbroken run of one mark is one change,
// so several can sit on the same line.
function countTextChanges(node: PMNode): ChangeCount {
const count: ChangeCount = { removed: 0, added: 0 }
let previous: string | null = null
node.descendants((child) => {
if (!child.isText) {
return true
}
const marks = child.marks.map((mark) => mark.type.name)
let current: string | null = null
if (marks.includes(DIFF_TEXT_REMOVED_MARK_NAME)) {
current = DIFF_TEXT_REMOVED_MARK_NAME
} else if (marks.includes(DIFF_TEXT_ADDED_MARK_NAME)) {
current = DIFF_TEXT_ADDED_MARK_NAME
}
if (current === DIFF_TEXT_REMOVED_MARK_NAME && previous !== current) {
count.removed++
}
if (current === DIFF_TEXT_ADDED_MARK_NAME && previous !== current) {
count.added++
}
previous = current
return false
})
return count
}

15
web/app/components/editor/diff/config.ts
View File

@ -3,7 +3,6 @@ import {
FILE_BLOCK_NAME,
IMAGE_BLOCK_NAME,
MERMAID_BLOCK_NAME,
METRIC_BLOCK_NAME,
METRIC_GRID_NAME,
TITLED_CODE_BLOCK_NAME,
SPLIT_DOCUMENTATION_NAME,
@ -14,9 +13,8 @@ import {
SPLIT_DOCUMENTATION_PARAMETER_LIST_ITEM_HEADER_NAME,
} from "~/components/editor/blocks/node-names"
import Paragraph from "@tiptap/extension-paragraph"
import HorizontalRule from "@tiptap/extension-horizontal-rule"
import { COMMENT_MARK_NAME } from "~/components/editor/mark-names"
import { NODE_COMMENT_ID_ATTR } from "~/components/editor/comments/node-comment-extension"
import { NODE_COMMENT_ID_ATTR } from "~/components/editor/attribute-names"
import { SIMULATION_ACTIVE_ATTR } from "~/components/editor/blocks/metrics/simulation"
import {
BulletList,
@ -28,17 +26,6 @@ import {
import type { MergeOptions } from "./compute-merged-document"
import type { OverlayPadding } from "./diff-decorations"
/**
* node types treated as opaque — when modified, the whole node gets
* a diff-modified class instead of character-level inline diffs.
*/
export const DEFAULT_OPAQUE_TYPES: string[] = [
IMAGE_BLOCK_NAME,
FILE_BLOCK_NAME,
METRIC_BLOCK_NAME,
HorizontalRule.name,
]
/**
* node types that handle diff styling inside their node-view component
* rather than via ProseMirror decorations on the root element.

2
web/app/components/editor/diff/content-hash.ts
View File

@ -1,6 +1,6 @@
import type { JSONContent } from "@tiptap/core"
import { COMMENT_MARK_NAME } from "~/components/editor/mark-names"
import { NODE_COMMENT_ID_ATTR } from "~/components/editor/comments/node-comment-extension"
import { NODE_COMMENT_ID_ATTR } from "~/components/editor/attribute-names"
import { SIMULATION_ACTIVE_ATTR } from "~/components/editor/blocks/metrics/simulation"
export interface HashOptions {

40
web/app/components/editor/diff/diff-decorations.browser.test.ts
View File

@ -43,7 +43,7 @@ const ImageBlockStub = Node.create({
},
})
function makeEditor(content: JSONContent, opaqueTypes?: string[]): Editor {
function makeEditor(content: JSONContent): Editor {
const editor = new Editor({
element: document.createElement("div"),
injectCSS: false,
@ -65,9 +65,7 @@ function makeEditor(content: JSONContent, opaqueTypes?: string[]): Editor {
IMAGE_BLOCK_NAME,
],
}),
opaqueTypes
? DiffDecorations.configure({ opaqueTypes })
: DiffDecorations,
DiffDecorations,
],
})
@ -152,9 +150,7 @@ describe("DiffDecorations", () => {
editor.destroy()
})
it("marks a modified opaque node with the diff-modified class", ({
expect,
}) => {
it("leaves a modified horizontal rule undecorated", ({ expect }) => {
const editor = makeEditor(
doc({
type: "horizontalRule",
@ -162,29 +158,7 @@ describe("DiffDecorations", () => {
}),
)
const hr = editor.view.dom.querySelector("hr")
expect(hr?.classList.contains("diff-modified")).toBe(true)
editor.destroy()
})
it("treats configured opaque types as whole-node modifications", ({
expect,
}) => {
const editor = makeEditor(
doc(
heading("new title", {
diffStatus: DiffStatus.Modified,
oldNode: heading("old title"),
}),
),
[Heading.name],
)
const h1 = editor.view.dom.querySelector("h1")
expect(h1?.classList.contains("diff-modified")).toBe(true)
expect(editor.view.dom.querySelector(".diff-text-added")).toBeNull()
expect(editor.view.dom.querySelector(".diff-text-removed")).toBeNull()
expect(diffElements(editor)).toHaveLength(0)
editor.destroy()
})
@ -202,7 +176,7 @@ describe("DiffDecorations", () => {
editor.destroy()
})
it("highlights the whole task item when its checked state changed", ({
it("tints the checkbox of a task item whose checked state changed", ({
expect,
}) => {
const editor = makeEditor(
@ -219,8 +193,8 @@ describe("DiffDecorations", () => {
)
const li = editor.view.dom.querySelector("li")
expect(li?.classList.contains("diff-overlay-anchor")).toBe(true)
expect(li?.querySelector("span.diff-overlay.diff-modified")).not.toBeNull()
expect(li?.classList.contains("diff-checkbox-changed")).toBe(true)
expect(editor.view.dom.querySelector(".diff-overlay")).toBeNull()
editor.destroy()
})

45
web/app/components/editor/diff/diff-decorations.ts
View File

@ -13,19 +13,7 @@ import Italic from "@tiptap/extension-italic"
import Strike from "@tiptap/extension-strike"
import Underline from "@tiptap/extension-underline"
import { TaskItem } from "@tiptap/extension-list"
import {
DEFAULT_OPAQUE_TYPES,
OVERLAY_NODES,
SELF_DECORATED_TYPES,
} from "./config"
export interface DiffDecorationsOptions {
/**
* node types treated as opaque — when modified, the whole node gets
* a diff-modified class instead of character-level inline diffs.
*/
opaqueTypes: string[]
}
import { OVERLAY_NODES, SELF_DECORATED_TYPES } from "./config"
const pluginKey = new PluginKey("diffDecorations")
@ -40,18 +28,10 @@ export interface OverlayPadding {
left?: string
}
export const DiffDecorations = Extension.create<DiffDecorationsOptions>({
export const DiffDecorations = Extension.create({
name: "diffDecorations",
addOptions() {
return {
opaqueTypes: DEFAULT_OPAQUE_TYPES,
}
},
addProseMirrorPlugins() {
const opaqueTypes = new Set(this.options.opaqueTypes)
return [
new Plugin({
key: pluginKey,
@ -68,7 +48,7 @@ export const DiffDecorations = Extension.create<DiffDecorationsOptions>({
// always rebuild: comment transactions use setNodeMarkup
// which replaces nodes, causing DecorationSet.map() to
// drop decorations spanning the replaced range
return buildDiffDecorations(tr.doc, opaqueTypes)
return buildDiffDecorations(tr.doc)
},
},
@ -82,10 +62,7 @@ export const DiffDecorations = Extension.create<DiffDecorationsOptions>({
},
})
function buildDiffDecorations(
doc: PMNode,
opaqueTypes: Set<string>,
): DecorationSet {
function buildDiffDecorations(doc: PMNode): DecorationSet {
const decorations: Decoration[] = []
doc.descendants((node, pos) => {
@ -110,17 +87,17 @@ function buildDiffDecorations(
}
// status === DiffStatus.Modified
if (opaqueTypes.has(node.type.name)) {
pushDiffDecoration(decorations, node, pos, "diff-modified", false)
return false
}
// task items with a changed checked status get whole-item highlight
// a task item whose checkbox changed tints only the checkbox, then
// its content is diffed as usual
if (
node.type.name === TaskItem.name &&
hasTaskListItemCheckedChanged(node)
) {
pushDiffDecoration(decorations, node, pos, "diff-modified", true)
decorations.push(
Decoration.node(pos, pos + node.nodeSize, {
class: "diff-checkbox-changed",
}),
)
return true
}

6
web/app/components/editor/test-helpers/node-view.ts
View File

@ -27,8 +27,9 @@ export interface ChainedCall {
args: unknown[]
}
// only attrs, textContent, nodeSize and the type name are read by a node
// view rendering on its own, so a stand-in spares every suite a schema
// only attrs, textContent, nodeSize, the type name and a walk over no
// children are read by a node view rendering on its own, so a stand-in
// spares every suite a schema
export function makeNode(
attrs: Record<string, unknown> = {},
options: { typeName?: string; textContent?: string; nodeSize?: number } = {},
@ -38,6 +39,7 @@ export function makeNode(
textContent: options.textContent ?? "",
nodeSize: options.nodeSize ?? 2,
type: { name: options.typeName ?? "testNode" },
descendants: () => undefined,
} as unknown as PMNode
}

5
web/i18n/locales/en/editor.json
View File

@ -839,6 +839,11 @@
"decline": "Decline"
}
},
"diff-change-marker": {
"label": "{removed} removed, {added} added",
"removed": "−{count}",
"added": "+{count}"
},
"tags": {
"label": "Tags",
"trigger-title": "Manage Tags",