oficina / sheets Public

fix: the selection had a fill and no outline in dark

Reported from a canary: a range in dark mode is a navy slab with one
bright white line down its left side and no edge anywhere else.

Every colour in the vendored Jspreadsheet stylesheet is a literal, and
sheets.css restated the ones for the grid, the headers and the range
fill. It stopped there. The outline is where the rest of them live:

  .highlight-right/-bottom   border: 1px solid #000
  .highlight-top/-left       the same, plus box-shadow: -1px 0 #ccc

In dark that renders backwards. The black borders vanish into a #111418
grid. The two that would not are overwritten by our own
`td { border-top-color: var(--rst-line) }`, which at (0,2,3) outranks
the library's (0,2,0) `.jss_worksheet .highlight-top` — so they resolve
to ordinary grid lines. What survives is the #ccc box-shadow, a shim
that exists only to mask the grid line the border sits on, and it
becomes the only visible edge. Hence the white L.

Three more of the same shape, found while measuring the first:

  - The selected row's header was #dcdcdc — a light slab in a dark grid
    carrying --rst-text-muted at 1.9:1. sheets.css DID have an override
    for it, `tbody > tr > td:first-child.selected`, but the library puts
    .selected on the <tr>, not on its first cell, so the rule matched
    nothing and looked like a fix. A CSS rule that matches nothing fails
    exactly like a rule nobody wrote.
  - The fill handle is a black knob ringed in white: inverted in dark.
  - The copy marquee and the range a formula points at are dashed and
    dotted #000, both invisible.

The fix is one outline colour, --rst-accent, on all four sides of all
three range states, scoped `> tbody > tr > td.foo` so it clears the grid
line, and the #ccc shadow dropped — with an outline that reads, masking
the line beneath it buys nothing and costs the selection its symmetry.
The library gives each state a different border STYLE (solid, dashed,
dotted) and the style survives a colour-only override, so that is what
tells them apart. --rst-accent on --rst-accent-soft measures 6.59:1 in
dark and 4.75:1 in light, both past 1.4.11's 3:1 for a component
boundary.

One thing here is not dark-specific. .highlight-selected — the anchor
cell the keyboard is on — is transparent in the library so it reads out
of the fill, and our opaque .highlight rule took that back. Restored: a
range now shows its cursor in both schemes.

TestVendorSelectionChromeIsRestated is the gate. It checks both
directions: that each vendored selector still exists — v5 renamed every
.jexcel to .jss_* once already, and a stale list is how that goes
unnoticed a second time — and that sheets.css restates it. Rules the app
cannot render (.readonly, .jss_freezed, pagination, the editors we do not
enable) are named in the test with the reason, not silently omitted. A
second test pins the specificity, which is the half of the bug that is
invisible in a diff.

Verified in Chromium at 1200x620 in both schemes, on the real binary:
outline #7ba7ff dark / #2464e0 light on all four sides, box-shadow none,
row header on --rst-accent-soft with --rst-text, handle on the accent,
marquee dashed and visible. make test green including vet, fmt,
migrations and race.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UiPVt2xfkKStDgkv4qLCm1
Paul Campbell pushed by paul@keymail.dev 4a259e083d9a5a79f1bcd45195a63bc639292684 parent 01e8adf
3 files changed, +243 −1
  • AGENTS.md +9 −0
  • internal/guardstest/vendorpaint_test.go +146 −0
  • internal/sheets/static/sheets.css +88 −1

CI passed — run details

Intent

Attributed to this commit: 74.8 g CO₂e (7575461 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on worktree-dark-selection, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.

diff --git a/AGENTS.md b/AGENTS.md
index 0b48e98..306ee56 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -142,6 +142,15 @@ relying on remembering one, make it a test instead.
**Frontend**
- No build step. Jspreadsheet CE is vendored as a static asset, not fetched
from a CDN and not bundled.
+- **The vendored stylesheet's colours are literals, and every one the app
+ can render must be restated in `sheets.css`.** Enforced by
+ `TestVendorSelectionChromeIsRestated`, which also checks the other
+ direction — that each vendored selector still exists, since v5 renamed
+ every `.jexcel` to `.jss_*` once already. Restate at
+ `> tbody > tr > td.foo`, not at the library's own
+ `.jss_worksheet .foo`: our grid-line rule is one step more specific, so
+ a shorter selector is a rule that parses, uses a token, and paints
+ nothing.
- **Write the attribute grammar: `rst-btn="primary compact"`, not
`class="rst-btn rst-btn--primary"`.** This reverses the earlier rule in
this file, which was written against v0.20.0 where the attribute
diff --git a/internal/guardstest/vendorpaint_test.go b/internal/guardstest/vendorpaint_test.go
new file mode 100644
index 0000000..8423e9d
--- /dev/null
+++ b/internal/guardstest/vendorpaint_test.go
@@ -0,0 +1,146 @@
+package guards
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// The vendored Jspreadsheet stylesheet, and ours.
+const (
+ vendorSheet = "internal/sheets/static/vendor/jspreadsheet.css"
+ ourSheet = "internal/sheets/static/sheets.css"
+)
+
+// selectionChrome is every vendored rule that paints the SELECTION in a
+// hard-coded colour and that this app can actually render.
+//
+// sheets.css restated the vendored greys for the grid, the headers and
+// the range fill, and stopped there. It did not restate the four
+// highlight-* borders, the four copying-* borders, the four selection-*
+// borders, .jss_corner, or the selected row's header — so in dark mode
+// the range fill was drawn and its outline was not:
+//
+// - highlight-right/-bottom border 1px solid #000 → invisible on #111418.
+// - highlight-top/-left their border is overwritten by our own
+// `td { border-top-color: var(--rst-line) }`, one specificity step
+// higher, so the outline became an ordinary grid line — while their
+// decorative `box-shadow: -1px 0 #ccc` survived and became the one
+// bright edge on screen. A selection lit down its left side and
+// nowhere else.
+// - tr.selected > td:first-child #dcdcdc → a light slab in a dark
+// grid, carrying --rst-text-muted at 1.9:1.
+// - .jss_corner a black knob ringed in white → reads inverted.
+//
+// The row header is the instructive one. sheets.css DID write an
+// override for it — `tbody > tr > td:first-child.selected` — but
+// Jspreadsheet puts `.selected` on the <tr>, not on the <td>, so the
+// rule matched nothing and looked like a fix. A CSS rule that matches
+// nothing fails exactly like a rule nobody wrote.
+//
+// Each entry is checked in BOTH directions: the vendored selector must
+// still exist (v5 renamed every `.jexcel` to `.jss_*`, and a stale list
+// like this one is how that rename goes unnoticed a second time), and
+// ours must restate it.
+//
+// Vendored rules with hard-coded colours that are deliberately NOT here,
+// because this app never renders them — if one starts rendering it
+// belongs in the list:
+//
+// .readonly no column sets readOnly and nothing calls
+// setReadOnly; grid.js contains neither word.
+// .jss_freezed grid.js passes no freezeColumns or freezeRows.
+// .jss_pagination a sheet is one page; pagination is off.
+// .jupload, editors we do not enable — the cell editor is the
+// .jss_richtext plain text one.
+var selectionChrome = []struct {
+ vendor string // must still exist in the vendored stylesheet
+ restated string // sheets.css must carry this selector fragment
+}{
+ {".jss_worksheet > tbody > tr.selected > td:first-child", "tr.selected > td:first-child"},
+
+ {".jss_worksheet .highlight-top", ".highlight-top"},
+ {".jss_worksheet .highlight-left", ".highlight-left"},
+ {".jss_worksheet .highlight-right", ".highlight-right"},
+ {".jss_worksheet .highlight-bottom", ".highlight-bottom"},
+ {".jss_worksheet .highlight-selected", ".highlight-selected"},
+
+ {".jss_worksheet .selection-top", ".selection-top"},
+ {".jss_worksheet .selection-bottom", ".selection-bottom"},
+ {".jss_worksheet .selection-left", ".selection-left"},
+ {".jss_worksheet .selection-right", ".selection-right"},
+
+ {".jss_worksheet .copying-top", ".copying-top"},
+ {".jss_worksheet .copying-bottom", ".copying-bottom"},
+ {".jss_worksheet .copying-left", ".copying-left"},
+ {".jss_worksheet .copying-right", ".copying-right"},
+
+ {".jss_corner", ".jss_corner"},
+}
+
+func TestVendorSelectionChromeIsRestated(t *testing.T) {
+ root := repoRoot(t)
+
+ vendor, err := os.ReadFile(filepath.Join(root, vendorSheet))
+ if err != nil {
+ t.Fatalf("guards: read %s: %v", vendorSheet, err)
+ }
+ ours, err := os.ReadFile(filepath.Join(root, ourSheet))
+ if err != nil {
+ t.Fatalf("guards: read %s: %v", ourSheet, err)
+ }
+
+ for _, c := range selectionChrome {
+ if !strings.Contains(string(vendor), c.vendor) {
+ t.Errorf("%s: no rule for %q — the vendored library renamed or dropped it "+
+ "(v5 renamed every .jexcel to .jss_*). Re-derive this list from the "+
+ "vendored stylesheet; deleting the entry hides the rule, it does not fix it",
+ vendorSheet, c.vendor)
+ continue
+ }
+ if !strings.Contains(string(ours), c.restated) {
+ t.Errorf("%s: %s is painted in a hard-coded colour by %s and is not restated here — "+
+ "a literal does not follow the person's light/dark choice, so the selection "+
+ "loses its outline in one of the two schemes",
+ ourSheet, c.restated, vendorSheet)
+ }
+ }
+}
+
+// outlineCollidesWithGridLine are the selection-outline classes that set
+// the same two properties as our own grid line, and so must be written
+// at the same specificity or higher.
+//
+// This is the half of the bug that is invisible in a diff. sheets.css
+// sets border-top-color and border-left-color on every body cell at
+// (0,2,3); the vendored `.jss_worksheet .highlight-top` is (0,2,0). A
+// restatement written at the vendor's specificity loses to our own grid
+// line and paints nothing: the rule is there, the colour is a token, and
+// the outline still does not appear. Scoping it through
+// `> tbody > tr > td` the way the grid-line rule is scoped puts it back
+// on top.
+var outlineCollidesWithGridLine = []string{
+ "> tbody > tr > td.highlight-top",
+ "> tbody > tr > td.highlight-left",
+ "> tbody > tr > td.selection-top",
+ "> tbody > tr > td.selection-left",
+ "> tbody > tr > td.copying-top",
+ "> tbody > tr > td.copying-left",
+}
+
+func TestSelectionOutlineOutranksTheGridLine(t *testing.T) {
+ root := repoRoot(t)
+ body, err := os.ReadFile(filepath.Join(root, ourSheet))
+ if err != nil {
+ t.Fatalf("guards: read %s: %v", ourSheet, err)
+ }
+ for _, sel := range outlineCollidesWithGridLine {
+ if !strings.Contains(string(body), sel) {
+ t.Errorf("%s: expected a rule scoped `%s`. Written any shorter it is less "+
+ "specific than our own `td { border-top-color: var(--rst-line) }`, and the "+
+ "selection's top and left edges resolve to the grid line instead of to the "+
+ "outline colour", ourSheet, sel)
+ }
+ }
+}
diff --git a/internal/sheets/static/sheets.css b/internal/sheets/static/sheets.css
index c2dc2d4..a451d27 100644
--- a/internal/sheets/static/sheets.css
+++ b/internal/sheets/static/sheets.css
@@ -128,17 +128,104 @@
font-weight: 600;
}
+/* The names of the selected column and row. The library puts .selected
+ * on the thead td but on the tbody TR — not on its first cell — so the
+ * two halves need different selectors. Written as `td:first-child.selected`
+ * the second half matched nothing at all, and a rule that matches nothing
+ * fails exactly like a rule nobody wrote: the vendored #dcdcdc stood, and
+ * a dark grid grew a light-grey slab down its left edge carrying muted
+ * grey text at 1.9:1. */
[data-sheet-grid] .jss_worksheet > thead > tr > td.selected,
-[data-sheet-grid] .jss_worksheet > tbody > tr > td:first-child.selected {
+[data-sheet-grid] .jss_worksheet > tbody > tr.selected > td:first-child {
background-color: var(--rst-accent-soft);
color: var(--rst-text);
}
+/* ── The selection ───────────────────────────────────────────────────
+ *
+ * A range is a fill plus an outline. This file used to restate only the
+ * fill, and the outline is where every hard-coded colour in the vendored
+ * stylesheet lives:
+ *
+ * .highlight-right/-bottom border: 1px solid #000
+ * .highlight-top/-left border: 1px solid #000, plus a decorative
+ * box-shadow of #ccc just outside it
+ *
+ * In dark that renders backwards. The black borders disappear into the
+ * grid; the two that do not are overwritten by our own
+ * `td { border-top-color: var(--rst-line) }` above, which is one
+ * specificity step higher than the library's `.jss_worksheet .highlight-top`
+ * — so they become ordinary grid lines, while the #ccc shadow, which
+ * exists only to mask the grid line the border sits on, survives as the
+ * one bright thing on screen. What you get is a navy slab lit down its
+ * top and left and nowhere else, with no way to see where it ends.
+ *
+ * So: one outline colour, --rst-accent, on all four sides, at a
+ * specificity that clears the grid line (6.59:1 on the fill in dark,
+ * 4.75:1 in light — both past 1.4.11's 3:1 for a component boundary).
+ * The shadow goes: with the outline in a colour that reads, masking the
+ * line underneath it buys nothing and costs the selection its symmetry.
+ *
+ * The library gives each of its three range states a different border
+ * STYLE — solid for the selection, dashed for the copy marquee, dotted
+ * for the range a formula is pointing at — and the style survives our
+ * colour-only override. That is the distinction doing the work, so all
+ * three take the same accent and stay told apart by shape. */
[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight {
background-color: var(--rst-accent-soft);
color: var(--rst-text);
}
+/* The anchor — the one cell the keyboard is on. The library paints it
+ * transparent on purpose so it reads out of the fill; our .highlight
+ * rule was opaque and took that back, leaving a range with no visible
+ * cursor in either scheme. */
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight-selected {
+ background-color: var(--rst-bg);
+}
+
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight-top,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.selection-top,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.copying-top {
+ border-top-color: var(--rst-accent);
+ box-shadow: none;
+}
+
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight-left,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.selection-left,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.copying-left {
+ border-left-color: var(--rst-accent);
+ box-shadow: none;
+}
+
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight-right,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.selection-right,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.copying-right {
+ border-right-color: var(--rst-accent);
+}
+
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight-bottom,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.selection-bottom,
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.copying-bottom {
+ border-bottom-color: var(--rst-accent);
+}
+
+/* The range a formula is pointing at while you edit it. Vendored as
+ * rgba(0,0,0,0.05) — a darkening, which on a dark grid is nothing. */
+[data-sheet-grid] .jss_worksheet > tbody > tr > td.selection {
+ background-color: var(--rst-accent-soft);
+ color: var(--rst-text);
+}
+
+/* The fill handle at the corner of the range: a black knob ringed in
+ * white, which in dark reads as a white dot with a dark centre — the
+ * inverse of the thing it is meant to be. It is 1px plus its ring, so
+ * it is the accent or it is invisible. */
+[data-sheet-grid] .jss_corner {
+ background-color: var(--rst-accent);
+ border-color: var(--rst-bg);
+}
+
/* The cell editor is an input the library drops into the td. Left
* alone it inherits the browser's own white. */
[data-sheet-grid] .jss_worksheet td input,