Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions scripts/test-spaces-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3594,5 +3594,46 @@ function fsTable(f: string): string {
}


// ---- ONE BRANCH PER LAYOUT -------------------------------------------------
// A rebase duplicated a whole `if (layout === 'table')` block into render.ts.
// Both copies compiled, both were valid, and the FIRST one returned — so the
// second, which is the one carrying #390's sortable headers and editable
// cells, was unreachable. Click-to-sort and edit-in-place shipped to main and
// silently did nothing; the table rendered, so nothing looked broken.
//
// It survived review twice. The rig that was supposed to cover #390 checked
// editor.ts for the click WIRING, which exists and is correct — the handler
// was fine, the markup it needed was never rendered. That is a source grep
// answering a question about the wrong file.
//
// The same commit also duplicated a whole section of THIS rig, which was
// caught and removed. So the class is: a rebase of a stacked branch onto a
// squashed base silently duplicates a block. Count the dispatch branches;
// duplicates are the thing to fail on, not any one of their contents.
{
const fs = await import('node:fs')
const ren = fs.readFileSync(new URL('../spaces/src/render.ts', import.meta.url), 'utf8')

const seen = new Map<string, number>()
for (const [, word] of ren.matchAll(/layout === '([a-z]+)'/g)) {
seen.set(word, (seen.get(word) ?? 0) + 1)
}
const dupes = [...seen].filter(([, n]) => n > 1).map(([w, n]) => `${w}×${n}`)
ok(dupes.length === 0,
`each layout is dispatched from exactly ONE branch in render.ts${dupes.length ? ' — duplicated: ' + dupes.join(', ') : ''}`)

// …and the branch that survives is the one that can actually sort and edit.
// Deleting the wrong copy of a duplicate pair passes the count check above
// and loses the feature, so name what the surviving branch must contain.
// \b, not a bare substring: the first draft of this check was /sortCol/, and
// renaming the property to `sortColX` — which is exactly what deleting the
// wrong half of the pair looks like — left it GREEN. The sabotage found my
// assertion, not the code.
ok(/\bdataset\.sortCol\b/.test(ren),
'the surviving table branch renders sortable headers')
ok(seen.get('table') === 1, 'exactly one table branch, so that header markup is reachable')
}


console.log(`\n${checks - failures}/${checks} checks passed`)
if (failures) process.exit(1)
9 changes: 9 additions & 0 deletions spaces/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ Versions follow `0.MINOR.PATCH` while pre-1.0.

## [Unreleased]

- **The table sorts and edits again.** A rebase duplicated the whole
`layout === 'table'` branch in the renderer. Both copies compiled, and the
first one returned — so the second, the one carrying click-to-sort headers
and edit-in-place cells, was unreachable from the moment covers and the
gallery landed. The table still drew, so nothing looked broken; it was
simply read-only and unsortable. Measured in the built shell before the fix:
0 header buttons, 0 cell buttons, no sort attribute anywhere. After: 2 and 6,
and clicking a column header actually reorders the rows.

- **The whole gallery card is the target, and a long title stops inflating its
row.** In a shelf of covers the picture is what you point at, so the title's
link now stretches over the card rather than the card holding a second one —
Expand Down
68 changes: 0 additions & 68 deletions spaces/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1281,74 +1281,6 @@ function renderView(host: HTMLElement, b: Block, doc: SpacesDoc, opts: RenderOpt
return a
}

// TABLE — the shape a base is usually looked at in, and the one this app did
// not have. Columns are the fields the ROWS ACTUALLY CARRY, in the schema's
// declared order: a table of books should not carry an Estimate column
// because the vocabulary happens to contain one, and a page that has a field
// the others lack should not be the reason everyone gets an empty column.
if (layout === 'table') {
const keys = fieldsOf(doc).map((f) => f.key).filter((k) => rows.some((r) => r.values.has(k)))
const wrap = document.createElement('div')
// its own scroller: a wide table must not make the PAGE scroll sideways
wrap.className = 'sp-view-tablewrap'
const table = document.createElement('table')
table.className = 'sp-view-table'
const thead = document.createElement('thead')
const hr = document.createElement('tr')
const th0 = document.createElement('th')
th0.textContent = t('Page')
hr.appendChild(th0)
for (const k of keys) {
const th = document.createElement('th')
th.textContent = fieldByKey(doc, k)?.label ?? k
hr.appendChild(th)
}
thead.appendChild(hr)
table.appendChild(thead)
const tb = document.createElement('tbody')
for (const r of rows) {
const tr = document.createElement('tr')
const td0 = document.createElement('td')
// the page itself, reached the same way a card reaches it
const a = document.createElement('a')
a.className = 'sp-view-cellink'
a.href = `#p/${r.page.id}`
a.dataset.page = r.page.id
a.textContent = r.page.title || t('Untitled')
td0.appendChild(a)
tr.appendChild(td0)
for (const k of keys) {
const td = document.createElement('td')
const f = fieldByKey(doc, k)
const v = r.values.get(k)
// THROUGH THE OPTION, so a select shows its label and its colour rather
// than the id the model stores — the same thing propHtml does for the
// header strip, and for the same reason: the id is not for reading.
const opt = optionOf(f, v)
if (opt) {
const chip = document.createElement('span')
chip.className = 'sp-prop-chip'
const dot = document.createElement('span')
dot.className = 'sp-prop-dot'
if (opt.color) dot.style.background = opt.color
chip.append(dot, document.createTextNode(opt.label))
td.appendChild(chip)
} else if (v !== undefined && v !== null && String(v) !== '') {
td.textContent = String(v)
} else {
td.className = 'sp-view-empty'
td.textContent = '—'
}
tr.appendChild(td)
}
tb.appendChild(tr)
}
table.appendChild(tb)
wrap.appendChild(table)
host.appendChild(wrap)
return
}

// GALLERY — the shape that makes a reading list look like a reading list. A
// board answers "what state is this in" and a table answers "what does it
// say"; a gallery answers "which one is it", which for books, films, recipes
Expand Down
Loading