From 50f541fa646f1db7a3448833413f028231f8773f Mon Sep 17 00:00:00 2001 From: ntbang0901 <121003585+ntbang0901@users.noreply.github.com> Date: Tue, 10 Nov 2026 00:24:23 +0700 Subject: [PATCH] fix(vcs): centre the diff viewer's revert controls (#2346) The diff viewer's gutter arrows and the inline Revert button sat off-centre on their change blocks: they used fixed offsets that did not follow the editor's line height or the gutter width. They are now positioned from the editor's metrics and spacing tokens, centred independently of width. Covers modified, multi-line, deletion-only and insertion-only blocks, paired and merge accept controls, font sizes and zoom. Co-authored-by: ntbang0901 <121003585+ntbang0901@users.noreply.github.com> --- src/features/vcs/DiffEditor.module.css | 14 +++++-- src/features/vcs/__tests__/diffScroll.test.ts | 42 +++++++++++++++++++ src/features/vcs/diffScroll.ts | 2 + 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/features/vcs/DiffEditor.module.css b/src/features/vcs/DiffEditor.module.css index 041e32327..859baa65e 100644 --- a/src/features/vcs/DiffEditor.module.css +++ b/src/features/vcs/DiffEditor.module.css @@ -184,11 +184,12 @@ .host :global(.cm-diffHoverRevert) { position: absolute; right: 8px; - top: 0; + /* Centre on the first row, including when the editor's line height changes. */ + top: calc((var(--openidea-editor-line-height, var(--lh-code)) - var(--space-4)) / 2); display: inline-flex; align-items: center; gap: 3px; - height: 16px; + height: var(--space-4); padding: 0 6px; border: 1px solid var(--border-strong); border-radius: var(--radius-small); @@ -239,7 +240,8 @@ border-left: 1px solid var(--border); border-right: 1px solid var(--border); } -.host :global(.cm-diffDivider svg) { +/* Only the band layer: the chevrons inside the arrow buttons are centred by the button's flex box. */ +.host :global(.cm-diffDivider > svg) { position: absolute; inset: 0; } @@ -281,6 +283,12 @@ .host :global(.cm-diffDivider button.cm-diffArrow-b-to-a) { right: 1px; } +/* With one direction there is no pair to keep apart: the arrow sits in the middle of the divider. */ +.host :global(.cm-diffDivider-lone button.cm-diffArrow) { + left: 50%; + right: auto; + transform: translateX(-50%); +} .host :global(.cm-diffDivider button.cm-diffArrow:hover) { color: var(--fg-strong); background: transparent; diff --git a/src/features/vcs/__tests__/diffScroll.test.ts b/src/features/vcs/__tests__/diffScroll.test.ts index 30bf85a9a..0b6461b59 100644 --- a/src/features/vcs/__tests__/diffScroll.test.ts +++ b/src/features/vcs/__tests__/diffScroll.test.ts @@ -3,6 +3,8 @@ import { Chunk, MergeView } from "@codemirror/merge"; import { Text } from "@codemirror/state"; import type { EditorView } from "@codemirror/view"; import { bandAt, bandPath, transferY, UnalignedPanes, type Band } from "../diffScroll"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; const doc = (s: string) => Text.of(s.split("\n")); const LINE = 20; @@ -171,3 +173,43 @@ describe("animated synchronized scrolling", () => { } }); }); + +describe("divider arrows", () => { + const dividerOf = (arrows: { toA: boolean; toB: boolean } | undefined) => { + const root = document.createElement("div"); + const wrapper = root.appendChild(document.createElement("div")); + const dom = wrapper.appendChild(document.createElement("div")); + const view = { dom, scrollDOM: dom.appendChild(document.createElement("div")) } as unknown as EditorView; + const controller = new UnalignedPanes({ sync: false, arrows: arrows && { ...arrows, apply: () => {} } }); + controller.attach({ a: view, b: view, chunks: [] } as unknown as MergeView); + const divider = root.querySelector(".cm-diffDivider")!; + controller.destroy(); + return divider; + }; + + it.each([ + ["only the right pane is editable", { toA: false, toB: true }, true], + ["only the left pane is editable", { toA: true, toB: false }, true], + ["both panes are editable", { toA: true, toB: true }, false], + ["no pane is editable", { toA: false, toB: false }, false], + ["there are no arrows", undefined, false], + ])("centres a lone arrow only: %s", (_, arrows, lone) => { + expect(dividerOf(arrows).classList.contains("cm-diffDivider-lone")).toBe(lone); + }); +}); + +describe("diff controls in the stylesheet", () => { + // jsdom has no layout, so the selectors that decide how the controls are centred are read from the source. + const css = readFileSync(resolve(import.meta.dirname, "../DiffEditor.module.css"), "utf8"); + + it("lets only the band layer fill the divider, not the chevrons inside the arrow buttons", () => { + expect(css).toMatch(/\.cm-diffDivider > svg\)\s*\{[^}]*position: absolute;[^}]*inset: 0;/); + expect(css).not.toMatch(/\.cm-diffDivider svg\)/); + }); + + it("centres the hover Revert button on the line's own height", () => { + const rule = css.match(/\.cm-diffHoverRevert\)\s*\{([^}]*)\}/)![1]!; + expect(rule).toContain("top: calc((var(--openidea-editor-line-height, var(--lh-code)) - var(--space-4)) / 2);"); + expect(rule).not.toMatch(/top: 0;/); + }); +}); diff --git a/src/features/vcs/diffScroll.ts b/src/features/vcs/diffScroll.ts index 7afcf536b..c44fff902 100644 --- a/src/features/vcs/diffScroll.ts +++ b/src/features/vcs/diffScroll.ts @@ -175,6 +175,8 @@ export class UnalignedPanes { this.sync = opts.sync; this.extension = [gapMarks, changedLineGutters, EditorView.updateListener.of(() => this.schedule())]; this.divider.className = "cm-diffDivider"; + // A lone arrow is centred in the divider; a pair keeps to the two edges. + this.divider.classList.toggle("cm-diffDivider-lone", !!opts.arrows && opts.arrows.toA !== opts.arrows.toB); this.divider.setAttribute("data-wheel-scroll", ""); this.svg.setAttribute("aria-hidden", "true"); this.svg.appendChild(this.bandLayer);