oficina / sheets Public

Show comment text on the comments page

Each comment entry rendered inside rastrillo's rst-tblock, a toggle-block
whose body is display:none until a switch in its head is checked. Comments
have no switch, so every thread showed an author, a time and an empty box.
Entries are now plain always-open blocks inside the thread's rst-box.

Tests: TestCommentTextIsNotInACollapsedToggleBlock (make test) and
tools/drive/comments.mjs, which loads the real rendered page and
stylesheets in Chromium and WebKit at desktop and phone widths and asserts
each body is visible. Both fail on main and pass here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 3 weeks ago 2e9f3bc3a5185591954b896e473bee332a31d082 parent 336aa0d

5 files changed, +129 −6

internal/sheets/browser_comments_test.go +44 −0
internal/sheets/handlers_comments_test.go +19 −0
internal/sheets/static/sheets.css +22 −0
internal/sheets/templates/comments.html +7 −6
tools/drive/comments.mjs +37 −0

CI passed — run details

CI log

amadan: .amadan/timeout: this job's budget is 15m0s (this runner allows up to 30m0s)
=== vet ===
CGO_ENABLED=0 go vet ./...
=== fmt ===
=== build ===
CGO_ENABLED=0 go build ./...
=== migrations ===
rastrillo migration check
rastrillo migration check: models and migrations agree
=== go-test ===
CGO_ENABLED=0 go test -count=1 ./...
?   	amadan.net/oficina/sheets/cmd/sheets	[no test files]
ok  	amadan.net/oficina/sheets/internal/guardstest	0.084s
ok  	amadan.net/oficina/sheets/internal/sheets	10.593s
ok  	amadan.net/oficina/sheets/internal/sheets/calc	0.008s
ok  	amadan.net/oficina/sheets/internal/sheets/format	0.004s
ok  	amadan.net/oficina/sheets/internal/sheets/live	0.053s
ok  	amadan.net/oficina/sheets/internal/sheets/seal	0.003s
ok  	amadan.net/oficina/sheets/internal/sheets/sealtest	0.004s
ok  	amadan.net/oficina/sheets/internal/sheets/store	2.660s
ok  	amadan.net/oficina/sheets/internal/sheets/xlsx	0.269s
?   	amadan.net/oficina/sheets/tools/gencorpus	[no test files]
=== race ===
go test -race -count=1 ./internal/sheets/store/... ./internal/sheets/live/...
ok  	amadan.net/oficina/sheets/internal/sheets/store	45.832s
ok  	amadan.net/oficina/sheets/internal/sheets/live	1.059s

diff --git a/internal/sheets/browser_comments_test.go b/internal/sheets/browser_comments_test.go

new file mode 100644

index 0000000..a4e3eee

--- /dev/null

+++ b/internal/sheets/browser_comments_test.go

@@ -0,0 +1,44 @@

+package sheets

+import (

+ "encoding/json"

+ "net/http"

+ "net/http/httptest"

+ "os"

+ "regexp"

+ "testing"

+// Opt-in fixture for tools/drive/comments.mjs: the real server-rendered

+// comments page and its stylesheets, with a synthetic owner, sheet and

+// comment. No session cookie is exported.

+func TestBrowserCommentsFixture(t *testing.T) {

+ path := os.Getenv("SHEETS_COMMENTS_FIXTURE")

+ if path == "" {

+ t.Skip("set SHEETS_COMMENTS_FIXTURE to render the browser fixture")

+ }

+ app, cookies := signIn(t)

+ s, _ := newSheet(t, app, cookies, "Budget")

+ addComment(t, app, s, cookies, 0, 0, "Check this total")

+ addComment(t, app, s, cookies, 0, 0, "Checked, it matches the ledger")

+ page := do(t, app, as(httptest.NewRequest(http.MethodGet, commentsPath(s), nil), cookies))

+ if page.Code != http.StatusOK {

+ t.Fatalf("comments page: %d", page.Code)

+ }

+ assets := map[string]string{}

+ for _, match := range regexp.MustCompile(`(?:src|href)="(/static/[^" ]+)"`).FindAllStringSubmatch(page.Body.String(), -1) {

+ asset := do(t, app, as(httptest.NewRequest(http.MethodGet, match[1], nil), cookies))

+ if asset.Code != http.StatusOK {

+ t.Fatalf("asset %s: %d", match[1], asset.Code)

+ }

+ assets[match[1]] = asset.Body.String()

+ }

+ data, err := json.Marshal(map[string]any{"html": page.Body.String(), "assets": assets,

+ "bodies": []string{"Check this total", "Checked, it matches the ledger"}})

+ if err != nil {

+ t.Fatal(err)

+ }

+ if err := os.WriteFile(path, data, 0600); err != nil {

+ t.Fatal(err)

+ }

diff --git a/internal/sheets/handlers_comments_test.go b/internal/sheets/handlers_comments_test.go

index c94f15f..82b9e5b 100644

--- a/internal/sheets/handlers_comments_test.go

+++ b/internal/sheets/handlers_comments_test.go

@@ -6,6 +6,7 @@ import (

"net/http"

"net/http/httptest"

"net/url"

+ "regexp"

"strings"

"testing"

)

@@ -243,3 +244,21 @@ func TestTheResolveFormPostsPositionsNotIDs(t *testing.T) {

body[max(0, strings.Index(body, "resolve")-300):])

}

+// B6: comment text rendered inside rst-tblock, a toggle-block whose body

+// is display:none until a switch in its head is checked. Comments have no

+// switch, so every thread showed an author, a time and an empty box. The

+// visible-in-a-browser proof is tools/drive/comments.mjs; this keeps the

+// markup from drifting back into a collapsed component in make test.

+func TestCommentTextIsNotInACollapsedToggleBlock(t *testing.T) {

+ app, cookies := signIn(t)

+ s, _ := newSheet(t, app, cookies, "Budget")

+ addComment(t, app, s, cookies, 0, 0, "Check this total")

+ body := commentsBody(t, app, s, cookies)

+ if strings.Contains(body, "rst-tblock") {

+ t.Error("comment entries render inside rst-tblock, whose body is hidden without a checked switch")

+ }

+ if !regexp.MustCompile(`<div data-comment>\s*<p><strong>[^<]+</strong>[\s\S]*?<p>Check this total</p>`).MatchString(body) {

+ t.Error("the comment text is not rendered as an always-open entry")

+ }

diff --git a/internal/sheets/static/sheets.css b/internal/sheets/static/sheets.css

index 294690c..c3e940a 100644

--- a/internal/sheets/static/sheets.css

+++ b/internal/sheets/static/sheets.css

@@ -1437,3 +1437,25 @@ form[data-doc-title]:not(:focus-within) button {

outline: 2px solid var(--rst-accent);

outline-offset: -2px;

}

+/* One entry in a comment thread (comments.html): author and time, then

+ the text, always shown. Entries after the first get a rule between. */

+[data-comment] > p {

+ margin: 0;

+[data-comment] > p + p {

+ margin-block-start: var(--rst-sp-1);

+ white-space: pre-line;

+[data-comment] + [data-comment] {

+ border-block-start: 1px solid var(--rst-line);

+ margin-block-start: var(--rst-sp-3);

+ padding-block-start: var(--rst-sp-3);

+[data-comment] [rst-row-sub] {

+ display: inline;

+ margin-inline-start: var(--rst-sp-2);

diff --git a/internal/sheets/templates/comments.html b/internal/sheets/templates/comments.html

index fe86cb4..d912de7 100644

--- a/internal/sheets/templates/comments.html

+++ b/internal/sheets/templates/comments.html

@@ -44,13 +44,14 @@ addresses them: row 0, column 0 is A1.</p>

{{if .Resolved}}<span rst-badge="neutral">resolved</span>{{end}}

</div>

+{{/* Always open. Not rst-tblock: that is a toggle-block whose body

+ stays display:none until a switch in its head is checked, and a

+ comment has no switch, so its text never showed. */}}

-<div rst-tblock>

-<div rst-tblock-head>

-<span rst-tblock-title>{{.Author}}</span>

-<span rst-row-sub>{{.When.Format "2 Jan 2006, 15:04"}}</span>

-</div>

-<div rst-tblock-body><p>{{.Body}}</p></div>

+<div data-comment>

+<p><strong>{{.Author}}</strong>

+<span rst-row-sub>{{.When.Format "2 Jan 2006, 15:04"}}</span></p>

+<p>{{.Body}}</p>

</div>

diff --git a/tools/drive/comments.mjs b/tools/drive/comments.mjs

new file mode 100644

index 0000000..de29a60

--- /dev/null

+++ b/tools/drive/comments.mjs

@@ -0,0 +1,37 @@

+// Comment bodies are visible on the comments page (B6: they rendered inside a

+// rastrillo toggle-block, whose body is display:none without a checked switch).

+// Real server-rendered page and stylesheets, mocked transport; no live data.

+// SHEETS_COMMENTS_FIXTURE=/tmp/comments.json go test ./internal/sheets -run TestBrowserCommentsFixture -count=1

+// PLAYWRIGHT=/path/to/playwright/index.mjs SHEETS_COMMENTS_FIXTURE=/tmp/comments.json node tools/drive/comments.mjs

+import assert from 'node:assert/strict';

+import fs from 'node:fs';

+const pw = await import(process.env.PLAYWRIGHT || 'playwright');

+const fixture = JSON.parse(fs.readFileSync(process.env.SHEETS_COMMENTS_FIXTURE));

+const origin = 'https://sheets.test';

+for (const name of ['chromium', 'webkit']) {

+ const options = name === 'webkit' && process.env.WEBKIT_EXECUTABLE_PATH ? {executablePath:process.env.WEBKIT_EXECUTABLE_PATH} : {};

+ const browser = await pw[name].launch(options);

+ try {

+ for (const viewport of [{width:1440,height:900},{width:390,height:844}]) {

+ const page = await browser.newPage({viewport});

+ await page.route(origin+'/**', async route => {

+ const path = new URL(route.request().url()).pathname;

+ if (path in fixture.assets) return route.fulfill({body:fixture.assets[path],contentType:

+ path.endsWith('.js')?'application/javascript':path.endsWith('.css')?'text/css':'image/svg+xml'});

+ return route.fulfill({body:fixture.html,contentType:'text/html'});

+ });

+ await page.goto(origin+'/sheets/1/comments');

+ for (const text of fixture.bodies) {

+ const body = page.getByText(text, {exact:true});

+ assert.equal(await body.count(), 1, `${name}: "${text}" rendered once`);

+ assert.ok(await body.isVisible(), `${name} ${viewport.width}px: "${text}" is visible`);

+ const box = await body.boundingBox();

+ assert.ok(box && box.width > 0 && box.height > 0, `${name}: "${text}" has a painted box`);

+ }

+ await page.close();

+ }

+ console.log(`${name}: comment bodies visible`);

+ } finally {

+ await browser.close();

+ }