Skip to content

Commit ad56e35

Browse files
authored
fix(webview): key the concealed fence gap offset on the gap border, not the language tag (#396)
* fix(webview): key the concealed fence gap offset on the gap border, not the language tag A concealed fenced block with no language tag drew its copy button 4px ABOVE the panel's painted top edge, hanging over the rounded corner. Its tagged twin sat 4px inside. The panel fill is `background-clip: padding-box`, so a visible open edge that carries blockEdgeGapCorner's transparent --quoll-block-gap-y top border paints its surface that far below its border box. In the revealed state the copy button's anchor IS that bordered line, so `top: 0.3em` resolves against the paint. In the concealed state the anchor is the zero-height `.quoll-fenced-code-fence-hidden` row, which sits at the border box's top and has no border of its own — the same `top` resolves against a datum 8px too high. The rule that adds the gap back existed, but it was gated on the NEXT line being a `-has-language` header, and its comment declared bare blocks deliberately untouched. The language tag was never what asked for the offset — the gap border is, and a bare block's panel carries exactly the same one. The gate had simply frozen an attribute that happened to correlate while the header bar (a tagged-only feature) was being built. Widening it also fixes a second miss on the same axis: block-style suppresses the fenced `-outer-open` inside a blockquote and lets `quoll-blockquote-outer-open` supply the identical border instead, so blockquote-nested blocks were uncorrected too — tagged ones included. `concealedGapAnchor` now spells both sources as a selector LIST, which cannot double-count: the two classes are mutually exclusive on a fenced open edge, and two matching branches still apply one declaration. Measured in the real-browser harness across bare/tagged x concealed/revealed x top-level/blockquote/list/bodyless, plus a directly-adjacent pair (whose second block correctly needs no offset because it yields its `-outer-open`). Every state now lands 4px inside the paint; the already-correct ones did not move. * docs(theme): clarify concealed-gap re-add rule count in comment The intro comment above fencedHeaderBarThemeSpec's "Conceal/reveal geometry" paragraph said "two :has(+ …) / descendant rules" re-add the alignment offsets, but the spec actually has three: two independent `:has(+ …)` selector lists from concealedGapAnchor (one per control — copy button, label) for the vertical gap-y offset, plus one descendant rule for the label's horizontal column inset. The undercount predates this PR; this PR only touched the paragraph while generalizing concealedGapAnchor to take a control argument, so resolve the pre-existing ambiguity while the comment is already being edited. Comment-only change, no selectors or behaviour touched. * refactor(test): drop a no-op cast and a dead field from the header-bar pins paintedPanelTop's `lines[i] as HTMLElement` restated a type the array already has, and the cases array declared an `expected?: number` field no case ever set and no assertion ever read — the band comes from the module-level EXPECTED_INSET / TOLERANCE. Both are cosmetic; no assertion, statement, or selector changed. * test(webview): drop leftover no-op cast in fenced-header-bar test insets[0] is already typed number (copyInsets returns number[], and noUncheckedIndexedAccess is not enabled in any tsconfig), so the 'as number' cast at line 208 was a no-op. Cycle 1 refactor commit b46e98e removed the same-shape cast at line 146 but missed this one. Consistency cleanup, not a correctness fix.
1 parent 8dd69a1 commit ad56e35

3 files changed

Lines changed: 195 additions & 26 deletions

File tree

src/webview/cm/theme.ts

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,13 +1008,55 @@ export const quollCopyButtonTheme = EditorView.theme(copyButtonThemeSpec);
10081008
// Conceal/reveal geometry: in the revealed state the controls share the open fence
10091009
// line's padding box with the bar → aligned. In the concealed state the controls
10101010
// sit in the zero-height `-fence-hidden` row while the bar rides the migrated
1011-
// first-body-line; that row lacks the line's transparent alignment borders, so two
1012-
// :has(+ …) / descendant rules re-add the gap-y (vertical, outer-open only) and the
1013-
// 6px column inset (horizontal) offsets so the fence-hidden controls land on the
1014-
// strip in both states (PR #246 adjacency precedent). Separate EditorView.theme for
1015-
// the same unlayered-override reason as copyButtonThemeSpec; exported plain so the
1016-
// picker suite pins the contract.
1017-
export const fencedHeaderBarThemeSpec = {
1011+
// first-body-line; that row lacks the line's transparent alignment borders, so
1012+
// three re-add rules restore it: two `:has(+ …)` selector lists — one per control
1013+
// (copy button, label) — for the vertical gap-y offset (see concealedGapAnchor),
1014+
// plus one descendant rule for the label's 6px horizontal column inset. Together
1015+
// they land the fence-hidden controls on the strip in both states (PR #246
1016+
// adjacency precedent). Separate EditorView.theme for the same unlayered-override
1017+
// reason as copyButtonThemeSpec; exported plain so the picker suite pins the
1018+
// contract.
1019+
1020+
/** Selector for a CONCEALED open fence's header controls that must be pushed back
1021+
* down by the panel's EXTERNAL --quoll-block-gap-y top border.
1022+
*
1023+
* Why the offset exists: the panel fill uses `background-clip: padding-box`, so a
1024+
* visible open edge carrying blockEdgeGapCorner's transparent top border paints its
1025+
* surface --quoll-block-gap-y BELOW its border box. The concealed anchor is the
1026+
* zero-height `-fence-hidden` row, which sits at that border box's TOP and has no
1027+
* such border of its own — so an absolute `top` on it resolves against a datum
1028+
* --quoll-block-gap-y too high, and the control hangs out over the panel's rounded
1029+
* top edge. Adding the gap back restores the same inset the revealed state gets for
1030+
* free (its anchor IS the bordered line).
1031+
*
1032+
* Why the condition is the GAP BORDER, not the language tag: the border is emitted
1033+
* by whichever `-outer-open` class marks the panel's true outer boundary, and both
1034+
* spellings resolve the SAME --quoll-block-gap-y token via blockEdgeGapCorner — a
1035+
* top-level fenced panel gets `quoll-fenced-code-outer-open`, a blockquote-nested one
1036+
* gets `quoll-blockquote-outer-open` instead (block-style.ts suppresses fenced
1037+
* outer-open inside a quote). The language tag has nothing to do with it, so gating
1038+
* on `-has-language` left every BARE block — and every blockquote-nested block, tagged
1039+
* or not — uncorrected. Real-pixel evidence for all of these states:
1040+
* test/webview-browser/fenced-header-bar.browser.test.ts.
1041+
*
1042+
* A selector LIST (not a compound) is what keeps this safe from double-counting: the
1043+
* two classes are mutually exclusive on a fenced open edge, and even if they were not,
1044+
* matching two branches applies the one declaration once. Requiring
1045+
* `.quoll-fenced-code-open` on the adjacent line keeps the match to a fenced panel's
1046+
* own migrated header line rather than any neighbouring outer-open block. */
1047+
const concealedGapAnchor = (control: string): string =>
1048+
[".quoll-fenced-code-outer-open", ".quoll-blockquote-outer-open"]
1049+
.map(
1050+
(outerOpen) =>
1051+
`.cm-line.quoll-fenced-code-fence-hidden:has(+ .cm-line.quoll-fenced-code-open${outerOpen}) ${control}`
1052+
)
1053+
.join(", ");
1054+
1055+
// Explicitly typed (the sibling specs infer): the two computed template-literal
1056+
// keys below make TS infer a UNION of per-key value shapes, each padding the other
1057+
// keys' properties with `?: undefined`, which no longer satisfies StyleSpec's index
1058+
// signature. The annotation states the shape the spec has always had.
1059+
export const fencedHeaderBarThemeSpec: Record<string, Record<string, string>> = {
10181060
".cm-line.quoll-fenced-code-open.quoll-fenced-code-has-language": {
10191061
// Self-sufficient positioning context (do NOT rely on copyButtonThemeSpec's).
10201062
position: "relative",
@@ -1127,24 +1169,20 @@ export const fencedHeaderBarThemeSpec = {
11271169
},
11281170
// Concealed-state alignment. The `-fence-hidden` row lacks the line's transparent
11291171
// alignment borders, so absolute offsets resolve to the row's raw left / top edge.
1130-
// - VERTICAL: only when the migrated header line is a TRUE outer boundary (its
1131-
// transparent --quoll-block-gap-y top border pushes the bar's padding box
1132-
// down) — offset both controls down by the gap. Scoped by the NEXT line being
1133-
// a has-language outer-open header, so bare blocks are untouched.
1172+
// - VERTICAL: {@link concealedGapAnchor} — the offset follows the GAP BORDER, not
1173+
// the language tag (a bare block's panel carries the same border).
11341174
// - HORIZONTAL: the revealed line's 6px transparent left border insets the label;
11351175
// a plain (non-blockquote) fence-hidden row has none, so re-add the column
11361176
// inset. A BLOCKQUOTE-nested fence-hidden row ALREADY carries the blockquote's
11371177
// 6px transparent border (same --quoll-column-inset-left token), so it must NOT
11381178
// re-add it (double-count → the nested label drifts ~6px right) — hence
11391179
// :not(.quoll-blockquote).
1140-
".cm-line.quoll-fenced-code-fence-hidden:has(+ .cm-line.quoll-fenced-code-has-language.quoll-fenced-code-outer-open) .quoll-copy-button":
1141-
{
1142-
top: "calc(0.3em + var(--quoll-block-gap-y, 8px))",
1143-
},
1144-
".cm-line.quoll-fenced-code-fence-hidden:has(+ .cm-line.quoll-fenced-code-has-language.quoll-fenced-code-outer-open) .quoll-language-picker-label.is-labeled":
1145-
{
1146-
top: "var(--quoll-block-gap-y, 8px)",
1147-
},
1180+
[concealedGapAnchor(".quoll-copy-button")]: {
1181+
top: "calc(0.3em + var(--quoll-block-gap-y, 8px))",
1182+
},
1183+
[concealedGapAnchor(".quoll-language-picker-label.is-labeled")]: {
1184+
top: "var(--quoll-block-gap-y, 8px)",
1185+
},
11481186
".cm-line.quoll-fenced-code-fence-hidden:not(.quoll-blockquote) .quoll-language-picker-label.is-labeled":
11491187
{
11501188
left: "calc(0.55em + var(--quoll-column-inset-left, 6px))",

test/webview-browser/fenced-header-bar.browser.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@
88
// styles.css is not loaded, so every --quoll-* token resolves to its in-spec
99
// fallback (header height 2.1em, gap-y 8px, column inset 6px), which is exactly the
1010
// production geometry we want to pin.
11+
//
12+
// The second suite below pins the VERTICAL half of that correction across every
13+
// nesting/conceal combination. It is the only place the bug it guards can be seen:
14+
// the panel's painted top edge is its padding-box top (background-clip: padding-box
15+
// over a transparent --quoll-block-gap-y border), and only a layout engine reports
16+
// where that lands. Before concealedGapAnchor generalised the offset off the language
17+
// tag, a bare concealed block put the copy button 4px ABOVE the paint (over the
18+
// rounded corner) while its tagged twin sat 4px inside.
1119
import { markdown, markdownLanguage } from "@codemirror/lang-markdown";
1220
import { EditorSelection, EditorState } from "@codemirror/state";
1321
import { EditorView } from "@codemirror/view";
@@ -121,3 +129,101 @@ describe("fenced-code header bar — real-pixel layout", () => {
121129
expect(shown(label(view)), "label hidden while editing").toBe(false);
122130
});
123131
});
132+
133+
// ---------------------------------------------------------------------------
134+
// Copy-button vertical inset vs the panel's PAINTED top edge.
135+
// ---------------------------------------------------------------------------
136+
137+
/** The painted top edge of the panel the control belongs to: the padding-box top of
138+
* the panel's visible open-edge line. The fill is `background-clip: padding-box`, so
139+
* a transparent --quoll-block-gap-y top border is NOT painted — the surface starts
140+
* that much below the border box. Walking forward from the control's own anchor line
141+
* to the first `-open` line pairs each control with ITS panel (adjacent blocks). */
142+
function paintedPanelTop(view: EditorView, control: HTMLElement): number {
143+
const lines = [...view.contentDOM.querySelectorAll<HTMLElement>(".cm-line")];
144+
const anchor = control.closest(".cm-line") as HTMLElement | null;
145+
for (let i = anchor === null ? 0 : lines.indexOf(anchor); i < lines.length; i++) {
146+
const line = lines[i];
147+
if (line.classList.contains("quoll-fenced-code-open")) {
148+
return (
149+
line.getBoundingClientRect().top + Number.parseFloat(getComputedStyle(line).borderTopWidth)
150+
);
151+
}
152+
}
153+
throw new Error("no visible fenced open edge for this control");
154+
}
155+
156+
/** How far the copy button's top sits INSIDE the painted panel top. Positive = inside
157+
* (correct); negative = hanging over the panel's rounded top edge (the bug). */
158+
function copyInsets(view: EditorView): number[] {
159+
return [...view.contentDOM.querySelectorAll<HTMLElement>(".quoll-copy-button")].map(
160+
(cp) => cp.getBoundingClientRect().top - paintedPanelTop(view, cp)
161+
);
162+
}
163+
164+
// `top: 0.3em` at the fence-hidden row's 0.9em-of-16px font ≈ 4px. Pinning the band
165+
// (not the float) keeps the test about the CONTRACT — the button is inset by the
166+
// declared 0.3em, not the gap-sized 8px-off datum a missing correction produces
167+
// (-4px) nor a doubled one (+12px) — while tolerating sub-pixel font rounding.
168+
const EXPECTED_INSET = 4;
169+
const TOLERANCE = 1.5;
170+
171+
describe("copy button sits INSIDE the painted panel top in every concealed nesting", () => {
172+
// Each case is a doc whose caret parks OUTSIDE every fenced block (reading mode →
173+
// concealed fences), except where the name says otherwise. The bug this pins was
174+
// concealed-only: revealed anchors are the bordered open line itself.
175+
const cases: Array<{ name: string; doc: string; caret?: number }> = [
176+
{ name: "bare block, concealed (the reported bug)", doc: "```\nconst a = 1;\n```\n\npara" },
177+
{ name: "language-tagged block, concealed", doc: "```js\nconst a = 1;\n```\n\npara" },
178+
{ name: "bare block, revealed", doc: "```\nconst a = 1;\n```\n\npara", caret: 2 },
179+
{ name: "language-tagged block, revealed", doc: "```js\nconst a = 1;\n```\n\npara", caret: 2 },
180+
// Blockquote-nested: block-style suppresses the FENCED outer-open inside a quote
181+
// and the QUOTE's outer-open supplies the identical gap border instead — the
182+
// second source concealedGapAnchor has to cover.
183+
{
184+
name: "blockquote-nested bare block, concealed",
185+
doc: "> ```\n> const a = 1;\n> ```\n\npara",
186+
},
187+
{
188+
name: "blockquote-nested tagged block, concealed",
189+
doc: "> ```js\n> const a = 1;\n> ```\n\npara",
190+
},
191+
{
192+
name: "list-nested bare block, concealed",
193+
doc: "- item\n\n ```\n const a = 1;\n ```\n\npara",
194+
},
195+
// Bodyless: no body line for the edges to migrate onto, so the open fence line
196+
// keeps the panel itself — already-correct states that must not gain a second
197+
// offset from the widened selector.
198+
{ name: "bodyless bare block", doc: "```\n```\n\npara" },
199+
{ name: "bodyless tagged block (header-only bar)", doc: "```js\n```\n\npara" },
200+
];
201+
202+
for (const c of cases) {
203+
it(c.name, async () => {
204+
view = mount(c.doc, c.caret ?? c.doc.indexOf("para") + 1);
205+
await settled();
206+
const insets = copyInsets(view);
207+
expect(insets, "one copy button per fenced block").toHaveLength(1);
208+
const inset = insets[0];
209+
expect(inset, "button top is below the painted panel top").toBeGreaterThan(0);
210+
expect(Math.abs(inset - EXPECTED_INSET), `inset ${inset}px`).toBeLessThanOrEqual(TOLERANCE);
211+
});
212+
}
213+
214+
// Two directly-adjacent blocks collapse to ONE gap: the SECOND block yields its
215+
// -outer-open, so its open edge has NO gap border and needs NO correction. Pins the
216+
// adjacency gate itself — a correction applied unconditionally would push this one
217+
// 8px too low.
218+
it("directly-adjacent pair: both buttons land at the same inset", async () => {
219+
const doc = "```\na\n```\n```\nb\n```\n\npara";
220+
view = mount(doc, doc.indexOf("para") + 1);
221+
await settled();
222+
const insets = copyInsets(view);
223+
expect(insets, "one copy button per block").toHaveLength(2);
224+
for (const inset of insets) {
225+
expect(inset).toBeGreaterThan(0);
226+
expect(Math.abs(inset - EXPECTED_INSET), `inset ${inset}px`).toBeLessThanOrEqual(TOLERANCE);
227+
}
228+
});
229+
});

test/webview/fenced-code/cm-fenced-code-language-picker.test.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -540,13 +540,38 @@ describe("quollFencedHeaderBarTheme (spec contract)", () => {
540540
// Explicit after `font: inherit` so the concealed row's line-height:0 can't clip.
541541
expect(sel?.lineHeight).toBe("normal");
542542
});
543-
it("offsets fence-hidden controls by the gap when the next line is an outer-open header", () => {
544-
const key = Object.keys(fencedHeaderBarThemeSpec).find(
545-
(k) => k.includes(":has(") && k.includes("quoll-copy-button")
546-
);
547-
expect(key).toBeDefined();
548-
expect(rule(key as string)?.top).toContain("--quoll-block-gap-y");
549-
});
543+
// The gap offset keys the correction on the GAP BORDER, not on the language tag:
544+
// both `-outer-open` spellings emit the same --quoll-block-gap-y border (a
545+
// top-level panel gets the fenced one, a blockquote-nested panel the quote's), and
546+
// a bare block's panel carries it just as a tagged one does. Gating on
547+
// `-has-language` left every bare and every blockquote-nested block uncorrected —
548+
// the button then hung 4px over the panel's rounded top edge. The real-pixel
549+
// evidence lives in test/webview-browser/fenced-header-bar.browser.test.ts; this
550+
// pins the selector shape so the fix cannot be silently re-narrowed here.
551+
for (const [control, prop] of [
552+
[".quoll-copy-button", "top"],
553+
[".quoll-language-picker-label.is-labeled", "top"],
554+
] as const) {
555+
it(`offsets the fence-hidden ${control} by the gap on BOTH outer-open sources`, () => {
556+
const key = Object.keys(fencedHeaderBarThemeSpec).find(
557+
(k) => k.includes(":has(") && k.endsWith(` ${control}`)
558+
);
559+
expect(key).toBeDefined();
560+
const selector = key as string;
561+
expect(rule(selector)?.[prop]).toContain("--quoll-block-gap-y");
562+
// One branch per gap-border source, and neither gated on the language tag.
563+
const branches = selector.split(", ");
564+
expect(branches).toHaveLength(2);
565+
expect(branches.some((b) => b.includes(".quoll-fenced-code-outer-open"))).toBe(true);
566+
expect(branches.some((b) => b.includes(".quoll-blockquote-outer-open"))).toBe(true);
567+
for (const branch of branches) {
568+
expect(branch).not.toContain("quoll-fenced-code-has-language");
569+
// Anchored on the fence-hidden row, matched against its ADJACENT open edge.
570+
expect(branch.startsWith(".cm-line.quoll-fenced-code-fence-hidden:has(+ ")).toBe(true);
571+
expect(branch).toContain(".cm-line.quoll-fenced-code-open");
572+
}
573+
});
574+
}
550575
it("re-adds the column inset to a NON-blockquote fence-hidden labelled wrapper only", () => {
551576
// The :not(.quoll-blockquote) scope prevents double-counting the inset on a
552577
// blockquote-nested fence-hidden row (which already carries the quote's border).

0 commit comments

Comments
 (0)