oficina / sheets Public

fix: the review's blocker, and eleven findings under it

§Review by a model that did not write the code returned REQUEST
CHANGES. Discussion 10 carries the verdict; this is the response.

**The blocker: alignment did nothing.** Jspreadsheet writes text-align
as an INLINE style on every data cell, from the column's align or from
defaultColAlign. An inline declaration outranks any selector at any
specificity, so the three toolbar buttons and the Format submenu set
their attribute, reflected aria-pressed, and moved no text. The control
looked like it worked.

The branch's own browser drive did not catch it because it checked bold
and fill — both of which paint properties the library never touches.
That is the third instance tonight of one shape: a probe returning the
same answer whether or not the thing works. The drive now asserts the
COMPUTED alignment, because the attribute was always right.

Two things fell out of fixing it:

- The grid centred everything, while formatpatch.go's comment claimed
  the engine right-aligned numbers and left-aligned text — true of the
  table view and false here. The server now says which cells hold a
  NUMBER (a property of the computed value, which only the engine
  knows) and the stylesheet right-aligns those. A spreadsheet that
  centres its numbers does not look like one.
- defaultColAlign is a WORKSHEET option and was set at spreadsheet
  level, where it is accepted and never read — the exact mirror of the
  handlers, which are spreadsheet-level and ignored inside a worksheet.
  Found by the drive, not by reading.

Also fixed:

- The anchor cursor vanished on filled cells. The rule that marks it
  carries a comment about coming last so the cursor survives; the new
  fill rules land later at the same specificity, so the failure it
  guards against arrived by a new route. It is now an inset ring, which
  is drawn over any fill and costs no layout.
- Delete wrote every selected cell twice. The library already clears a
  selection through its own setValue, which reaches write(); ours was a
  second pass, and Backspace — which the library also handles — went
  once. Inconsistent either way.
- The number format was 128 bytes of free text stored verbatim. An
  Excel pattern admits quoted literals, so that was an arbitrary
  plaintext write into the formats table through a public API. It now
  validates against the five patterns the UI offers, and the UI renders
  from the same list, so the two cannot drift.
- Resize is drag-only and AGENTS.md requires a menu equivalent. The
  Format menu now carries column-width and row-height fields.
- The toolbar's tooltips never rendered: rastrillo's is
  [rst-tip]::after { content: attr(data-tip) }, and only data-tip was
  written.
- A plain viewer got an empty Insert menu; the list row lost its
  thumbnail column below 800px, where rastrillo sets
  grid-template-columns directly rather than through --rst-cols; a
  toolbar dropdown near the right edge overflowed a 320px viewport; a
  colour picked from a toolbar menu dropped focus to <body>.
- Export now sends RFC 6266's two-parameter Content-Disposition. The
  ASCII form drops every non-ASCII rune, so a suite called Oficina
  would have mangled the first Spanish title anybody exported.
- schema.sql was not regenerated for migration 0008.
- The colour-literal guard did not know about oklch(), which this
  branch introduced — a literal oklch would have passed the test whose
  whole job is to catch one.
- Nothing exercised writing an ink: migration 0008's columns and
  applyInk were reachable only by clearing.
- Deterministic export ordering, dead code removed, and three comments
  that had stopped being true.

Two of the review's findings are accepted and NOT fixed here, both
recorded in discussion 10: the read-modify-write in applyFormat is not
under the handle lock, so two concurrent patches to one cell can lose
each other's fields; and a max-size patch is ~12,000 statements in
autocommit. Both want a store change to the locking, which is the most
delicate code in the repo and not a thing to do at the end of a long
session.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0182f7QfojuuqdS4Xd2fYFaL
Paul Campbell pushed by paul@keymail.dev b6853b61bc20821cd335d571da631b9157851d69 parent ce97906
21 files changed, +542 −155
  • internal/guardstest/vendorpaint_test.go +8 −1
  • internal/sheets/formatpatch.go +4 −9
  • internal/sheets/grid.go +18 −0
  • internal/sheets/handlers_export.go +57 −2
  • internal/sheets/handlers_export_test.go +43 −0
  • internal/sheets/handlers_format.go +4 −4
  • internal/sheets/handlers_format_test.go +37 −2
  • internal/sheets/handlers_sheets.go +5 −0
  • internal/sheets/migrations/schema.sql +1 −1
  • internal/sheets/palette.go +47 −10
  • internal/sheets/preview.go +6 −5
  • internal/sheets/preview_test.go +1 −2
  • internal/sheets/sheetsvc.go +7 −4
  • internal/sheets/static/commands.js +34 −7
  • internal/sheets/static/grid.js +34 −34
  • internal/sheets/static/paint.js +46 −3
  • internal/sheets/static/sheets.css +81 −9
  • internal/sheets/store/handle_formats.go +0 −34
  • internal/sheets/templates/sheet.html +47 −24
  • internal/sheets/xlsx/sizes.go +29 −4
  • tools/drive/chrome.mjs +33 −0

CI passed — run details

diff --git a/internal/guardstest/vendorpaint_test.go b/internal/guardstest/vendorpaint_test.go
index 47703d9..4be8eef 100644
--- a/internal/guardstest/vendorpaint_test.go
+++ b/internal/guardstest/vendorpaint_test.go
@@ -197,7 +197,14 @@ func TestVendorLinksLoadAfterOurs(t *testing.T) {
// restatement check above, and is wrong in light mode forever.
func TestOurStylesheetHasNoColourLiterals(t *testing.T) {
css := rulesOnly(t, ourSheet)
- literal := regexp.MustCompile(`(?i)(#[0-9a-f]{3,8}\b|\brgba?\(|\bhsla?\(|:\s*(red|blue|green|black|white|gray|grey|orange|yellow|purple)\b)`)
+ // oklch() and color() are here because the app now paints cell fills
+ // with them. A LITERAL oklch — `oklch(0.92 0.08 145)` — is exactly
+ // the defect this test exists for and the original pattern could not
+ // see it. The legitimate uses all resolve a custom property, so the
+ // check is for a bare number where the lightness goes.
+ literal := regexp.MustCompile(`(?i)(#[0-9a-f]{3,8}\b|\brgba?\(|\bhsla?\(|` +
+ `\b(?:oklch|oklab|lch|lab|color)\(\s*[0-9.]|` +
+ `:\s*(red|blue|green|black|white|gray|grey|orange|yellow|purple)\b)`)
for i, line := range strings.Split(css, "\n") {
if m := literal.FindString(line); m != "" {
t.Errorf("%s:%d: colour literal %q — every colour here is a token, never a "+
diff --git a/internal/sheets/formatpatch.go b/internal/sheets/formatpatch.go
index 2e46bfc..763e939 100644
--- a/internal/sheets/formatpatch.go
+++ b/internal/sheets/formatpatch.go
@@ -6,13 +6,6 @@ import (
"amadan.net/oficina/sheets/internal/sheets/store"
)
-// maxNumFmtBytes caps a number-format pattern.
-//
-// A pattern, not a document. format's tokenizer walks it per painted
-// cell, so a long one is paid for on every render of every cell using
-// it — and no pattern anybody means to write is close to this.
-const maxNumFmtBytes = 128
-
// formatPatch is a change to the formatting of a rectangle of cells.
//
// **Every field is a pointer, and that is the whole design.** A toolbar
@@ -48,7 +41,7 @@ type formatPatch struct {
var (
errBadAlign = errors.New("sheets: unknown alignment")
errBadSwatch = errors.New("sheets: unknown colour")
- errBadNumFmt = errors.New("sheets: number format too long")
+ errBadNumFmt = errors.New("sheets: unknown number format")
errEmptyPatch = errors.New("sheets: patch changes nothing")
)
@@ -81,7 +74,9 @@ func (p formatPatch) validate() error {
if p.Align != nil && *p.Align != "" && !alignments[*p.Align] {
return errBadAlign
}
- if p.NumFmt != nil && len(*p.NumFmt) > maxNumFmtBytes {
+ // Against the offered list, not a length. See palette.go's
+ // numberFormat for why a byte cap was the wrong check.
+ if p.NumFmt != nil && !knownNumberFormat(*p.NumFmt) {
return errBadNumFmt
}
if p.Fill != nil && *p.Fill != noColour {
diff --git a/internal/sheets/grid.go b/internal/sheets/grid.go
index 96df309..980b171 100644
--- a/internal/sheets/grid.go
+++ b/internal/sheets/grid.go
@@ -26,6 +26,16 @@ type gridCell struct {
Raw string `json:"raw"`
Display string `json:"display"`
Derived bool `json:"derived,omitempty"`
+ // Num marks a cell whose COMPUTED value is a number, so the client
+ // can right-align it.
+ //
+ // It is a property of the value, not of the input: "=1+1" is a
+ // number and "1a" is text, and only the engine knows which. The
+ // alternative — letting the client sniff the display string — puts
+ // a second, worse number parser in JavaScript, and it would
+ // disagree with this one the first time a number format put a
+ // currency symbol in front of the digits.
+ Num bool `json:"n,omitempty"`
// F is the id of this cell's interned format, into the response's
// own Formats table. Empty for an unformatted cell — the common
// case, and the reason this is a key rather than an inline object.
@@ -75,6 +85,12 @@ type gridResponse struct {
// least one check crossing from computed value to painted output — and
// it is also what keeps the client from needing a number-format
// implementation that could disagree with the exporter's.
+// isNumber reports whether a computed value should read as a number.
+//
+// Booleans are excluded deliberately: Excel centres TRUE and FALSE, and
+// they are not quantities. Errors are excluded for the same reason.
+func isNumber(v calc.Value) bool { return v.Kind == calc.KindNum }
+
func display(v calc.Value, f *store.Format) string {
numFmt := ""
if f != nil {
@@ -144,6 +160,7 @@ func snapshot(h *store.Handle, idx posIndex, rows, cols int) ([]gridCell, styleT
R: r, C: colPos,
Raw: raw,
Display: display(h.Value(c.RowID, c.ColID), col.numFmtFor(c.RowID, c.ColID)),
+ Num: isNumber(h.Value(c.RowID, c.ColID)),
F: col.keyFor(c.RowID, c.ColID),
})
}
@@ -181,6 +198,7 @@ func fromReport(rep store.Report, idx posIndex, h *store.Handle) ([]gridCell, st
Raw: c.Raw,
Display: display(c.Value, col.numFmtFor(c.RowID, c.ColID)),
Derived: c.Derived,
+ Num: isNumber(c.Value),
F: col.keyFor(c.RowID, c.ColID),
})
}
diff --git a/internal/sheets/handlers_export.go b/internal/sheets/handlers_export.go
index 82b5be4..edf9ae8 100644
--- a/internal/sheets/handlers_export.go
+++ b/internal/sheets/handlers_export.go
@@ -64,7 +64,7 @@ func (a *App) handleExport(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type",
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")
- w.Header().Set("Content-Disposition", `attachment; filename="`+downloadName(s)+`"`)
+ w.Header().Set("Content-Disposition", contentDisposition(s))
w.Header().Set("Content-Length", strconv.Itoa(buf.Len()))
// A sealed document must not sit in a shared cache: the response
// body is every plaintext cell in the sheet.
@@ -73,7 +73,62 @@ func (a *App) handleExport(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write(buf.Bytes())
}
-// downloadName turns a sheet's title into a filename.
+// contentDisposition names the download, twice.
+//
+// RFC 6266's two-parameter form: an ASCII `filename` every client
+// understands, and an RFC 5987 `filename*` carrying the real title in
+// UTF-8 for every client of the last fifteen years. A client that
+// understands both must prefer `filename*`.
+//
+// Both, because the ASCII form drops every non-ASCII rune — so a suite
+// called Oficina would download "Presupuesto — año" as
+// "Presupuesto ao.xlsx" on day one. The fallback is still worth having:
+// it is what a client that cannot parse the extended form gets, and a
+// mangled name beats a refused download.
+func contentDisposition(s SheetSummary) string {
+ ascii := downloadName(s)
+ return `attachment; filename="` + ascii + `"; filename*=UTF-8''` + rfc5987(s)
+}
+
+// rfc5987 percent-encodes a filename for the extended parameter.
+//
+// Everything outside the RFC's `attr-char` set is escaped, which is
+// deliberately conservative: the set excludes the quote, the semicolon
+// and the backslash that could otherwise break out of the header, so
+// this is the encoding and the safety check at once.
+func rfc5987(s SheetSummary) string {
+ name := s.Title
+ if strings.TrimSpace(name) == "" {
+ name = "sheet-" + strconv.FormatUint(s.ID, 10)
+ }
+ var b strings.Builder
+ for _, r := range []byte(name + ".xlsx") {
+ if isAttrChar(r) {
+ b.WriteByte(r)
+ continue
+ }
+ b.WriteString("%" + strings.ToUpper(hexByte(r)))
+ if b.Len() > 400 {
+ break
+ }
+ }
+ return b.String()
+}
+
+func isAttrChar(c byte) bool {
+ switch {
+ case c >= 'A' && c <= 'Z', c >= 'a' && c <= 'z', c >= '0' && c <= '9':
+ return true
+ }
+ return strings.IndexByte("!#$&+-.^_`|~", c) >= 0
+}
+
+func hexByte(c byte) string {
+ const digits = "0123456789abcdef"
+ return string([]byte{digits[c>>4], digits[c&0x0f]})
+}
+
+// downloadName turns a sheet's title into an ASCII filename.
//
// ASCII, quoted, and conservative about what survives, because this
// goes into a header whose quoting rules are older than Unicode and
diff --git a/internal/sheets/handlers_export_test.go b/internal/sheets/handlers_export_test.go
new file mode 100644
index 0000000..54cf691
--- /dev/null
+++ b/internal/sheets/handlers_export_test.go
@@ -0,0 +1,43 @@
+package sheets
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+)
+
+func TestExportNamesTheFileAfterTheSheet(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Presupuesto — año")
+
+ rec := do(t, app, as(httptest.NewRequest(http.MethodGet,
+ "/sheets/"+itoaPos(int(s.ID))+"/export.xlsx", nil), cookies))
+ if rec.Code != http.StatusOK {
+ t.Fatalf("export = %d: %s", rec.Code, rec.Body.String())
+ }
+ // A real workbook, not an error page rendered with a 200.
+ if b := rec.Body.Bytes(); len(b) < 4 || b[0] != 'P' || b[1] != 'K' {
+ t.Fatalf("body is not a zip: %d bytes", len(b))
+ }
+
+ cd := rec.Header().Get("Content-Disposition")
+ // The ASCII fallback: every client understands it, and it must not
+ // carry a quote or a newline that could split the header.
+ if !strings.Contains(cd, `filename="Presupuesto ao.xlsx"`) {
+ t.Errorf("no usable ASCII filename in %q", cd)
+ }
+ // The extended form: without it, a suite called Oficina mangles the
+ // first Spanish sheet title anybody exports.
+ if !strings.Contains(cd, "filename*=UTF-8''") {
+ t.Errorf("no RFC 5987 filename in %q", cd)
+ }
+ if !strings.Contains(cd, "%C3%B1") { // the n-tilde of "año"
+ t.Errorf("the extended filename lost its non-ASCII characters: %q", cd)
+ }
+ // A sealed document must not sit in a shared cache: the body is
+ // every plaintext cell in the sheet.
+ if rec.Header().Get("Cache-Control") != "no-store" {
+ t.Errorf("Cache-Control = %q, want no-store", rec.Header().Get("Cache-Control"))
+ }
+}
diff --git a/internal/sheets/handlers_format.go b/internal/sheets/handlers_format.go
index fa74ad8..6f3f428 100644
--- a/internal/sheets/handlers_format.go
+++ b/internal/sheets/handlers_format.go
@@ -34,8 +34,9 @@ const maxFormatCells = 4000
// not a thing to do in passing. Until then the UI does not offer undo
// for formatting.
func (a *App) handleFormatPost(w http.ResponseWriter, r *http.Request) {
- actor, ok := a.actor(r)
- if !ok {
+ // The viewer is checked but not recorded: formatting appends no
+ // event, so there is no actor column to write one to.
+ if _, ok := a.actor(r); !ok {
http.Error(w, "Sign in first.", http.StatusForbidden)
return
}
@@ -71,8 +72,6 @@ func (a *App) handleFormatPost(w http.ResponseWriter, r *http.Request) {
a.fail(w, r, err)
return
}
- _ = actor // formatting records no event, so there is no actor to write
-
resp, err := formatResponse(h, coords, p.Client)
if err != nil {
a.fail(w, r, err)
@@ -155,6 +154,7 @@ func formatResponse(h *store.Handle, coords []store.CellCoord, client string) (g
R: rowPos, C: colPos,
Raw: raw,
Display: display(h.Value(c.RowID, c.ColID), col.numFmtFor(c.RowID, c.ColID)),
+ Num: isNumber(h.Value(c.RowID, c.ColID)),
F: col.keyFor(c.RowID, c.ColID),
})
}
diff --git a/internal/sheets/handlers_format_test.go b/internal/sheets/handlers_format_test.go
index d9f0806..c16ccaa 100644
--- a/internal/sheets/handlers_format_test.go
+++ b/internal/sheets/handlers_format_test.go
@@ -114,6 +114,36 @@ func TestFormatFillIsIntentNotHex(t *testing.T) {
}
}
+// TestFormatInkIsIntentNotHex is fill's twin, and it exists because
+// nothing else exercised the write side of ink at all: migration 0008's
+// columns, applyInk and the InkH/InkC wire fields were reachable only by
+// CLEARING them, so a bug that stored no ink would have passed
+// everything.
+func TestFormatInkIsIntentNotHex(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+
+ g := decodeGrid(t, postFormat(t, app, cookies, s, `{"r1":0,"c1":0,"r2":0,"c2":0,"ink":"red"}`))
+ st := styleFor(t, g, 0, 0)
+ if st.Ink != "" {
+ t.Errorf("ink hex = %q, want empty — an ink we apply is stored as intent so it "+
+ "follows the reader's theme, and a literal would be a light-mode colour "+
+ "sitting on a dark grid", st.Ink)
+ }
+ if st.InkH == nil || st.InkC == nil {
+ t.Fatalf("no ink intent: %+v", st)
+ }
+ if *st.InkH != 27 {
+ t.Errorf("ink hue = %v, want red's 27", *st.InkH)
+ }
+ // Ink is more saturated than a fill on purpose — a fill sits behind
+ // text and must not fight it, ink IS the text. The cap is what
+ // TestEverySwatchClearsAA measures against.
+ if *st.InkC != inkChroma {
+ t.Errorf("ink chroma = %v, want %v", *st.InkC, inkChroma)
+ }
+}
+
func TestFormatClearingAFillClearsTheImportedHexToo(t *testing.T) {
app, cookies := signIn(t)
s, _ := newSheet(t, app, cookies, "Budget")
@@ -178,8 +208,13 @@ func TestFormatRefusesWhatItCannotApply(t *testing.T) {
for name, body := range map[string]string{
"unknown colour": `{"r1":0,"c1":0,"r2":0,"c2":0,"fill":"chartreuse"}`,
"unknown alignment": `{"r1":0,"c1":0,"r2":0,"c2":0,"align":"justify"}`,
- "empty patch": `{"r1":0,"c1":0,"r2":0,"c2":0}`,
- "negative range": `{"r1":-1,"c1":0,"r2":0,"c2":0,"bold":true}`,
+ // A pattern the UI never offers. Before the allowlist this was
+ // 128 bytes of free text stored verbatim, and an Excel pattern
+ // admits quoted literals — so it was an arbitrary plaintext
+ // write into the formats table through a public API.
+ "unoffered number format": `{"r1":0,"c1":0,"r2":0,"c2":0,"numfmt":"\"Overdue: \"0"}`,
+ "empty patch": `{"r1":0,"c1":0,"r2":0,"c2":0}`,
+ "negative range": `{"r1":-1,"c1":0,"r2":0,"c2":0,"bold":true}`,
} {
t.Run(name, func(t *testing.T) {
rec := postFormat(t, app, cookies, s, body)
diff --git a/internal/sheets/handlers_sheets.go b/internal/sheets/handlers_sheets.go
index e04c191..706b906 100644
--- a/internal/sheets/handlers_sheets.go
+++ b/internal/sheets/handlers_sheets.go
@@ -61,6 +61,10 @@ type sheetPageView struct {
// both the buttons and the write endpoint's validation read.
FillSwatches []swatch
InkSwatches []swatch
+ // NumberFormats is the same list the write endpoint validates
+ // against, so a button the UI renders is a pattern the route
+ // accepts — by construction rather than by both being edited.
+ NumberFormats []numberFormat
// Warned lists cells whose IMPORTED fill and ink fail WCAG AA.
// Spec §11.1 ruled that such a pair is rendered as authored and
// that the obligation is surfacing, not overriding — so this is a
@@ -137,6 +141,7 @@ func (a *App) handleSheetPage(w http.ResponseWriter, r *http.Request) {
view := sheetPageView{
Sheet: s, Rows: defaultWindowRows, Cols: defaultWindowCols,
FillSwatches: fillSwatches, InkSwatches: inkSwatches,
+ NumberFormats: numberFormats,
}
if viewer, ok := a.actor(r); ok {
view.MayShare = a.mayShare(s.ID, viewer)
diff --git a/internal/sheets/migrations/schema.sql b/internal/sheets/migrations/schema.sql
index f386203..cbc143a 100644
--- a/internal/sheets/migrations/schema.sql
+++ b/internal/sheets/migrations/schema.sql
@@ -9,7 +9,7 @@ CREATE TABLE `comments` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer
CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text,`tab_id` integer,`actor_id` integer,`payload_sealed` blob,`trigger_id` integer,`derived_count` integer,`created_at` datetime);
-CREATE TABLE `formats` (`id` integer PRIMARY KEY AUTOINCREMENT,`digest` text,`number_fmt` text,`bold` numeric,`italic` numeric,`fill_intent_h` real,`fill_intent_c` real,`fill_hex` text,`align` text, `ink_hex` text);
+CREATE TABLE `formats` (`id` integer PRIMARY KEY AUTOINCREMENT,`digest` text,`number_fmt` text,`bold` numeric,`italic` numeric,`fill_intent_h` real,`fill_intent_c` real,`fill_hex` text,`align` text, `ink_hex` text, `ink_intent_h` real, `ink_intent_c` real);
CREATE TABLE `grants` (`id` integer PRIMARY KEY AUTOINCREMENT,`sheet_id` integer,`member_id` integer,`role` text,`granted_by` integer,`created_at` datetime,`updated_at` datetime);
diff --git a/internal/sheets/palette.go b/internal/sheets/palette.go
index f891ed2..2c3c6fe 100644
--- a/internal/sheets/palette.go
+++ b/internal/sheets/palette.go
@@ -90,6 +90,53 @@ func buildSwatches(chroma float64) []swatch {
return out
}
+// numberFormat is one pattern the toolbar and the Format menu offer.
+//
+// **The list is the validation.** Before it existed, the format route
+// took 128 bytes of free text and stored it: an Excel pattern admits
+// quoted literals (`"Overdue: "0`), so that was an arbitrary plaintext
+// write into the formats table through a public API, in the family spec
+// §2.1 calls "user data living inside a format-adjacent record". The UI
+// has only ever offered five patterns; accepting only those closes the
+// channel and costs nothing.
+//
+// The XLSX importer is deliberately not bound by this. A file's own
+// custom format is the document's, retained as authored the same way
+// its fills are — a different path, a different obligation, and one the
+// spec already accounts for.
+type numberFormat struct {
+ // Code is the pattern format.Display renders. Empty is "Automatic",
+ // which is a real state and not a synonym for the general format.
+ Code string
+ // Label is what a person reads; Sample shows the pattern's effect
+ // on a number, which is the only way to tell 0.00 from #,##0.00
+ // without knowing the grammar.
+ Label, Sample string
+}
+
+// numberFormats is what the UI offers, in the order it renders them.
+//
+// Currency is absent on purpose: the symbol is a locale decision nobody
+// has made, and guessing it would put a wrong currency on somebody's
+// invoice.
+var numberFormats = []numberFormat{
+ {"", "Automatic", ""},
+ {"0.00", "Number", "1234.57"},
+ {"#,##0.00", "Thousands", "1,234.57"},
+ {"0%", "Percent", "57%"},
+ {"yyyy-mm-dd", "Date", "2026-09-02"},
+}
+
+// knownNumberFormat reports whether a code is one the UI offers.
+func knownNumberFormat(code string) bool {
+ for _, f := range numberFormats {
+ if f.Code == code {
+ return true
+ }
+ }
+ return false
+}
+
// noColour is the wire value that CLEARS a fill or an ink.
//
// A distinct name rather than an empty string, because an empty string
@@ -113,13 +160,3 @@ func lookupSwatch(list []swatch, name string) (swatch, bool) {
}
return swatch{}, false
}
-
-// FillSwatches and InkSwatches expose the palettes to the templates.
-//
-// Methods on App rather than package-level template funcs, because the
-// template already receives the app's page data and adding a func map
-// entry for a constant list is a second mechanism for no gain.
-func (a *App) FillSwatches() []swatch { return fillSwatches }
-
-// InkSwatches is FillSwatches' twin — see it.
-func (a *App) InkSwatches() []swatch { return inkSwatches }
diff --git a/internal/sheets/preview.go b/internal/sheets/preview.go
index 9472e77..0107434 100644
--- a/internal/sheets/preview.go
+++ b/internal/sheets/preview.go
@@ -14,8 +14,9 @@ import (
// — and it never opens a sealed value. That is not a shortcut, it is
// the design: a preview that cannot read a cell cannot leak one, so the
// one rule needs no argument here and no exemption. It also means
-// building one costs two indexed queries rather than opening a Handle
-// and recomputing a formula graph, which is what makes a list of twenty
+// building one costs four indexed reads — rows, columns, cells, and the
+// interned formats those cells name — rather than opening a Handle and
+// recomputing a formula graph, which is what makes a list of twenty
// sheets affordable at all.
//
// What it therefore shows is real and honest: where the data is, which
@@ -70,9 +71,9 @@ type preview struct {
// buildPreview reads a tab's shape.
//
-// Two queries and no Handle. The row and column records give position,
-// the cells table gives presence and a format id, and the formats table
-// gives the rest. Nothing here can reach a sealed column.
+// Four indexed reads and no Handle: the row and column records give
+// position, the cells table gives presence and a format id, and the
+// formats table gives the rest. Nothing here can reach a sealed column.
func (a *App) buildPreview(tabID uint64) preview {
if a.db == nil || tabID == 0 {
return preview{Empty: true}
diff --git a/internal/sheets/preview_test.go b/internal/sheets/preview_test.go
index ea6a12e..67ca9ed 100644
--- a/internal/sheets/preview_test.go
+++ b/internal/sheets/preview_test.go
@@ -57,7 +57,7 @@ func firstPreview(t *testing.T, page string) string {
// rectangles could plausibly emit.
func TestAPreviewNeverCarriesACell(t *testing.T) {
app, cookies := signIn(t)
- s, cells := newSheet(t, app, cookies, "Ledger")
+ _, cells := newSheet(t, app, cookies, "Ledger")
secrets := []string{"Zanzibar", "Quixotry", "Vellichor"}
for i, word := range secrets {
@@ -66,7 +66,6 @@ func TestAPreviewNeverCarriesACell(t *testing.T) {
t.Fatalf("write %q = %d", word, rec.Code)
}
}
- _ = s
svg := firstPreview(t, listBody(t, app, cookies))
diff --git a/internal/sheets/sheetsvc.go b/internal/sheets/sheetsvc.go
index 4ca1dba..9f1e74b 100644
--- a/internal/sheets/sheetsvc.go
+++ b/internal/sheets/sheetsvc.go
@@ -233,10 +233,13 @@ func (a *App) summarize(s store.Sheet) (SheetSummary, error) {
// tabName unseals a tab's name, falling back to a generic label.
//
-// The fallback covers two real cases and not a hypothetical one: a tab
-// created before anything wrote a name (NameSealed is empty on the
-// first tab of every sheet this app has ever made), and a name that
-// will not open. Both should leave a person looking at a working grid.
+// The fallback is for a name that will not open — a row written under a
+// key this process does not have. It is not for an empty NameSealed:
+// createSheet has always sealed firstTabName, so no tab this app has
+// made lacks one. Either way the answer is the same and it is the
+// right one: the tab strip is chrome, and a grid nobody can open
+// because its tab's name is unreadable is a worse outcome than a tab
+// labelled generically.
func tabName(a *App, tab store.Tab) string {
if len(tab.NameSealed) == 0 {
return "Sheet 1"
diff --git a/internal/sheets/static/commands.js b/internal/sheets/static/commands.js
index d863dc3..ea8efc9 100644
--- a/internal/sheets/static/commands.js
+++ b/internal/sheets/static/commands.js
@@ -154,7 +154,33 @@
e.preventDefault();
run(el.dataset.cmd, el.dataset.value !== undefined ? el.dataset.value : el.dataset.fn);
var panel = el.closest('[data-tool-menu]');
- if (panel) panel.open = false;
+ if (!panel) return;
+ /* Closing the panel leaves focus on a button inside a closed
+ * <details>, which drops it to <body> — so the keyboard ends up
+ * at the top of the document after picking a colour. Hand it back
+ * to the grid, which is the same rule menubar.js applies to a
+ * menu item: the thing you asked for happened to a cell, so that
+ * is where you want to be next. */
+ panel.open = false;
+ g.focus();
+ });
+
+ /* The menu's resize forms. The size route takes one axis at a time
+ * and applies to the selection's anchor, which is the column or row
+ * the cursor is in — the same thing dragging its edge would do. */
+ document.querySelectorAll('[data-size-form]').forEach(function (form) {
+ form.addEventListener('submit', function (e) {
+ e.preventDefault();
+ var sel = g.selection();
+ var px = parseInt(form.querySelector('input[name=size]').value, 10);
+ if (!sel || !isFinite(px)) return;
+ var body = form.dataset.axis === 'col' ? { col: sel.c } : { row: sel.r };
+ body.size = px;
+ g.post('/size', body);
+ var menu = form.closest('details');
+ if (menu) menu.open = false;
+ g.focus();
+ });
});
/* ---- shortcuts ---------------------------------------------------
@@ -171,12 +197,13 @@
var t = e.target;
if (t && (t.tagName === 'INPUT' || t.tagName === 'TEXTAREA' || t.isContentEditable)) return;
- if (e.key === 'Delete' && g.mayWrite) {
- if (!g.selection()) return;
- e.preventDefault();
- run('clear');
- return;
- }
+ /* Delete is deliberately NOT handled here. The vendored grid
+ * already clears the selection through its own setValue, which
+ * fires onchange and reaches write() — so handling it too posted
+ * every selected cell twice, and Backspace (which the library
+ * also handles) only once, which is worse than either. The menu
+ * item stays: it is the affordance, and it runs the same code the
+ * key does, once. */
if (!e.ctrlKey && !e.metaKey) return;
if (e.altKey || e.shiftKey) return;
var cmd = KEYS[e.key.toLowerCase()];
diff --git a/internal/sheets/static/grid.js b/internal/sheets/static/grid.js
index e116254..6ab046d 100644
--- a/internal/sheets/static/grid.js
+++ b/internal/sheets/static/grid.js
@@ -32,13 +32,22 @@
var base = '/api/sheets/' + sheetId + '/tabs/' + tabId;
var host = surface.querySelector('[data-sheet-grid]');
+ /* paint.js, or no-ops. Resolved once so no call site guards, and so a
+ * module that failed to load costs styling and not the grid. */
+ var noop = function () {};
+ var paint = window.SheetsPaint || { bind: noop, repaint: noop, sweep: noop };
+
/* A per-tab id so this tab can recognise its own edit coming back
* over the stream. Without it the tab you typed in repaints the cell
* you are still editing. */
var clientId = 'c' + Math.random().toString(36).slice(2) + Date.now().toString(36);
- var raw = Object.create(null); /* "r,c" -> the input text */
- var fmtOf = Object.create(null); /* "r,c" -> a format key, or absent */
+ /* meta is everything known about a cell that is not its painted
+ * string: "r,c" -> {raw, f, n}. One map rather than three parallel
+ * ones, because every path that adds a cell has to add it to all of
+ * them and every path that clears has to clear all of them — and the
+ * one that forgets is invisible until a format outlives its cell. */
+ var meta = Object.create(null);
var styles = Object.create(null); /* format key -> the style */
var version = 0; /* the newest version painted */
var ws = null;
@@ -47,33 +56,17 @@
var listeners = [];
function key(r, c) { return r + ',' + c; }
+ function at(r, c) { return meta[key(r, c)] || {}; }
function blank() {
var grid = [], r, c, row;
for (r = 0; r < rows; r++) {
- row = [];
- for (c = 0; c < cols; c++) row.push('');
+ for (row = [], c = 0; c < cols; c++) row.push('');
grid.push(row);
}
return grid;
}
- /* tdAt reaches the cell element Jspreadsheet rendered. Verified
- * against the vendored build: setValueFromCoords rewrites the td's
- * innerHTML and leaves the element, its attributes and its style
- * alone, so a paint applied here survives every later value update.
- * The only code that resets a className is the delete-column path,
- * which is disabled below. */
- function tdAt(r, c) {
- var rec = ws && ws.records && ws.records[r];
- return (rec && rec[c] && rec[c].element) || null;
- }
-
- function repaint(r, c) {
- if (!window.SheetsPaint) return;
- window.SheetsPaint.cell(tdAt(r, c), styles[fmtOf[key(r, c)]]);
- }
-
/* applyBatch paints a server response.
*
* Out-of-order paints are discarded BY VERSION, not by arrival (spec
@@ -106,11 +99,9 @@
try {
payload.cells.forEach(function (cell) {
if (cell.r >= rows || cell.c >= cols) return;
- var k = key(cell.r, cell.c);
- raw[k] = cell.raw || '';
- if (cell.f) { fmtOf[k] = cell.f; } else { delete fmtOf[k]; }
+ meta[key(cell.r, cell.c)] = { raw: cell.raw || '', f: cell.f, n: cell.n };
ws.setValueFromCoords(cell.c, cell.r, cell.display, true);
- repaint(cell.r, cell.c);
+ paint.repaint(cell.r, cell.c);
});
} finally {
applying = false;
@@ -128,9 +119,9 @@
* lost while this tab was disconnected — those cells are simply
* absent from it, so nothing below would ever unpaint them. */
version = 0;
- fmtOf = Object.create(null);
+ meta = Object.create(null);
styles = Object.create(null);
- if (window.SheetsPaint) window.SheetsPaint.sweep(host);
+ paint.sweep(host);
applyBatch(payload);
})
.catch(function () { /* the stream's own reconnect will try again */ });
@@ -177,9 +168,7 @@
(payload.cells || []).forEach(function (cell) {
if (cell.r >= rows || cell.c >= cols) return;
data[cell.r][cell.c] = cell.display;
- var k = key(cell.r, cell.c);
- raw[k] = cell.raw || '';
- if (cell.f) fmtOf[k] = cell.f;
+ meta[key(cell.r, cell.c)] = { raw: cell.raw || '', f: cell.f, n: cell.n };
});
if (payload.formats) styles = payload.formats;
version = payload.version || 0;
@@ -210,7 +199,7 @@
* on the next frame — best effort, because the formula bar is
* the reliable path and this is the convenience. */
oneditionstart: function (instance, td, c, r) {
- var text = raw[key(parseInt(r, 10), parseInt(c, 10))];
+ var text = at(parseInt(r, 10), parseInt(c, 10)).raw;
if (!text) return;
requestAnimationFrame(function () {
var input = td.querySelector('input, textarea');
@@ -220,6 +209,11 @@
worksheets: [{
data: data,
+ /* What "no alignment" means; see sheets.css's alignment block.
+ * A WORKSHEET option — the exact mirror of the handlers above,
+ * which are spreadsheet-level. Each is silently ignored in the
+ * other's position: accepted, never read. */
+ defaultColAlign: 'left',
minDimensions: [cols, rows],
tableOverflow: true,
columnSorting: false, /* sorting is a later slice; a half-done sort is worse than none */
@@ -250,14 +244,20 @@
* disagree — so the local stack is switched off rather than raced. */
ws.ignoreHistory = true;
+ /* paint.js owns the td and the drawing; this says what a cell is. */
+ paint.bind(ws, function (r, c) {
+ var m = at(r, c);
+ return { style: styles[m.f], n: m.n };
+ });
+
/* Paint the formats the snapshot arrived with. Deferred a frame:
* the records array is populated during construction, and painting
* inside the same tick reaches a table the library has not finished
* laying out. */
requestAnimationFrame(function () {
- Object.keys(fmtOf).forEach(function (k) {
+ Object.keys(meta).forEach(function (k) {
var parts = k.split(',');
- repaint(parseInt(parts[0], 10), parseInt(parts[1], 10));
+ paint.repaint(parseInt(parts[0], 10), parseInt(parts[1], 10));
});
});
@@ -277,8 +277,8 @@
cols: cols,
mayWrite: surface.dataset.mayWrite === '1',
selection: function () { return sel; },
- rawAt: function (r, c) { return raw[key(r, c)] || ''; },
- styleAt: function (r, c) { return styles[fmtOf[key(r, c)]] || null; },
+ rawAt: function (r, c) { return at(r, c).raw || ''; },
+ styleAt: function (r, c) { return styles[at(r, c).f] || null; },
worksheet: function () { return ws; },
focus: function () { if (host) host.focus(); },
onSelection: function (fn) { listeners.push(fn); if (sel) fn(sel); }
diff --git a/internal/sheets/static/paint.js b/internal/sheets/static/paint.js
index b3baa03..64d08ea 100644
--- a/internal/sheets/static/paint.js
+++ b/internal/sheets/static/paint.js
@@ -19,8 +19,8 @@
/* Every attribute this module owns. Listed once, used by both the
* clear path and the sweep grid.js does before a full snapshot, so
* the two cannot disagree about what "painted" means. */
- var ATTRS = ['data-b', 'data-i', 'data-a', 'data-fill', 'data-ink',
- 'data-fx', 'data-k', 'data-contrast'];
+ var ATTRS = ['data-b', 'data-i', 'data-a', 'data-n', 'data-fill',
+ 'data-ink', 'data-fx', 'data-k', 'data-contrast'];
var PROPS = ['--sheet-h', '--sheet-c', '--sheet-kh', '--sheet-kc',
'--sheet-fx', '--sheet-k'];
@@ -58,11 +58,27 @@
for (i = 0; i < PROPS.length; i++) td.style.removeProperty(PROPS[i]);
}
+ /* number marks a cell whose computed value the SERVER says is a
+ * number, so the stylesheet can right-align it. Separate from cell()
+ * because it is a property of the value and not of the format, and
+ * the two arrive on different fields and change independently. */
+ function number(td, isNum) {
+ if (!td) return;
+ if (isNum) td.setAttribute('data-n', '');
+ else td.removeAttribute('data-n');
+ }
+
/* cell paints one td. Clearing first means a format change never has
* to be a diff: whatever the cell looked like, it now looks like s. */
function cell(td, s) {
if (!td) return;
+ var wasNum = td.hasAttribute('data-n');
clear(td);
+ /* clear() drops every attribute this module owns, including the
+ * numeric mark, which is not part of the style being applied — so
+ * it is put back. Losing it here would right-align a number until
+ * you formatted it and then never again. */
+ if (wasNum) td.setAttribute('data-n', '');
if (!s) return;
if (s.b) td.setAttribute('data-b', '');
@@ -113,6 +129,33 @@
if (s.lc) td.setAttribute('data-contrast', 'low');
}
+ /* bind ties this module to a live worksheet and to a way of looking a
+ * cell's paint up, so nothing outside it has to reach into the
+ * library's records array. Painting lives here; grid.js says WHAT a
+ * cell is, not how to draw it.
+ *
+ * Verified against the vendored build: setValueFromCoords rewrites a
+ * td's innerHTML and leaves the element, its attributes and its style
+ * alone, so a paint applied here survives every later value update.
+ * The only code that resets a className is the delete-column path,
+ * which the grid disables. */
+ var sheet = null;
+ var lookup = null;
+
+ function bind(ws, fn) { sheet = ws; lookup = fn; }
+
+ function tdAt(r, c) {
+ var rec = sheet && sheet.records && sheet.records[r];
+ return (rec && rec[c] && rec[c].element) || null;
+ }
+
+ function repaint(r, c) {
+ if (!lookup) return;
+ var td = tdAt(r, c), m = lookup(r, c);
+ cell(td, m.style);
+ number(td, !!m.n);
+ }
+
/* sweep unpaints everything, for the one case a per-cell repaint
* cannot cover: a full snapshot supersedes the whole window, and a
* cell whose format was removed while this tab was disconnected is
@@ -123,5 +166,5 @@
for (var i = 0; i < painted.length; i++) clear(painted[i]);
}
- window.SheetsPaint = { cell: cell, sweep: sweep };
+ window.SheetsPaint = { bind: bind, repaint: repaint, sweep: sweep };
}());
diff --git a/internal/sheets/static/sheets.css b/internal/sheets/static/sheets.css
index c7fa716..1b9a8b2 100644
--- a/internal/sheets/static/sheets.css
+++ b/internal/sheets/static/sheets.css
@@ -102,6 +102,11 @@
inset-inline-start: 0;
inset-inline-end: auto;
min-inline-size: 15rem;
+ /* 15rem is 240px, and the menus nearest the right edge open from
+ * there — so on a 320px viewport an open panel ran off the screen
+ * even though the closed chrome fits. max wins over min in CSS, so
+ * this bounds it without the panel becoming narrow on a wide one. */
+ max-inline-size: calc(100vw - 1rem);
}
/* An item is a row with its shortcut pushed to the far end, which is
@@ -204,8 +209,7 @@
color: var(--rst-text);
}
-[data-tool][aria-pressed="true"],
-[data-tool][aria-current] {
+[data-tool][aria-pressed="true"] {
background: var(--rst-accent-soft);
box-shadow: inset 0 0 0 1px currentColor;
color: var(--rst-accent);
@@ -559,13 +563,27 @@
* rule was opaque and took that back, leaving a range with no visible
* cursor in either scheme.
*
- * It comes LAST on purpose. It ties .highlight and .selection at
- * (0,3,3), and the anchor is inside both while you drag the fill handle
- * — so on a tie it has to be the later rule or the cursor disappears at
- * exactly the moment you are aiming with it. */
+ * It comes LAST among the SELECTION rules on purpose. It ties
+ * .highlight and .selection at (0,3,3), and the anchor is inside both
+ * while you drag the fill handle — so on a tie it has to be the later
+ * rule or the cursor disappears at exactly the moment you are aiming
+ * with it.
+ *
+ * **The ring is what actually marks it, and the background no longer
+ * can.** Cell fills are painted further down this file, at the same
+ * (0,3,3) and therefore later, so on a filled range a background-only
+ * anchor is indistinguishable from the rest of the selection — the
+ * exact failure the paragraph above guards against, arriving by a new
+ * route. An inset shadow survives any fill because it is drawn over
+ * one, and it costs no layout, so it cannot shift a cell by a pixel
+ * when the cursor lands on it.
+ *
+ * The background stays for the unfilled case, where it is what makes
+ * the anchor read as "not part of the tint". */
[data-sheet-grid] .jss_worksheet > tbody > tr > td.highlight-selected {
background-color: var(--rst-bg);
color: var(--rst-text);
+ box-shadow: inset 0 0 0 2px var(--rst-accent);
}
/* The fill handle at the corner of the range: a black knob ringed in
@@ -842,16 +860,39 @@
font-style: italic;
}
+/* ── Alignment, and why these three rules carry !important ───────────
+ *
+ * Jspreadsheet writes `text-align` as an INLINE style on every data
+ * cell, from the column's `align` or from `defaultColAlign`. An inline
+ * declaration outranks any selector, at any specificity, so the
+ * ordinary version of these rules set the attribute, reflected
+ * aria-pressed on the button, and changed nothing on screen. The
+ * toolbar looked like it worked. Only a browser could see it, and the
+ * branch's own drive did not check alignment — bold and fill, which
+ * both paint from properties the library does not touch, passed.
+ *
+ * !important is the correct tool here and not a shortcut: it is what
+ * outranks an inline style, and there is no other. These are new paint
+ * rather than a restatement of a vendored rule, so the restatement
+ * guard is unaffected.
+ *
+ * The numeric rule is important for the same reason and comes FIRST,
+ * so an explicit alignment beats it on source order — both are
+ * important and (0,3,3), so nothing else separates them. */
+[data-sheet-grid] .jss_worksheet > tbody > tr > td[data-n] {
+ text-align: end !important;
+}
+
[data-sheet-grid] .jss_worksheet > tbody > tr > td[data-a="left"] {
- text-align: start;
+ text-align: start !important;
}
[data-sheet-grid] .jss_worksheet > tbody > tr > td[data-a="center"] {
- text-align: center;
+ text-align: center !important;
}
[data-sheet-grid] .jss_worksheet > tbody > tr > td[data-a="right"] {
- text-align: end;
+ text-align: end !important;
}
/* Ours: hue and chroma from the cell, lightness from the theme. */
@@ -892,6 +933,17 @@
--rst-cols: 26px 1fr;
}
+/* Below 800px rastrillo sets grid-template-columns on [rst-lrow]
+ * DIRECTLY rather than through --rst-cols, so the hook above stops
+ * being read and the thumbnail lands in the title's column with an
+ * empty one trailing. This is (0,2,0) against the framework's (0,1,0),
+ * so it wins wherever it sits in the file. */
+@media (max-width: 800px) {
+ [data-sheet-list] [rst-lrow] {
+ grid-template-columns: 26px minmax(0, 1fr);
+ }
+}
+
[data-preview-slot] {
display: block;
block-size: 34px;
@@ -901,3 +953,23 @@
[data-preview] {
display: block;
}
+
+/* The Format menu's two resize forms. A row rather than a stack: the
+ * label, the number and the button read as one control, and the panel
+ * is already 15rem wide enough to hold them. */
+[data-size-form] {
+ align-items: center;
+ display: flex;
+ gap: var(--rst-sp-2);
+ padding: var(--rst-sp-1) var(--rst-sp-2);
+}
+
+[data-size-form] label {
+ color: var(--rst-text-muted);
+ flex: 1;
+ font-size: var(--rst-fs-sm);
+}
+
+[data-size-form] input {
+ inline-size: 4.5rem;
+}
diff --git a/internal/sheets/store/handle_formats.go b/internal/sheets/store/handle_formats.go
index 58d4a8d..e623dda 100644
--- a/internal/sheets/store/handle_formats.go
+++ b/internal/sheets/store/handle_formats.go
@@ -66,37 +66,3 @@ func (h *Handle) Formats() (map[CellCoord]*Format, error) {
}
return out, nil
}
-
-// FormatByID returns one interned Format, or nil.
-//
-// It exists for the write path, which knows the id it just interned and
-// wants the row back to echo to the client without re-deriving it.
-func (h *Handle) FormatByID(id uint64) *Format {
- if id == 0 {
- return nil
- }
- var f Format
- if err := h.db.First(&f, id).Error; err != nil {
- return nil
- }
- return &f
-}
-
-// FormatIDAt returns the Format id assigned to a cell, or 0.
-//
-// The id rather than the row, because the caller that needs this — a
-// format toggle merging into what a cell already has — needs to know
-// whether two cells share a format, and comparing ids answers that
-// without reading either row.
-func (h *Handle) FormatIDAt(rowID, colID uint64) uint64 {
- var cell Cell
- if err := h.db.Select("format_id").
- Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, rowID, colID).
- First(&cell).Error; err != nil {
- return 0
- }
- if cell.FormatID == nil {
- return 0
- }
- return *cell.FormatID
-}
diff --git a/internal/sheets/templates/sheet.html b/internal/sheets/templates/sheet.html
index 5729403..4220f2c 100644
--- a/internal/sheets/templates/sheet.html
+++ b/internal/sheets/templates/sheet.html
@@ -119,6 +119,7 @@
</details>
{{end}}
+{{if or .Data.MayWrite .Data.MayComment}}
<details rst-dropdown name="rst-menus">
<summary>Insert</summary>
<div rst-dropdown-menu>
@@ -144,6 +145,7 @@
{{if .Data.MayComment}}<a href="/sheets/{{.Data.Sheet.ID}}/comments">Comment…</a>{{end}}
</div>
</details>
+{{end}}
{{if .Data.MayWrite}}
{{/* The Format menu is entirely script-driven, so the whole menu is
@@ -166,15 +168,12 @@
<details rst-menu-group>
<summary>Number</summary>
<div>
-{{/* Every pattern here is one the server's own format package
- renders. Currency is absent on purpose: the symbol is a
- locale decision nobody has made, and guessing it would put a
- wrong currency on somebody's invoice. */}}
-<button type="button" data-cmd="numfmt" data-value="">Automatic</button>
-<button type="button" data-cmd="numfmt" data-value="0.00">Number <span data-sample>1234.57</span></button>
-<button type="button" data-cmd="numfmt" data-value="#,##0.00">Thousands <span data-sample>1,234.57</span></button>
-<button type="button" data-cmd="numfmt" data-value="0%">Percent <span data-sample>57%</span></button>
-<button type="button" data-cmd="numfmt" data-value="yyyy-mm-dd">Date <span data-sample>2026-09-02</span></button>
+{{/* Ranged over the list the write endpoint validates against, so a
+ button the UI renders is a pattern the route accepts — by
+ construction, not by remembering to edit both. */}}
+{{range .Data.NumberFormats}}
+<button type="button" data-cmd="numfmt" data-value="{{.Code}}">{{.Label}}{{if .Sample}} <span data-sample>{{.Sample}}</span>{{end}}</button>
+{{end}}
</div>
</details>
<details rst-menu-group>
@@ -198,6 +197,28 @@
</div>
</details>
<hr>
+{{/* The menu equivalent of dragging a column edge (AGENTS.md: every
+ drag-only affordance has one). A drag is unreachable by keyboard
+ and awkward on a touch screen, and the size it produces is the
+ one thing on this menu you cannot express as a choice from a
+ list — so it is a number field rather than a set of buttons.
+
+ Real forms rather than a prompt(): a prompt is not styleable, not
+ translatable, and is blocked outright in some embedding contexts.
+ They post through commands.js because the size route takes JSON;
+ the scriptless path to a resize is named in the spec's §14 with
+ the rest of the table view's formatting debt. */}}
+<form data-size-form data-axis="col">
+<label for="colw">Column width</label>
+<input rst-input id="colw" name="size" type="number" min="16" max="2000" step="1" placeholder="px">
+<button rst-btn="ghost" type="submit">Set</button>
+</form>
+<form data-size-form data-axis="row">
+<label for="rowh">Row height</label>
+<input rst-input id="rowh" name="size" type="number" min="16" max="2000" step="1" placeholder="px">
+<button rst-btn="ghost" type="submit">Set</button>
+</form>
+<hr>
<button type="button" data-cmd="clear-format">Clear formatting</button>
</div>
</details>
@@ -253,30 +274,28 @@
deliberately, so the two apps behave alike. */}}
<div data-toolbar role="toolbar" aria-label="Formatting" aria-controls="main" hidden data-needs-js>
-<button type="button" data-tool data-cmd="undo" aria-label="Undo" data-tip="Undo (Ctrl+Z)">{{template "glyph-undo"}}</button>
+<button type="button" data-tool rst-tip data-cmd="undo" aria-label="Undo" data-tip="Undo (Ctrl+Z)">{{template "glyph-undo"}}</button>
<span data-tool-rule aria-hidden="true"></span>
<details rst-dropdown name="rst-tools" data-tool-menu>
-<summary data-tool aria-label="Number format" data-tip="Number format">123<span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary>
+<summary data-tool rst-tip aria-label="Number format" data-tip="Number format">123<span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary>
<div rst-dropdown-menu>
-<button type="button" data-cmd="numfmt" data-value="">Automatic</button>
-<button type="button" data-cmd="numfmt" data-value="0.00">Number</button>
-<button type="button" data-cmd="numfmt" data-value="#,##0.00">Thousands</button>
-<button type="button" data-cmd="numfmt" data-value="0%">Percent</button>
-<button type="button" data-cmd="numfmt" data-value="yyyy-mm-dd">Date</button>
+{{range .Data.NumberFormats}}
+<button type="button" data-cmd="numfmt" data-value="{{.Code}}">{{.Label}}</button>
+{{end}}
</div>
</details>
<span data-tool-rule aria-hidden="true"></span>
-<button type="button" data-tool data-cmd="bold" aria-pressed="false" aria-label="Bold" data-tip="Bold (Ctrl+B)"><b>B</b></button>
-<button type="button" data-tool data-cmd="italic" aria-pressed="false" aria-label="Italic" data-tip="Italic (Ctrl+I)"><i>I</i></button>
+<button type="button" data-tool rst-tip data-cmd="bold" aria-pressed="false" aria-label="Bold" data-tip="Bold (Ctrl+B)"><b>B</b></button>
+<button type="button" data-tool rst-tip data-cmd="italic" aria-pressed="false" aria-label="Italic" data-tip="Italic (Ctrl+I)"><i>I</i></button>
<span data-tool-rule aria-hidden="true"></span>
<details rst-dropdown name="rst-tools" data-tool-menu>
-<summary data-tool aria-label="Fill colour" data-tip="Fill colour"><span data-fill-chip aria-hidden="true"></span><span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary>
+<summary data-tool rst-tip aria-label="Fill colour" data-tip="Fill colour"><span data-fill-chip aria-hidden="true"></span><span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary>
<div rst-dropdown-menu data-swatches>
<button type="button" data-cmd="fill" data-value="none">No fill</button>
{{range .Data.FillSwatches}}
@@ -287,7 +306,7 @@
</details>
<details rst-dropdown name="rst-tools" data-tool-menu>
-<summary data-tool aria-label="Text colour" data-tip="Text colour"><span data-ink-chip aria-hidden="true">A</span><span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary>
+<summary data-tool rst-tip aria-label="Text colour" data-tip="Text colour"><span data-ink-chip aria-hidden="true">A</span><span rst-caret aria-hidden="true">{{icon "chevron-down"}}</span></summary>
<div rst-dropdown-menu data-swatches>
<button type="button" data-cmd="ink" data-value="none">Default</button>
{{range .Data.InkSwatches}}
@@ -299,9 +318,9 @@
<span data-tool-rule aria-hidden="true"></span>
-<button type="button" data-tool data-cmd="align" data-value="left" aria-pressed="false" aria-label="Align left" data-tip="Align left">{{template "glyph-align-left"}}</button>
-<button type="button" data-tool data-cmd="align" data-value="center" aria-pressed="false" aria-label="Align centre" data-tip="Align centre">{{template "glyph-align-center"}}</button>
-<button type="button" data-tool data-cmd="align" data-value="right" aria-pressed="false" aria-label="Align right" data-tip="Align right">{{template "glyph-align-right"}}</button>
+<button type="button" data-tool rst-tip data-cmd="align" data-value="left" aria-pressed="false" aria-label="Align left" data-tip="Align left">{{template "glyph-align-left"}}</button>
+<button type="button" data-tool rst-tip data-cmd="align" data-value="center" aria-pressed="false" aria-label="Align centre" data-tip="Align centre">{{template "glyph-align-center"}}</button>
+<button type="button" data-tool rst-tip data-cmd="align" data-value="right" aria-pressed="false" aria-label="Align right" data-tip="Align right">{{template "glyph-align-right"}}</button>
</div>
{{end}}
@@ -355,7 +374,11 @@ read every cell, edit any of them, one page at a time.</p>
<nav data-sheet-tabs aria-label="Sheets in this document">
<a href="/sheets/{{.Data.Sheet.ID}}" aria-current="page">{{.Data.Sheet.TabName}}</a>
</nav>
-<span data-selection-size role="status"></span>
+{{/* Deliberately NOT role="status". It changes on every arrow key, and
+ a live region announcing "3 cells" after each one talks over the
+ cell content a screen reader is trying to read. It is a glance,
+ and the selection itself is what announces. */}}
+<span data-selection-size></span>
<span data-sheet-live data-state="off" role="status">Connecting</span>
</div>
{{end}}
diff --git a/internal/sheets/xlsx/sizes.go b/internal/sheets/xlsx/sizes.go
index 4e7bf62..2d5295c 100644
--- a/internal/sheets/xlsx/sizes.go
+++ b/internal/sheets/xlsx/sizes.go
@@ -2,6 +2,7 @@ package xlsx
import (
"math"
+ "sort"
"github.com/xuri/excelize/v2"
@@ -95,8 +96,18 @@ func importSizes(f *excelize.File, sheet string, h *store.Handle, rows, cols int
// probeDistance is how far past the used range to look for a sheet's
// default size.
//
-// Far enough that no real file has an explicit width out there, near
-// enough to stay inside the format's limits.
+// Far enough that a file written by Excel has no explicit width out
+// there, near enough to stay inside the format's limits.
+//
+// **It is not far enough for every producer, and cannot be.**
+// LibreOffice writes a single `<col min="1" max="1025">` covering the
+// whole sheet, so the probe lands inside it and reads that file's
+// uniform width as the default — with the result that a uniformly
+// widened LibreOffice file imports with no widths at all. That is the
+// safe direction to be wrong in (nothing invented, the app's own
+// default used) and the alternative is reading excelize's internals to
+// distinguish a span from a default, which is a coupling to a library
+// version rather than to a format. Named rather than papered over.
const probeDistance = 64
// defaultColWidth asks the file what an UNTOUCHED column is.
@@ -151,7 +162,11 @@ func exportSizes(f *excelize.File, sheet string, h *store.Handle) error {
if err != nil {
return err
}
- for pos, px := range cols {
+ // Sorted, so two exports of an unchanged tab produce identical
+ // bytes. Go randomises map iteration, which would otherwise reorder
+ // the <cols> block on every run and make a corpus diff unreadable.
+ for _, pos := range sortedKeys(cols) {
+ px := cols[pos]
name, err := excelize.ColumnNumberToName(pos + 1)
if err != nil {
continue
@@ -167,10 +182,20 @@ func exportSizes(f *excelize.File, sheet string, h *store.Handle) error {
return err
}
}
- for pos, px := range rows {
+ for _, pos := range sortedKeys(rows) {
+ px := rows[pos]
if err := f.SetRowHeight(sheet, pos+1, math.Round(float64(px)/pointsToPixels*100)/100); err != nil {
return err
}
}
return nil
}
+
+func sortedKeys(m map[int]int) []int {
+ out := make([]int, 0, len(m))
+ for k := range m {
+ out = append(out, k)
+ }
+ sort.Ints(out)
+ return out
+}
diff --git a/tools/drive/chrome.mjs b/tools/drive/chrome.mjs
index 9f1351c..65083a8 100644
--- a/tools/drive/chrome.mjs
+++ b/tools/drive/chrome.mjs
@@ -124,6 +124,29 @@ check('...and the CSS actually applies it', parseInt(hdr.weight, 10) >= 600, `fo
const pressed = await page.getAttribute('[data-toolbar] [data-cmd="bold"]', 'aria-pressed');
check('the toolbar reflects the selection', pressed === 'true', `aria-pressed=${pressed}`);
+/* Alignment. This check exists because its absence let a blocker
+ * through: the library writes text-align as an INLINE style on every
+ * cell, so the attribute was set, aria-pressed reflected it, and the
+ * text did not move. Bold and fill both passed, because neither paints
+ * a property the library touches. Assert the COMPUTED value, never the
+ * attribute — the attribute was always right. */
+await page.click('[data-toolbar] [data-cmd="align"][data-value="right"]');
+await page.waitForTimeout(400);
+const aligned = await page.evaluate(() =>
+ getComputedStyle(window.SheetsGrid.worksheet().records[0][0].element).textAlign);
+check('alignment actually moves the text', aligned === 'right' || aligned === 'end', `computed ${aligned}`);
+await page.click('[data-toolbar] [data-cmd="align"][data-value="right"]');
+await page.waitForTimeout(400);
+
+/* Numbers right-align without anybody asking. A spreadsheet that
+ * centres its numbers does not look like one. */
+const numAlign = await page.evaluate(() =>
+ getComputedStyle(window.SheetsGrid.worksheet().records[1][2].element).textAlign);
+check('numbers right-align on their own', numAlign === 'right' || numAlign === 'end', `computed ${numAlign}`);
+const textAlign = await page.evaluate(() =>
+ getComputedStyle(window.SheetsGrid.worksheet().records[1][1].element).textAlign);
+check('...and text does not', textAlign === 'left' || textAlign === 'start', `computed ${textAlign}`);
+
/* Fill: a swatch by name, resolved against the theme's lightness. */
await page.evaluate(() => window.SheetsGrid.worksheet().updateSelectionFromCoords(1, 4, 3, 4));
await page.waitForTimeout(150);
@@ -161,6 +184,16 @@ await page.waitForTimeout(250);
const jumped = await page.evaluate(() => { const s = window.SheetsGrid.selection(); return `${s.r},${s.c}`; });
check('typing a reference moves the selection', jumped === '1,1', `selection is ${jumped}`);
+/* The anchor cursor has to survive a fill. It used to be marked by a
+ * background alone, and the fill rules land later at the same
+ * specificity — so on a filled range the cursor vanished at exactly the
+ * moment you were aiming with it. */
+await page.evaluate(() => window.SheetsGrid.worksheet().updateSelectionFromCoords(1, 4, 3, 4));
+await page.waitForTimeout(300);
+const anchorRing = await page.evaluate(() =>
+ getComputedStyle(window.SheetsGrid.worksheet().records[4][1].element).boxShadow);
+check('the anchor is visible on a filled cell', anchorRing && anchorRing !== 'none', `box-shadow ${anchorRing}`);
+
console.log('\n— column widths');
/* Resize through the library the way a drag does, then prove it survived
* a reload: a width the server did not keep looks identical until the