feat(board): one card model and one card component for both boards (#1265)

changelog.d/1265-board-cards.added.md

@@ -0,0 +1 @@
+The Delivery board and the task board now draw their cards through one component, and a card says which side it came from: a repository card is keyed `owner/name#number` — one key for an issue and the pull request that closes it — and a task card `task:<id>` (#1265, first step).

core/src/client/board-card.js

@@ -0,0 +1,62 @@
+// One card component behind both boards (#1265, step 1). A delivery card and a task card draw the
+// same pieces — an identity line, a title, what it wears, who has it, what it is doing — so they
+// render through this one component with the `source` their card model carries.
+//
+// `skin` is the calling board's class vocabulary. Two skins, not a third look: the delivery board
+// keeps its stylesheet and the task board keeps its own while the shape is shared, and moving the
+// columns into one set (step 2) no longer has to touch either stylesheet's card markup.
+import { createElement as h } from 'react'
+import { t } from './i18n.js'
+
+export const DELIVERY_SKIN = {
+  card: 'bt-del-card', top: 'bt-del-card-top', id: 'bt-del-id', badge: 'bt-del-badge',
+  title: 'bt-del-title', labels: 'bt-del-labels', label: 'bt-del-label', owner: 'bt-del-owner',
+  linked: 'bt-del-linked', sha: 'bt-del-sha',
+}
+
+export const TASK_SKIN = {
+  card: 'bt-board-card', top: 'bt-board-card-top', id: 'bt-board-id', badge: 'bt-board-badge',
+  title: 'bt-board-title', labels: 'bt-board-labels', label: 'bt-board-label', owner: 'bt-board-who',
+  linked: 'bt-board-linked', sha: 'bt-board-sha', note: 'bt-board-note',
+}
+
+// `attrs` is the board's own extra attributes (a task's `data-column`, a card's `data-number`), so
+// the existing selectors and tests keep reading the same elements.
+export function BoardCard({
+  card, skin = DELIVERY_SKIN, attrs = {}, idText = '', topExtra = null, badge = '',
+  meta = null, note = '', footer = null, actions = null, emptyTitle = '',
+  draggable = false, dragValue = '', onDragStart = null, onDragEnd = null,
+}) {
+  const labels = Array.isArray(card?.labels) ? card.labels : []
+  return h('article', {
+    ...attrs,
+    className: skin.card,
+    'data-source': card?.source,
+    'data-key': card?.key ?? undefined,
+    draggable: draggable || undefined,
+    onDragStart: draggable
+      ? (event) => {
+          event.dataTransfer?.setData('text/plain', dragValue)
+          if (event.dataTransfer) event.dataTransfer.effectAllowed = 'move'
+          onDragStart?.()
+        }
+      : undefined,
+    onDragEnd: draggable ? () => onDragEnd?.() : undefined,
+  },
+  h('span', { className: skin.top },
+    h('span', { className: skin.id }, idText),
+    topExtra,
+    badge ? h('span', { className: skin.badge, 'data-badge': card?.badge ?? undefined }, badge) : null),
+  h('span', { className: skin.title }, card?.title || emptyTitle),
+  labels.length
+    ? h('span', { className: skin.labels }, labels.map(name => h('span', { key: name, className: skin.label, 'data-label': name }, name)))
+    : null,
+  card?.owner ? h('span', { className: skin.owner, 'data-kind': card.ownerKind ?? 'human' }, card.owner) : null,
+  note ? h('span', { className: skin.note ?? skin.linked }, note) : null,
+  meta,
+  card?.linked ? h('span', { className: skin.linked }, t('linked to #{number}', { number: card.linked })) : null,
+  card?.sha ? h('span', { className: skin.sha }, card.sha) : null,
+  card?.url ? h('a', { className: skin.sha, href: card.url, target: '_blank', rel: 'noreferrer' }, t('open on GitHub')) : null,
+  footer,
+  actions)
+}

core/src/client/delivery-view.js

@@ -9,6 +9,8 @@
 import { createElement as h, useEffect, useState } from 'react'
 import { t } from './i18n.js'
 import { DeliveryMetrics, DELIVERY_CHART_CSS, todayPartial } from './delivery-chart.js'
+import { githubCard } from '../shared/board-card.js'
+import { BoardCard, DELIVERY_SKIN } from './board-card.js'
 
 const POLL_MS = 60_000
 // While the host is still building there is nothing to see, so ask often; once the board is up,
@@ -391,7 +393,7 @@ export function DeliveryBoard({ view, snapshot, message = '', repo = null, onRet
           h('span', { className: 'bt-board-count' }, String((columns[column.id] ?? []).length))),
         (columns[column.id] ?? []).length === 0
           ? h('p', { className: 'bt-board-empty' }, t('Nothing here.'))
-          : (columns[column.id] ?? []).map(card => h(DeliveryCard, { key: `${card.kind}-${card.number}`, card }))))))
+          : (columns[column.id] ?? []).map(card => h(DeliveryCard, { key: `${card.kind}-${card.number}`, card, repo: payload.repo }))))))
 }
 
 export function DeliveryView({ actions }) {
@@ -404,22 +406,17 @@ export function DeliveryView({ actions }) {
   })
 }
 
-function Label({ name }) {
-  return h('span', { className: 'bt-del-label', 'data-label': name }, name)
-}
-
-export function DeliveryCard({ card }) {
-  const badge = badgeLabel(card?.badge)
-  return h('article', { className: 'bt-del-card', 'data-kind': card?.kind, 'data-number': card?.number, 'data-badge': card?.badge ?? undefined },
-    h('span', { className: 'bt-del-card-top' },
-      h('span', { className: 'bt-del-id' }, `#${card?.number ?? '?'}`),
-      badge ? h('span', { className: 'bt-del-badge', 'data-badge': card?.badge }, badge) : null),
-    h('span', { className: 'bt-del-title' }, card?.title ?? ''),
-    card?.labels?.length ? h('span', { className: 'bt-del-labels' }, card.labels.map(name => h(Label, { key: name, name }))) : null,
-    card?.owner ? h('span', { className: 'bt-del-owner', 'data-kind': card.ownerKind ?? 'human' }, card.owner) : null,
-    card?.linked ? h('span', { className: 'bt-del-linked' }, t('linked to #{number}', { number: card.linked })) : null,
-    card?.sha ? h('span', { className: 'bt-del-sha' }, card.sha) : null,
-    card?.url ? h('a', { className: 'bt-del-sha', href: card.url, target: '_blank', rel: 'noreferrer' }, t('open on GitHub')) : null)
+// A repository row as a card on the one board (#1265 step 1): the same component the task cards
+// use, keyed `owner/name#number` so an issue and the pull request that closes it are one identity.
+export function DeliveryCard({ card, repo = null }) {
+  const model = githubCard(card, repo) ?? { source: 'github', key: null, title: card?.title ?? '', badge: card?.badge ?? null }
+  return h(BoardCard, {
+    card: model,
+    skin: DELIVERY_SKIN,
+    attrs: { 'data-kind': card?.kind, 'data-number': card?.number, 'data-badge': card?.badge ?? undefined },
+    idText: `#${card?.number ?? '?'}`,
+    badge: badgeLabel(card?.badge),
+  })
 }
 
 export const DELIVERY_CSS = `

core/src/client/kanban.js

@@ -3,6 +3,8 @@
 // owner. Tasks opens one Bot's board, a child of that Bot, or a group an admin can see.
 import { createElement as h, useCallback, useEffect, useState } from 'react'
 import { columnOf, priorityOf, progressOf } from '../shared/task-board.js'
+import { taskCard } from '../shared/board-card.js'
+import { BoardCard as BoardCardFrame, TASK_SKIN } from './board-card.js'
 import { CloseIcon } from './icons.js'
 import { useEscape } from './overlay-hooks.js'
 import { DeliveryView, DELIVERY_CSS } from './delivery-view.js'
@@ -193,44 +195,55 @@ function RailButton({ row, selected, meta, onSelect }) {
   meta ? h('span', { className: 'bt-board-rail-meta' }, meta) : null)
 }
 
+// A task record as a card on the one board (#1265 step 1): the same component the delivery cards
+// use, with the task's own attributes and actions hanging off it.
 export function BoardCard({ task, roster, movable, onMove, onDrag, partOf }) {
   const column = columnOf(task)
   const priority = priorityOf(task)
   const progress = progressOf(task)
   const who = botName(roster, task)
   const question = task.waitingFor === 'input' ? task.inputRequest?.question : ''
   const overall = task.kind === 'group'
-  return h('article', {
-    className: 'bt-board-card',
-    'data-task': task.id,
-    'data-column': column,
-    'data-priority': priority,
-    'data-kind': overall ? 'group' : undefined,
-    draggable: movable || undefined,
-    onDragStart: (event) => {
-      event.dataTransfer?.setData('text/plain', task.id)
-      if (event.dataTransfer) event.dataTransfer.effectAllowed = 'move'
-      onDrag?.(task.id)
+  return h(BoardCardFrame, {
+    card: taskCard(task),
+    skin: TASK_SKIN,
+    attrs: {
+      'data-task': task.id,
+      'data-column': column,
+      'data-priority': priority,
+      'data-kind': overall ? 'group' : undefined,
     },
+    idText: shortId(task.id),
+    emptyTitle: t('Untitled task'),
+    topExtra: [
+      h('span', { key: 'priority', className: 'bt-board-priority', 'data-priority': priority }, priorityLabel(priority)),
+      h('span', { key: 'status', className: 'bt-board-status' }, statusLabel(task.status)),
+    ],
+    note: task.note ?? '',
+    meta: [
+      overall ? h('span', { key: 'kind', className: 'bt-board-kind' }, t('Group task')) : null,
+      partOf ? h('span', { key: 'part', className: 'bt-board-part' }, t('Part of {title}', { title: partOf })) : null,
+      question ? h('span', { key: 'wait', className: 'bt-board-wait' }, question) : null,
+    ],
+    footer: h('span', { className: 'bt-board-foot' },
+      who ? h('span', { className: 'bt-board-who' }, who) : null,
+      h('span', { className: 'bt-board-progress' },
+        h('span', { className: 'bt-board-progress-bar', role: 'progressbar', 'aria-valuemin': 0, 'aria-valuemax': 100, 'aria-valuenow': progress, 'aria-label': t('Progress') },
+          h('span', { style: { width: `${progress}%` } })),
+        h('span', { className: 'bt-board-percent' }, t('{percent}%', { percent: progress })))),
+    actions: [
+      movable && column === 'not-started'
+        ? h('button', { key: 'start', type: 'button', className: 'bt-soft bt-soft-sm', onClick: () => onMove?.(task.id, 'in-progress') }, t('Start'))
+        : null,
+      movable && column === 'in-progress' && task.status !== 'blocked'
+        ? h('button', { key: 'complete', type: 'button', className: 'bt-soft bt-soft-sm', onClick: () => onMove?.(task.id, 'done') }, t('Complete'))
+        : null,
+    ],
+    draggable: movable,
+    dragValue: task.id,
+    onDragStart: () => onDrag?.(task.id),
     onDragEnd: () => onDrag?.(null),
-  },
-  h('span', { className: 'bt-board-card-top' },
-    h('span', { className: 'bt-board-id' }, shortId(task.id)),
-    h('span', { className: 'bt-board-priority', 'data-priority': priority }, priorityLabel(priority)),
-    h('span', { className: 'bt-board-status' }, statusLabel(task.status))),
-  h('span', { className: 'bt-board-title' }, task.title || t('Untitled task')),
-  overall ? h('span', { className: 'bt-board-kind' }, t('Group task')) : null,
-  partOf ? h('span', { className: 'bt-board-part' }, t('Part of {title}', { title: partOf })) : null,
-  task.note ? h('span', { className: 'bt-board-note' }, task.note) : null,
-  question ? h('span', { className: 'bt-board-wait' }, question) : null,
-  h('span', { className: 'bt-board-foot' },
-    who ? h('span', { className: 'bt-board-who' }, who) : null,
-    h('span', { className: 'bt-board-progress' },
-      h('span', { className: 'bt-board-progress-bar', role: 'progressbar', 'aria-valuemin': 0, 'aria-valuemax': 100, 'aria-valuenow': progress, 'aria-label': t('Progress') },
-        h('span', { style: { width: `${progress}%` } })),
-      h('span', { className: 'bt-board-percent' }, t('{percent}%', { percent: progress })))),
-  movable && column === 'not-started' ? h('button', { type: 'button', className: 'bt-soft bt-soft-sm', onClick: () => onMove?.(task.id, 'in-progress') }, t('Start')) : null,
-  movable && column === 'in-progress' && task.status !== 'blocked' ? h('button', { type: 'button', className: 'bt-soft bt-soft-sm', onClick: () => onMove?.(task.id, 'done') }, t('Complete')) : null)
+  })
 }
 
 function cardProps(task, roster, onMove, onDrag, partOf) {

core/src/shared/board-card.js

@@ -0,0 +1,77 @@
+// One card behind two boards (#1265, step 1). The delivery board reads GitHub, the task board
+// reads FlyBot's task records; on screen they are the same object, so a card says which side it
+// came from and carries a key that is unique across both.
+//
+// A repository card is keyed by `owner/name#number`, not by kind: one issue is one card whether the
+// row arrived as an issue or as the pull request that closes it, which is what lets step 3 hang a
+// FlyBot record under the issue it belongs to. A task keeps its own id under `task:`. Nothing here
+// fetches, writes or reads a clock.
+
+export const CARD_SOURCES = ['github', 'task']
+
+export function githubCardKey(repo, number) {
+  const name = typeof repo === 'string' ? repo.trim() : ''
+  const value = Number(number)
+  if (name === '' || !Number.isInteger(value) || value <= 0) return null
+  return `${name}#${value}`
+}
+
+export function taskCardKey(id) {
+  const text = String(id ?? '').trim()
+  return text === '' ? null : `task:${text}`
+}
+
+const repoOf = (repo) => (typeof repo === 'string' && repo.trim() !== '' ? repo.trim() : null)
+
+// A delivery-board row (`cardOf` in src/host/delivery-board.js) as a card. Without a repository
+// there is no cross-repository identity, so the key falls back to the row's kind and number
+// instead of implying one.
+export function githubCard(row, repo) {
+  const number = Number(row?.number)
+  if (!Number.isInteger(number) || number <= 0) return null
+  const name = repoOf(repo)
+  return {
+    source: 'github',
+    key: githubCardKey(name, number) ?? `github:${row.kind ?? 'item'}:${number}`,
+    repo: name,
+    number,
+    kind: row.kind ?? null,
+    title: String(row.title ?? ''),
+    badge: row.badge ?? null,
+    labels: Array.isArray(row.labels) ? row.labels.slice() : [],
+    owner: row.owner ?? null,
+    ownerKind: row.ownerKind ?? null,
+    linked: row.linked ?? null,
+    sha: row.sha ?? null,
+    url: row.url ?? null,
+  }
+}
+
+// A task record (`src/host/tasks.js`) as a card. The record's own fields stay on the record: the
+// card carries what it draws, and the view reads the rest from the task it was built from.
+export function taskCard(task) {
+  const key = taskCardKey(task?.id)
+  if (key === null) return null
+  return {
+    source: 'task',
+    key,
+    id: String(task.id),
+    repo: repoOf(task.repo),
+    number: null,
+    kind: 'task',
+    title: String(task.title ?? ''),
+    badge: null,
+    labels: [],
+    owner: null,
+    ownerKind: null,
+    linked: null,
+    sha: null,
+    url: null,
+  }
+}
+
+// Two cards are the same card when they answer the same key, which is the identity step 3 hangs a
+// task record on. Kept here so both sides dedupe the same way.
+export function sameCard(a, b) {
+  return Boolean(a?.key) && a.key === b?.key
+}

core/test/board-card.test.js

@@ -0,0 +1,92 @@
+// One card behind two boards (#1265, step 1): the identity a card carries, and the one component
+// that draws a GitHub card and a task card. The view tests next door prove each board emits it;
+// this file proves the identity rules and the component itself, without React.
+import assert from 'node:assert/strict'
+import { test } from 'node:test'
+import { fileURLToPath } from 'node:url'
+import { runInNewContext } from 'node:vm'
+import { build } from 'esbuild'
+import { CARD_SOURCES, githubCard, githubCardKey, sameCard, taskCard, taskCardKey } from '../src/shared/board-card.js'
+
+const bundle = await build({
+  entryPoints: [fileURLToPath(new URL('../src/client/board-card.js', import.meta.url))],
+  bundle: true, format: 'cjs', platform: 'node', write: false,
+  external: ['react', 'react-dom'],
+})
+const module = { exports: {} }
+runInNewContext(bundle.outputFiles[0].text, {
+  module, exports: module.exports, setTimeout, clearTimeout, setInterval, clearInterval, Date, Promise,
+  require: (name) => {
+    const platform = {
+      react: { createElement: (type, props, ...children) => ({ type, props: props ?? {}, children: children.flat(Infinity) }) },
+    }
+    if (!(name in platform)) throw new Error(`no platform module ${name}`)
+    return platform[name]
+  },
+})
+const { BoardCard, DELIVERY_SKIN, TASK_SKIN } = module.exports
+
+const all = (node, out = []) => {
+  if (Array.isArray(node)) { for (const item of node) all(item, out); return out }
+  if (node && typeof node === 'object') { out.push(node); all(node.children, out) }
+  return out
+}
+const textOf = node => (typeof node === 'string' ? node : node && typeof node === 'object' ? node.children.map(textOf).join('') : '')
+
+const row = (number, extra) => ({ number, kind: 'pr', title: `Row ${number}`, labels: ['area:ui'], owner: 'Dev4', ownerKind: 'bot', badge: 'pass-enqueue', sha: 'abc1234', url: 'https://example.invalid/1', ...extra })
+
+test('a repository card is keyed by repository and number, so an issue and its pull request are one card', () => {
+  assert.equal(githubCardKey('o/r', 12), 'o/r#12')
+  assert.equal(githubCardKey('  o/r  ', '12'), 'o/r#12')
+  assert.equal(githubCardKey('', 12), null)
+  assert.equal(githubCardKey('o/r', 0), null)
+  assert.equal(githubCardKey('o/r', 'x'), null)
+  const issue = githubCard(row(12, { kind: 'issue' }), 'o/r')
+  const pull = githubCard(row(12, { kind: 'pr' }), 'o/r')
+  assert.equal(issue.key, pull.key)
+  assert.equal(sameCard(issue, pull), true)
+  assert.equal(issue.source, 'github')
+  assert.deepEqual(CARD_SOURCES, ['github', 'task'])
+})
+
+test('two repositories with the same number are two cards, and a row without one says so', () => {
+  assert.notEqual(githubCardKey('o/a', 7), githubCardKey('o/b', 7))
+  assert.equal(githubCard(row(7), 'o/a').repo, 'o/a')
+  // No repository: no cross-repository identity is implied, so the key names the kind instead.
+  assert.equal(githubCard(row(7), null).key, 'github:pr:7')
+  assert.equal(githubCard({ number: 0 }, 'o/r'), null)
+})
+
+test('a task card keeps its own id, and the two sources never collide', () => {
+  assert.equal(taskCardKey('task-abc'), 'task:task-abc')
+  assert.equal(taskCardKey('  '), null)
+  assert.equal(taskCard(null), null)
+  const card = taskCard({ id: 'task-abc', title: 'Ship it', note: 'halfway', ownerBotName: 'Dev4' })
+  assert.equal(card.source, 'task')
+  assert.equal(card.key, 'task:task-abc')
+  assert.equal(card.title, 'Ship it')
+  assert.equal(card.owner, null)
+  assert.equal(sameCard(card, githubCard(row(7), 'task:task-abc')), false)
+  assert.equal(sameCard(card, { key: 'task:task-abc' }), true)
+})
+
+test('one component draws both sources, each with the skin and identity of its board', () => {
+  const repository = githubCard(row(12, { linked: 13 }), 'o/r')
+  const task = taskCard({ id: 'task-abc', title: 'Ship it' })
+  const repositoryCard = BoardCard({ card: repository, skin: DELIVERY_SKIN, idText: '#12', badge: 'PASS · waiting for enqueue' })
+  const taskCardNode = BoardCard({ card: task, skin: TASK_SKIN, idText: '#abc' })
+
+  assert.equal(repositoryCard.props.className, 'bt-del-card')
+  assert.equal(repositoryCard.props['data-source'], 'github')
+  assert.equal(repositoryCard.props['data-key'], 'o/r#12')
+  assert.match(textOf(repositoryCard), /PASS · waiting for enqueue/)
+  assert.match(textOf(repositoryCard), /linked to/)
+
+  assert.equal(taskCardNode.props.className, 'bt-board-card')
+  assert.equal(taskCardNode.props['data-source'], 'task')
+  assert.equal(taskCardNode.props['data-key'], 'task:task-abc')
+  assert.match(textOf(taskCardNode), /Ship it/)
+
+  // The badge slot is only drawn when a board hands one in, so a task card keeps its own top row.
+  assert.equal(all(taskCardNode).some(node => node.props?.className === TASK_SKIN.badge), false)
+})